On Mon, Feb 10, 2025 at 05:03:19PM +0800, Ming Lei wrote: > PAGE_SIZE is applied in some block device queue limits, this way is > very fragile and is wrong: > > - queue limits are read from hardware, which is often one readonly > hardware property > > - PAGE_SIZE is one config option which can be changed during build time. This is true. > In RH lab, it has been found that max segment size of some mmc card is > less than 64K, then this kind of card can't work in case of 64K PAGE_SIZE. This is true, but check the note on block/blk-merge.c blk_bvec_map_sg(). It would seem that this is a limitation of MMC/SD and that this should ideally be fixed. > Fix this issue by using BLK_MIN_SEGMENT_SIZE in related code for dealing > with queue limits and checking if bio needn't split. Define BLK_MIN_SEGMENT_SIZE > as 4K(minimized PAGE_SIZE). But indeed if the block driver isn't yet fixed, then sure, we have to deal with the issue, I am not convinced that the logic below addresses this in a generic way, rather it seems to conflate the areas where we do need the generic block layer min defined, and when we have a block min segment limit. > Cc: Yi Zhang <yi.zhang@xxxxxxxxxx> > Cc: Luis Chamberlain <mcgrof@xxxxxxxxxx> > Cc: John Garry <john.g.garry@xxxxxxxxxx> > Cc: Bart Van Assche <bvanassche@xxxxxxx> > Cc: Keith Busch <kbusch@xxxxxxxxxx> > Link: https://lore.kernel.org/linux-block/20250102015620.500754-1-ming.lei@xxxxxxxxxx/ > Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx> > --- > V2: > - cover bio_split_rw_at() > - add BLK_MIN_SEGMENT_SIZE > > block/blk-merge.c | 2 +- > block/blk-settings.c | 6 +++--- > block/blk.h | 2 +- > include/linux/blkdev.h | 1 + > 4 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/block/blk-merge.c b/block/blk-merge.c > index 15cd231d560c..b55c52a42303 100644 > --- a/block/blk-merge.c > +++ b/block/blk-merge.c > @@ -329,7 +329,7 @@ int bio_split_rw_at(struct bio *bio, const struct queue_limits *lim, > > if (nsegs < lim->max_segments && > bytes + bv.bv_len <= max_bytes && > - bv.bv_offset + bv.bv_len <= PAGE_SIZE) { > + bv.bv_offset + bv.bv_len <= BLK_MIN_SEGMENT_SIZE) { > nsegs++; > bytes += bv.bv_len; I'll note that the 64k BLK_MAX_SEGMENT_SIZE is an old "odd historic" default value, ie, not a documented hard limit but some odd old thing which blk_validate_limits() encourages block drivers to override, so a soft max. That said, if we validate this soft max and if you also validate the min shouldn't value in the above instead be lim->max_segment_size instead, provided that we also address the coment in blk_bvec_map_sg()? More forward looking -- are you using BLK_MIN_SEGMENT_SIZE here due to the same mmc/sd limitations ? Can we overcome the mmc/sd limitations by only using this BLK_MIN_SEGMENT_SIZE only on block drivers which have the scatterlists limitation? The rest in your patch seem like sensible places to use a BLK_MIN_SEGMENT_SIZE although I need to think more about bio_may_need_split() with larger segments in mind some more. Luis
