Re: [PATCH V2] block: make segment size limit workable for > 4K PAGE_SIZE

[Ming Lei <ming.lei@redhat.com>]

On Fri, Feb 14, 2025 at 01:28:41PM +0100, Daniel Gomez wrote:
> On Fri, Feb 14, 2025 at 07:19:45PM +0100, Ming Lei wrote:
> > On Fri, Feb 14, 2025 at 10:38:36AM +0100, Daniel Gomez wrote:
> > > On Mon, Feb 10, 2025 at 05:03:19PM +0100, Ming Lei wrote:
> > > >  /**
> > > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> > > > index 248416ecd01c..32188af4051e 100644
> > > > --- a/include/linux/blkdev.h
> > > > +++ b/include/linux/blkdev.h
> > > > @@ -1163,6 +1163,7 @@ static inline bool bdev_is_partition(struct block_device *bdev)
> > > >  enum blk_default_limits {
> > > >  	BLK_MAX_SEGMENTS	= 128,
> > > >  	BLK_SAFE_MAX_SECTORS	= 255,
> > > > +	BLK_MIN_SEGMENT_SIZE	= 4096, /* min(PAGE_SIZE) */
> > > 
> > > I think it would be useful to expose this value to the queue_limits and
> > 
> > Can you share it is useful for what?
> 
> I meant for your use case.

No, it isn't single case, there are many such devices with < 64K
max_segment_size, please see previous Bart's post:

https://lore.kernel.org/linux-block/20230612203314.17820-1-bvanassche@acm.org/

> 
> > 
> > > sysfs (and remove it from here). We can default it to PAGE_SIZE (as it has
> > > always been) and allow to overwrite it when the block driver initializes the
> > 
> > Which device driver needs to initialize it?
> 
> I mean, it would be yours. Keeping the default minimum segment size to PAGE_SIZE
> rather than changing it to 4k, would keep the current behaviour. Then, adding
> the minimum segment limit would allow your driver to overwrite it for your use
> case.

But these devices doesn't export min_segment_size, why do you want to fake this
limit?

It is fragile to take variable PAGE_SIZE as soft min_segment_size, and
it is actually wrong to bind it with fixed hardware max_segment_size.

To be honest, not see any benefit with your approach, just make things
complicated.

Thanks,
Ming