linux-kernel - Re: [PATCH v4 6/6] block: use chunk_sectors when evaluating stacked atomic write limits

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f5ddc161-5683-f008-4794-32eccf88af65@redhat.com>
Date: Tue, 8 Jul 2025 18:59:17 +0200 (CEST)
From: Mikulas Patocka <mpatocka@...hat.com>
To: Nilay Shroff <nilay@...ux.ibm.com>
cc: John Garry <john.g.garry@...cle.com>, agk@...hat.com, snitzer@...nel.org, 
    song@...nel.org, yukuai3@...wei.com, hch@....de, axboe@...nel.dk, 
    cem@...nel.org, dm-devel@...ts.linux.dev, linux-kernel@...r.kernel.org, 
    linux-raid@...r.kernel.org, linux-block@...r.kernel.org, 
    ojaswin@...ux.ibm.com, martin.petersen@...cle.com, 
    akpm@...ux-foundation.org, linux-xfs@...r.kernel.org, djwong@...nel.org
Subject: Re: [PATCH v4 6/6] block: use chunk_sectors when evaluating stacked
 atomic write limits



On Tue, 8 Jul 2025, Nilay Shroff wrote:

> 
> 
> On 7/7/25 6:41 PM, John Garry wrote:
> > The atomic write unit max value is limited by any stacked device stripe
> > size.
> > 
> > It is required that the atomic write unit is a power-of-2 factor of the
> > stripe size.
> > 
> > Currently we use io_min limit to hold the stripe size, and check for a
> > io_min <= SECTOR_SIZE when deciding if we have a striped stacked device.
> > 
> > Nilay reports that this causes a problem when the physical block size is
> > greater than SECTOR_SIZE [0].
> > 
> > Furthermore, io_min may be mutated when stacking devices, and this makes
> > it a poor candidate to hold the stripe size. Such an example (of when
> > io_min may change) would be when the io_min is less than the physical
> > block size.
> > 
> > Use chunk_sectors to hold the stripe size, which is more appropriate.
> > 
> > [0] https://lore.kernel.org/linux-block/888f3b1d-7817-4007-b3b3-1a2ea04df771@linux.ibm.com/T/#mecca17129f72811137d3c2f1e477634e77f06781
> > 
> > Signed-off-by: John Garry <john.g.garry@...cle.com>
> > ---
> >  block/blk-settings.c | 58 ++++++++++++++++++++++++++------------------
> >  1 file changed, 35 insertions(+), 23 deletions(-)
> > 
> > diff --git a/block/blk-settings.c b/block/blk-settings.c
> > index 761c6ccf5af7..3259cfac5d0d 100644
> > --- a/block/blk-settings.c
> > +++ b/block/blk-settings.c
> > @@ -597,41 +597,52 @@ static bool blk_stack_atomic_writes_boundary_head(struct queue_limits *t,
> >  	return true;
> >  }
> >  
> > -
> > -/* Check stacking of first bottom device */
> > -static bool blk_stack_atomic_writes_head(struct queue_limits *t,
> > -				struct queue_limits *b)
> > +static void blk_stack_atomic_writes_chunk_sectors(struct queue_limits *t)
> >  {
> > -	if (b->atomic_write_hw_boundary &&
> > -	    !blk_stack_atomic_writes_boundary_head(t, b))
> > -		return false;
> > +	unsigned int chunk_sectors = t->chunk_sectors, chunk_bytes;
> >  
> > -	if (t->io_min <= SECTOR_SIZE) {
> > -		/* No chunk sectors, so use bottom device values directly */
> > -		t->atomic_write_hw_unit_max = b->atomic_write_hw_unit_max;
> > -		t->atomic_write_hw_unit_min = b->atomic_write_hw_unit_min;
> > -		t->atomic_write_hw_max = b->atomic_write_hw_max;
> > -		return true;
> > -	}
> > +	if (!chunk_sectors)
> > +		return;
> > +
> > +	/*
> > +	 * If chunk sectors is so large that its value in bytes overflows
> > +	 * UINT_MAX, then just shift it down so it definitely will fit.
> > +	 * We don't support atomic writes of such a large size anyway.
> > +	 */
> > +	if ((unsigned long)chunk_sectors << SECTOR_SHIFT > UINT_MAX)
> > +		chunk_bytes = chunk_sectors;
> > +	else
> > +		chunk_bytes = chunk_sectors << SECTOR_SHIFT;

Why do we cast it to unsigned long? unsigned long is 32-bit on 32-bit 
machines, so the code will not detect the overflow in that case. We should 
cast it to unsigned long long (or uint64_t).

Mikulas