On Fri, Jun 21, 2024 at 11:36:20AM +0800, Ming Lei wrote: > On Thu, Jun 20, 2024 at 11:07:19PM -0400, Kent Overstreet wrote: > > On Fri, Jun 21, 2024 at 09:48:11AM +0800, Ming Lei wrote: > > > On Thu, Jun 20, 2024 at 11:43:44AM -0400, Kent Overstreet wrote: > > > > On Thu, Jun 20, 2024 at 05:30:50PM +0200, Christoph Hellwig wrote: > > > > > On Thu, Jun 20, 2024 at 02:54:09PM +0100, Matthew Wilcox wrote: > > > > > > I'm against it. Block devices only do sector-aligned IO and we should > > > > > > not pretend otherwise. > > > > > > > > > > While I agree with that, the bvec_iter is actually used in a few other > > > > > places and could be used in more, and the 512-byte sector unit bi_sector > > > > > is the only weird thing that's not useful elsewhere. So turning that > > > > > into a > > > > > > > > > > u64 bi_addr; > > > > > > > > > > that is byte based where the meaning is specific to the user would > > > > > actually be kinda nice. For traditional block users we'd need a > > > > > bio_sector() helpers similar to the existing bio_sectors() one, > > > > > but a lot of non-trivial drivers actually need to translated to > > > > > a variable LBA-based addressing, which would be (a tiny little bit) > > > > > simpler with the byte address. As bi_size is already in bytes > > > > > it would also fit in pretty naturally with that. > > > > > > > > > > The only thing that is really off putting is the amount of churn that > > > > > this would cause. > > > > > > > > I'm being imprecise when I just say 'struct bio'; there's things in > > > > there that are block layer specific but there are also things in there > > > > you want that aren't block layer specific (completion callback, write > > > > flags, s/bi_bdev/bi_inode and that as well, perhaps). It's not at all > > > > clear to me we'd want to deal with the churn to split that up or make > > > > bio itself less block layer specific (although, but when I say 'aiming > > > > for commality with struct bio' that sort of thing is what I have in > > > > mind. > > > > > > > > But more immediately, yes - bi_addr as all we need for this, and like > > > > you said I think it'd be a worthwhile change. > > > > > > Still not clear why you need unaligned bi_addr for bio, if this bio needs > > > to call submit_bio(), it has to be aligned. Otherwise, you could invent any > > > structure for this purpose, and the structure can be payload of bio for > > > avoiding extra allocation, even it can be FS generic structure. > > > > We want to have fewer scatter/gather list data structures, not more. > > OK, that look fine to change to bi_addr since bvec_iter is widely used now, > maybe .bi_sector can be moved into bio, cause bvec iterator needn't it. No, we'd just add a bio_sector() helper that does the shift.