Re: [PATCH] badblocks: actually round to block boundaries in set/clear/check

[Coly Li]

Hi Rongqing,

On 9/30/26 11:49 AM, lirongqing wrote:
> From: Li RongQing <lirongqing@xxxxxxxxx>
>
> _badblocks_set(), _badblocks_clear() and badblocks_check() intend to
> round the start/end of a range to the bb->shift block boundary before
> operating on the bad-blocks table. They call rounddown()/roundup() as
> bare statements:
>
> 	rounddown(s, 1 << bb->shift);
> 	roundup(next, 1 << bb->shift);
> 	sectors = next - s;
>
> But rounddown()/roundup() (include/linux/math.h) are value-returning
> statement-expression macros; they do not modify their argument in
> place. The results were discarded, so s and next/target were never
> rounded and "sectors = next - s" collapsed back to the original
> length. The whole "if (bb->shift)" block was a no-op, and ranges on
> devices with bb->shift > 0 were stored/queried at raw sector
> granularity instead of being block-aligned -- exactly the "think a
> block is not bad when it is" hazard the _badblocks_clear() comment
> warns about.
>
> Assign the rounded values back so the alignment takes effect.
>
> Fixes: 3ea3354cb9f0 ("badblocks: improve badblocks_check() for multiple ranges handling")
> Fixes: db448eb68629 ("badblocks: improve badblocks_clear() for multiple ranges handling")
> Fixes: 1726c7746783 ("badblocks: improve badblocks_set() for multiple ranges handling")
> Signed-off-by: Li RongQing <lirongqing@xxxxxxxxx>


This is fixed and the patches were waiting for merge IIRC.

Just FYI.


Coly Li


> ---
>   block/badblocks.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/block/badblocks.c b/block/badblocks.c
> index ece64e7..eafdb6a 100644
> --- a/block/badblocks.c
> +++ b/block/badblocks.c
> @@ -857,8 +857,8 @@ static bool _badblocks_set(struct badblocks *bb, sector_t s, sector_t sectors,
>   		/* round the start down, and the end up */
>   		sector_t next = s + sectors;
>   
> -		rounddown(s, 1 << bb->shift);
> -		roundup(next, 1 << bb->shift);
> +		s = rounddown(s, 1 << bb->shift);
> +		next = roundup(next, 1 << bb->shift);
>   		sectors = next - s;
>   	}
>   
> @@ -1071,8 +1071,8 @@ static bool _badblocks_clear(struct badblocks *bb, sector_t s, sector_t sectors)
>   		 * isn't than to think a block is not bad when it is.
>   		 */
>   		target = s + sectors;
> -		roundup(s, 1 << bb->shift);
> -		rounddown(target, 1 << bb->shift);
> +		s = roundup(s, 1 << bb->shift);
> +		target = rounddown(target, 1 << bb->shift);
>   		sectors = target - s;
>   	}
>   
> @@ -1307,8 +1307,8 @@ int badblocks_check(struct badblocks *bb, sector_t s, sector_t sectors,
>   		/* round the start down, and the end up */
>   		sector_t target = s + sectors;
>   
> -		rounddown(s, 1 << bb->shift);
> -		roundup(target, 1 << bb->shift);
> +		s = rounddown(s, 1 << bb->shift);
> +		target = roundup(target, 1 << bb->shift);
>   		sectors = target - s;
>   	}