Message ID | 20241101052206.437530-2-hch@lst.de (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [1/4] block: fix bio_split_rw_at to take zone_write_granularity into account | expand |
On 11/1/24 14:21, Christoph Hellwig wrote: > Otherwise it can create unaligned writes on zoned devices. > > Fixes: a805a4fa4fa3 ("block: introduce zone_write_granularity limit") > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > block/blk-merge.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/block/blk-merge.c b/block/blk-merge.c > index d813d799cee7..d6895859a2fb 100644 > --- a/block/blk-merge.c > +++ b/block/blk-merge.c > @@ -358,7 +358,8 @@ int bio_split_rw_at(struct bio *bio, const struct queue_limits *lim, > * split size so that each bio is properly block size aligned, even if > * we do not use the full hardware limits. > */ > - bytes = ALIGN_DOWN(bytes, lim->logical_block_size); > + bytes = ALIGN_DOWN(bytes, lim->zone_write_granularity ? > + lim->zone_write_granularity : lim->logical_block_size); Nit: we could also do: bytes = ALIGN_DOWN(bytes, max(lim->logical_block_size, lim->zone_write_granularity)); Also, I wonder if we should leave read split as is based on the logical block size only ? Probably does not matter much... > > /* > * Bio splitting may cause subtle trouble such as hang when doing sync
On Fri, Nov 01, 2024 at 02:34:31PM +0900, Damien Le Moal wrote: > > - bytes = ALIGN_DOWN(bytes, lim->logical_block_size); > > + bytes = ALIGN_DOWN(bytes, lim->zone_write_granularity ? > > + lim->zone_write_granularity : lim->logical_block_size); > > Nit: we could also do: > > bytes = ALIGN_DOWN(bytes, > max(lim->logical_block_size, lim->zone_write_granularity)); That's what I had first. It is a little odd as zone_write_granularity is defined to be >= logical_block_size, though. > Also, I wonder if we should leave read split as is based on the logical block > size only ? Probably does not matter much... Good point. Probably doesn't matter much, but randomly forcing it on reads seems odd.
diff --git a/block/blk-merge.c b/block/blk-merge.c index d813d799cee7..d6895859a2fb 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c @@ -358,7 +358,8 @@ int bio_split_rw_at(struct bio *bio, const struct queue_limits *lim, * split size so that each bio is properly block size aligned, even if * we do not use the full hardware limits. */ - bytes = ALIGN_DOWN(bytes, lim->logical_block_size); + bytes = ALIGN_DOWN(bytes, lim->zone_write_granularity ? + lim->zone_write_granularity : lim->logical_block_size); /* * Bio splitting may cause subtle trouble such as hang when doing sync
Otherwise it can create unaligned writes on zoned devices. Fixes: a805a4fa4fa3 ("block: introduce zone_write_granularity limit") Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/blk-merge.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)