Re: bvec_iter.bi_sector -> loff_t? (was: Re: [PATCH] bcachefs: allow direct io fallback to buffer io for) unaligned length or offset

[Ming Lei <ming.lei@redhat.com>]

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.


Thanks,
Ming