On 4/7/25 03:07, Christoph Hellwig wrote:
On Sun, Apr 06, 2025 at 03:40:04PM -0400, Sean Anderson wrote:
Hi all,
I'm not really sure what guarantees the block layer makes regarding the
segments in a bio as part of a request submitted to a block driver. As
far as I can tell this is not documented anywhere. In particular,
First you need to define what segment you mean. We have at least two and
a half historical uses of the name. One is for each bio_vec attached to
the bio, either directly as submitted into ->submit_bio for bio based
drivers (case 1a), or generated by bio_split_to_limits (case 1b), which
is called for every blk-mq driver before calling into ->queue_rq(s) or
explicitly called by a few bio based driver.
The other is the bio-vec synthesized by bio_for_each_segment (case 2).
I'm referring to the bio_vecs you get from queue_mq. Which I think is the
latter.
- Is bv_len aligned to SECTOR_SIZE?
Yes.
- To logical_sector_size?
Yes.
OK, but...
- What if logical_sector_size > PAGE_SIZE?
Still always aligned to logical_sector_size.
- What about bv_offset?
bv_offset is a memory offset and must only be aligned to the
dma_alignment limit.
- Is it possible to have a bio where the total length is a multiple of
logical_sector_size, but the data is split across several segments
where each segment is a multiple of SECTOR_SIZE?
Yes.
...if this is the case, then for some of those segments wouldn't bv_len
not be a multiple of logical_sector_size?
- Is is possible to have segments not even aligned to SECTOR_SIZE?
No.
- Can I somehow request to only get segments with bv_len aligned to
logical_sector_size?
For drivers that use bio_split_to_limits implicitly or explicitly you can
do that by setting the right seg_boundary_mask.
Is that the right knob? It operates on the physical address, so it looked
more like something for broken DMA engines. For example (if I recall correctly)
MMC SDMA can't cross a page boundary, so you could use seg_boundary_mask to
enforce that.
make some big assumptions (which might be bugs?) For example, in
drivers/mtd/mtd_blkdevs.c, do_blktrans_request looks like:
- There is only one bio in a request. This one is a bit of a soft
assumption since we should only flush the pages in the bio and not the
whole request otherwise.
It always operates on the first bio in the request and then uses
blk_update_request to move the context past that. It is an old
and somewhat arkane way to write drivers, but should work. The
rq_for_each_segment looks do call flush_dcache_page look horribly
wrong for this model, though.
- The data is in lowmem OR bv_offset + bv_len <= PAGE_SIZE. kmap() only
maps a single page, so if we go past one page we end up in adjacent
kmapped pages.
Yes, this looks broken.
Am I missing something here? Handling highmem seems like a persistent
issue. E.g. drivers/mtd/ubi/block.c doesn't even bother doing a kmap.
Should both of these have BLK_FEAT_BOUNCE_HIGH?
BLK_FEAT_BOUNCE_HIGH needs to go away rather sooner than later.
in the short run the best fix would be to synthesized a
bio_for_each_segment like bio_vec that stays inside a single page
using bio_iter_iovec) at the top of do_blktrans_request and use
that for all references to the data.
OK, but if you have to stay inside a single page couldn't you end up
with a sector spanning a page boundary due to only being aligned to
dma_alignment? Or maybe we set seg_boundary_mask to PAGE_MASK to enforce that?
--Sean
