On Tue, Feb 11, 2025 at 10:10:36AM +0100, Ming Lei wrote: > On Mon, Feb 10, 2025 at 12:17:07PM -0800, Luis Chamberlain wrote: > > 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. > > The mmc card works just fine in case of 4K page size, there isn't any > limitation for the mmc/ssd from storage viewpoint, the failure is just > because this card's max segment size is < 64KB in case of 64K page size. > > > > > > 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. > > BLK_MAX_SEGMENT_SIZE is default or fallback max segment size if the hardware > doesn't provide this limit, so nothing odd here because block layer has > to use something reasonable here. > > > > > That said, if we validate this soft max and if you also validate the min > > There isn't soft max segment size. > > > shouldn't value in the above instead be lim->max_segment_size instead, > > min segment size is page_size and it is soft, and has been applied > for long time. This patch just fixes it as 4k(min(page_size)). > > > provided that we also address the coment in blk_bvec_map_sg()? > > The comment in blk_bvec_map_sg() has been removed, and blk_bvec_map_sg > has been re-written in commit b7175e24d6ac ("block: add a dma mapping > iterator") by following segment limits only. Would it be possible for the driver to split the minimum segment size, PAGE_SIZE (64k in your case), into smaller chunks that your hardware supports? For example, NVMe supports 512-byte I/Os while maintaining the minimum segment boundary at 4k. > > > > > 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? > > Please see my comment above, the mmc card doesn't have any limitation, > it is just that its max segment size is < 64K, which is absolutely > allowed from storage viewpoint. > > > Thanks, > Ming >
