From: Yu Kuai <yukuai1@huaweicloud.com>
To: Zheng Qixing <zhengqixing@huaweicloud.com>,
axboe@kernel.dk, song@kernel.org, colyli@kernel.org,
dan.j.williams@intel.com, vishal.l.verma@intel.com,
dave.jiang@intel.com, ira.weiny@intel.com, dlemoal@kernel.org,
yanjun.zhu@linux.dev, kch@nvidia.com, hare@suse.de,
zhengqixing@huawei.com, john.g.garry@oracle.com,
geliang@kernel.org, xni@redhat.com, colyli@suse.de
Cc: linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-raid@vger.kernel.org, nvdimm@lists.linux.dev,
yi.zhang@huawei.com, yangerkun@huawei.com,
"yukuai (C)" <yukuai3@huawei.com>
Subject: Re: [PATCH 12/12] badblocks: use sector_t instead of int to avoid truncation of badblocks length
Date: Sat, 22 Feb 2025 09:43:44 +0800 [thread overview]
Message-ID: <f013c2e1-adc0-9a09-67b3-e07f89f936e7@huaweicloud.com> (raw)
In-Reply-To: <20250221081109.734170-13-zhengqixing@huaweicloud.com>
在 2025/02/21 16:11, Zheng Qixing 写道:
> From: Zheng Qixing <zhengqixing@huawei.com>
>
> There is a truncation of badblocks length issue when set badblocks as
> follow:
>
> echo "2055 4294967299" > bad_blocks
> cat bad_blocks
> 2055 3
>
> Change 'sectors' argument type from 'int' to 'sector_t'.
>
> This change avoids truncation of badblocks length for large sectors by
> replacing 'int' with 'sector_t' (u64), enabling proper handling of larger
> disk sizes and ensuring compatibility with 64-bit sector addressing.
>
> Fixes: 9e0e252a048b ("badblocks: Add core badblock management code")
> Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
> ---
> block/badblocks.c | 20 ++++++++------------
> drivers/block/null_blk/main.c | 2 +-
> drivers/md/md.h | 6 +++---
> drivers/md/raid1-10.c | 2 +-
> drivers/md/raid1.c | 4 ++--
> drivers/md/raid10.c | 8 ++++----
> drivers/nvdimm/nd.h | 2 +-
> drivers/nvdimm/pfn_devs.c | 7 ++++---
> drivers/nvdimm/pmem.c | 2 +-
> include/linux/badblocks.h | 8 ++++----
> 10 files changed, 29 insertions(+), 32 deletions(-)
>
LGTM
Reviewed-by: Yu Kuai <yukuai3@huawei.com>
> diff --git a/block/badblocks.c b/block/badblocks.c
> index 8f057563488a..14e3be47d22d 100644
> --- a/block/badblocks.c
> +++ b/block/badblocks.c
> @@ -836,7 +836,7 @@ static bool try_adjacent_combine(struct badblocks *bb, int prev)
> }
>
> /* Do exact work to set bad block range into the bad block table */
> -static bool _badblocks_set(struct badblocks *bb, sector_t s, int sectors,
> +static bool _badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
> int acknowledged)
> {
> int len = 0, added = 0;
> @@ -956,8 +956,6 @@ static bool _badblocks_set(struct badblocks *bb, sector_t s, int sectors,
> if (sectors > 0)
> goto re_insert;
>
> - WARN_ON(sectors < 0);
> -
> /*
> * Check whether the following already set range can be
> * merged. (prev < 0) condition is not handled here,
> @@ -1048,7 +1046,7 @@ static int front_splitting_clear(struct badblocks *bb, int prev,
> }
>
> /* Do the exact work to clear bad block range from the bad block table */
> -static bool _badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
> +static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
> {
> struct badblocks_context bad;
> int prev = -1, hint = -1;
> @@ -1171,8 +1169,6 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
> if (sectors > 0)
> goto re_clear;
>
> - WARN_ON(sectors < 0);
> -
> if (cleared) {
> badblocks_update_acked(bb);
> set_changed(bb);
> @@ -1187,8 +1183,8 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
> }
>
> /* Do the exact work to check bad blocks range from the bad block table */
> -static int _badblocks_check(struct badblocks *bb, sector_t s, int sectors,
> - sector_t *first_bad, int *bad_sectors)
> +static int _badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
> + sector_t *first_bad, sector_t *bad_sectors)
> {
> int prev = -1, hint = -1, set = 0;
> struct badblocks_context bad;
> @@ -1298,8 +1294,8 @@ static int _badblocks_check(struct badblocks *bb, sector_t s, int sectors,
> * -1: there are bad blocks which have not yet been acknowledged in metadata.
> * plus the start/length of the first bad section we overlap.
> */
> -int badblocks_check(struct badblocks *bb, sector_t s, int sectors,
> - sector_t *first_bad, int *bad_sectors)
> +int badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
> + sector_t *first_bad, sector_t *bad_sectors)
> {
> unsigned int seq;
> int rv;
> @@ -1340,7 +1336,7 @@ EXPORT_SYMBOL_GPL(badblocks_check);
> * true: success
> * false: failed to set badblocks (out of space)
> */
> -bool badblocks_set(struct badblocks *bb, sector_t s, int sectors,
> +bool badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
> int acknowledged)
> {
> return _badblocks_set(bb, s, sectors, acknowledged);
> @@ -1361,7 +1357,7 @@ EXPORT_SYMBOL_GPL(badblocks_set);
> * true: success
> * false: failed to clear badblocks
> */
> -bool badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
> +bool badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
> {
> return _badblocks_clear(bb, s, sectors);
> }
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index 623db72ad66b..6e7d80b6e92b 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -1302,7 +1302,7 @@ static inline blk_status_t null_handle_badblocks(struct nullb_cmd *cmd,
> {
> struct badblocks *bb = &cmd->nq->dev->badblocks;
> sector_t first_bad;
> - int bad_sectors;
> + sector_t bad_sectors;
>
> if (badblocks_check(bb, sector, nr_sectors, &first_bad, &bad_sectors))
> return BLK_STS_IOERR;
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 923a0ef51efe..6edc0f71b7d4 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -266,8 +266,8 @@ enum flag_bits {
> Nonrot, /* non-rotational device (SSD) */
> };
>
> -static inline int is_badblock(struct md_rdev *rdev, sector_t s, int sectors,
> - sector_t *first_bad, int *bad_sectors)
> +static inline int is_badblock(struct md_rdev *rdev, sector_t s, sector_t sectors,
> + sector_t *first_bad, sector_t *bad_sectors)
> {
> if (unlikely(rdev->badblocks.count)) {
> int rv = badblocks_check(&rdev->badblocks, rdev->data_offset + s,
> @@ -284,7 +284,7 @@ static inline int rdev_has_badblock(struct md_rdev *rdev, sector_t s,
> int sectors)
> {
> sector_t first_bad;
> - int bad_sectors;
> + sector_t bad_sectors;
>
> return is_badblock(rdev, s, sectors, &first_bad, &bad_sectors);
> }
> diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
> index 4378d3250bd7..62b980b12f93 100644
> --- a/drivers/md/raid1-10.c
> +++ b/drivers/md/raid1-10.c
> @@ -247,7 +247,7 @@ static inline int raid1_check_read_range(struct md_rdev *rdev,
> sector_t this_sector, int *len)
> {
> sector_t first_bad;
> - int bad_sectors;
> + sector_t bad_sectors;
>
> /* no bad block overlap */
> if (!is_badblock(rdev, this_sector, *len, &first_bad, &bad_sectors))
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 8beb8cccc6af..0b2839105857 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1537,7 +1537,7 @@ static void raid1_write_request(struct mddev *mddev, struct bio *bio,
> atomic_inc(&rdev->nr_pending);
> if (test_bit(WriteErrorSeen, &rdev->flags)) {
> sector_t first_bad;
> - int bad_sectors;
> + sector_t bad_sectors;
> int is_bad;
>
> is_bad = is_badblock(rdev, r1_bio->sector, max_sectors,
> @@ -2886,7 +2886,7 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
> } else {
> /* may need to read from here */
> sector_t first_bad = MaxSector;
> - int bad_sectors;
> + sector_t bad_sectors;
>
> if (is_badblock(rdev, sector_nr, good_sectors,
> &first_bad, &bad_sectors)) {
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 7ed933181712..a8664e29aada 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -747,7 +747,7 @@ static struct md_rdev *read_balance(struct r10conf *conf,
>
> for (slot = 0; slot < conf->copies ; slot++) {
> sector_t first_bad;
> - int bad_sectors;
> + sector_t bad_sectors;
> sector_t dev_sector;
> unsigned int pending;
> bool nonrot;
> @@ -1438,7 +1438,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
> if (rdev && test_bit(WriteErrorSeen, &rdev->flags)) {
> sector_t first_bad;
> sector_t dev_sector = r10_bio->devs[i].addr;
> - int bad_sectors;
> + sector_t bad_sectors;
> int is_bad;
>
> is_bad = is_badblock(rdev, dev_sector, max_sectors,
> @@ -3413,7 +3413,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> sector_t from_addr, to_addr;
> struct md_rdev *rdev = conf->mirrors[d].rdev;
> sector_t sector, first_bad;
> - int bad_sectors;
> + sector_t bad_sectors;
> if (!rdev ||
> !test_bit(In_sync, &rdev->flags))
> continue;
> @@ -3609,7 +3609,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> for (i = 0; i < conf->copies; i++) {
> int d = r10_bio->devs[i].devnum;
> sector_t first_bad, sector;
> - int bad_sectors;
> + sector_t bad_sectors;
> struct md_rdev *rdev;
>
> if (r10_bio->devs[i].repl_bio)
> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> index 5ca06e9a2d29..cc5c8f3f81e8 100644
> --- a/drivers/nvdimm/nd.h
> +++ b/drivers/nvdimm/nd.h
> @@ -673,7 +673,7 @@ static inline bool is_bad_pmem(struct badblocks *bb, sector_t sector,
> {
> if (bb->count) {
> sector_t first_bad;
> - int num_bad;
> + sector_t num_bad;
>
> return !!badblocks_check(bb, sector, len / 512, &first_bad,
> &num_bad);
> diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
> index cfdfe0eaa512..8f3e816e805d 100644
> --- a/drivers/nvdimm/pfn_devs.c
> +++ b/drivers/nvdimm/pfn_devs.c
> @@ -367,9 +367,10 @@ static int nd_pfn_clear_memmap_errors(struct nd_pfn *nd_pfn)
> struct nd_namespace_common *ndns = nd_pfn->ndns;
> void *zero_page = page_address(ZERO_PAGE(0));
> struct nd_pfn_sb *pfn_sb = nd_pfn->pfn_sb;
> - int num_bad, meta_num, rc, bb_present;
> + int meta_num, rc, bb_present;
> sector_t first_bad, meta_start;
> struct nd_namespace_io *nsio;
> + sector_t num_bad;
>
> if (nd_pfn->mode != PFN_MODE_PMEM)
> return 0;
> @@ -394,7 +395,7 @@ static int nd_pfn_clear_memmap_errors(struct nd_pfn *nd_pfn)
> bb_present = badblocks_check(&nd_region->bb, meta_start,
> meta_num, &first_bad, &num_bad);
> if (bb_present) {
> - dev_dbg(&nd_pfn->dev, "meta: %x badblocks at %llx\n",
> + dev_dbg(&nd_pfn->dev, "meta: %llx badblocks at %llx\n",
> num_bad, first_bad);
> nsoff = ALIGN_DOWN((nd_region->ndr_start
> + (first_bad << 9)) - nsio->res.start,
> @@ -413,7 +414,7 @@ static int nd_pfn_clear_memmap_errors(struct nd_pfn *nd_pfn)
> }
> if (rc) {
> dev_err(&nd_pfn->dev,
> - "error clearing %x badblocks at %llx\n",
> + "error clearing %llx badblocks at %llx\n",
> num_bad, first_bad);
> return rc;
> }
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index d81faa9d89c9..43156e1576c9 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -249,7 +249,7 @@ __weak long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff,
> unsigned int num = PFN_PHYS(nr_pages) >> SECTOR_SHIFT;
> struct badblocks *bb = &pmem->bb;
> sector_t first_bad;
> - int num_bad;
> + sector_t num_bad;
>
> if (kaddr)
> *kaddr = pmem->virt_addr + offset;
> diff --git a/include/linux/badblocks.h b/include/linux/badblocks.h
> index 8764bed9ff16..996493917f36 100644
> --- a/include/linux/badblocks.h
> +++ b/include/linux/badblocks.h
> @@ -48,11 +48,11 @@ struct badblocks_context {
> int ack;
> };
>
> -int badblocks_check(struct badblocks *bb, sector_t s, int sectors,
> - sector_t *first_bad, int *bad_sectors);
> -bool badblocks_set(struct badblocks *bb, sector_t s, int sectors,
> +int badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
> + sector_t *first_bad, sector_t *bad_sectors);
> +bool badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
> int acknowledged);
> -bool badblocks_clear(struct badblocks *bb, sector_t s, int sectors);
> +bool badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors);
> void ack_all_badblocks(struct badblocks *bb);
> ssize_t badblocks_show(struct badblocks *bb, char *page, int unack);
> ssize_t badblocks_store(struct badblocks *bb, const char *page, size_t len,
>
next prev parent reply other threads:[~2025-02-22 1:43 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-21 8:10 [PATCH 00/12] badblocks: bugfix and cleanup for badblocks Zheng Qixing
2025-02-21 8:10 ` [PATCH 01/12] badblocks: Fix error shitf ops Zheng Qixing
2025-02-21 9:05 ` Yu Kuai
2025-02-21 9:58 ` Coly Li
2025-02-21 8:10 ` [PATCH 02/12] badblocks: factor out a helper try_adjacent_combine Zheng Qixing
2025-02-21 9:07 ` Yu Kuai
2025-02-21 10:04 ` Coly Li
2025-02-21 10:06 ` Coly Li
2025-02-21 8:11 ` [PATCH 03/12] badblocks: attempt to merge adjacent badblocks during ack_all_badblocks Zheng Qixing
2025-02-21 9:12 ` Yu Kuai
2025-02-21 10:08 ` Coly Li
2025-02-21 8:11 ` [PATCH 04/12] badblocks: return error directly when setting badblocks exceeds 512 Zheng Qixing
2025-02-21 9:55 ` Yu Kuai
2025-02-25 9:13 ` Li Nan
2025-02-21 8:11 ` [PATCH 05/12] badblocks: return error if any badblock set fails Zheng Qixing
2025-02-21 9:52 ` Coly Li
2025-02-21 10:09 ` Yu Kuai
2025-02-21 10:12 ` Coly Li
2025-02-22 1:12 ` Yu Kuai
2025-02-22 6:16 ` Coly Li
2025-02-25 9:12 ` Li Nan
2025-02-22 6:18 ` Coly Li
2025-02-21 8:11 ` [PATCH 06/12] badblocks: fix the using of MAX_BADBLOCKS Zheng Qixing
2025-02-21 10:09 ` Zhu Yanjun
2025-02-25 9:14 ` Li Nan
2025-02-21 10:22 ` Coly Li
2025-02-22 1:15 ` Yu Kuai
2025-02-21 8:11 ` [PATCH 07/12] badblocks: try can_merge_front before overlap_front Zheng Qixing
2025-02-22 1:14 ` Yu Kuai
2025-02-21 8:11 ` [PATCH 08/12] badblocks: fix merge issue when new badblocks align with pre+1 Zheng Qixing
2025-02-22 1:21 ` Yu Kuai
2025-02-21 8:11 ` [PATCH 09/12] badblocks: fix missing bad blocks on retry in _badblocks_check() Zheng Qixing
2025-02-21 10:27 ` Coly Li
2025-02-22 1:28 ` Yu Kuai
2025-02-21 8:11 ` [PATCH 10/12] badblocks: return boolen from badblocks_set() and badblocks_clear() Zheng Qixing
2025-02-22 1:26 ` Yu Kuai
2025-02-22 4:32 ` Paul Menzel
2025-02-22 4:36 ` Paul Menzel
2025-02-22 6:20 ` Coly Li
2025-02-21 8:11 ` [PATCH 11/12] md: improve return types of badblocks handling functions Zheng Qixing
2025-02-22 1:27 ` Yu Kuai
2025-02-21 8:11 ` [PATCH 12/12] badblocks: use sector_t instead of int to avoid truncation of badblocks length Zheng Qixing
2025-02-21 10:37 ` Coly Li
2025-02-22 1:43 ` Yu Kuai [this message]
2025-02-21 9:00 ` [PATCH 00/12] badblocks: bugfix and cleanup for badblocks Yu Kuai
2025-02-21 11:52 ` Coly Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f013c2e1-adc0-9a09-67b3-e07f89f936e7@huaweicloud.com \
--to=yukuai1@huaweicloud.com \
--cc=axboe@kernel.dk \
--cc=colyli@kernel.org \
--cc=colyli@suse.de \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=dlemoal@kernel.org \
--cc=geliang@kernel.org \
--cc=hare@suse.de \
--cc=ira.weiny@intel.com \
--cc=john.g.garry@oracle.com \
--cc=kch@nvidia.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-raid@vger.kernel.org \
--cc=nvdimm@lists.linux.dev \
--cc=song@kernel.org \
--cc=vishal.l.verma@intel.com \
--cc=xni@redhat.com \
--cc=yangerkun@huawei.com \
--cc=yanjun.zhu@linux.dev \
--cc=yi.zhang@huawei.com \
--cc=yukuai3@huawei.com \
--cc=zhengqixing@huawei.com \
--cc=zhengqixing@huaweicloud.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®