Re: [PATCH v2 00/21] netfs: Keep track of folios in a segmented bio_vec[] chain

[David Howells]

David Laight <david.laight.linux@xxxxxxxxx> wrote:

> > 	struct bvecq {
> > 		struct bvecq		*next;
> > 		struct bvecq		*prev;
> > 		unsigned long long	fpos;
> > 		refcount_t		ref;
> > 		u32			priv;
> > 		u16			nr_segs;
> > 		u16			max_segs;
> > 		enum bvecq_mem		mem_type:2;
> > 		bool			inline_bv:1;
> > 		bool			discontig:1;
> 
> There doesn't seem to be any point using bitfields.
> There is a massive hole here anyway.

Depends on how you define "massive".  On a 64-bit machine, the whole thing
fits into 48 bytes - 6 words (or 3 bio_vec slots).  next, prev, fpos, bv and
ref+priv take up 5 of those words; nr_segs and max_segs take up half of the
6th, leaving a 4 byte hole.

You're right, though, I could make them all non-bitfields as the enum is
marked mode(byte).

> >  (1) next, prev - Link segments together in a list.  I want this to be
> >      NULL-terminated linear rather than circular to make it possible to
> >      arbitrarily glue bits on the front.
> 
> Do you ever need to follow the list backwards?

iov_iter_revert() exists, unfortunately, but yes, I would like to avoid having
a prev pointer.

I have a couple of ideas on how to get rid of that - or at least store the
start in struct iov_iter and always work forwards - but I haven't got round to
trying that yet.

> >  (2) fpos, discontig - Note the current file position of the first byte of
> >      the segment; all the bio_vecs in ->bv[] must be contiguous in the file
> >      space.  The fpos can be used to find the folio by file position rather
> >      then from the info in the bio_vec.
> 
> Should fpos be off_t (or u64) rather than 'long long' (they are all the
> same underlying type).

It's not 'long long' and off_t is actually 'long' in asm-generic.  Actually, I
should probably switch to using uoff_t.  Note that this file position should
never be seen as negative; I think loff_t should only really be used in
llseek.

> >      If there's a discontiguity, this should break over into a new bvecq
> >      segment with the discontig flag set (though this is redundant if you
> >      keep track of the file position).  Note that the beginning and end
> >      file positions in a segment need not be aligned to any filesystem
> >      block size.
> 
> At this point you lose me :-)

Apologies, but I'm trying to define how a bvecq chain works.  I need to codify
it more coherently.

So there's a number of reasons I want to be able to maintain the file position
information in the chain:

 (1) I can treat buffered writeback and DIO write more similarly if there's no
     requirement to access the folios in the list to get file position
     information.

 (2) When cleaning up lists of folios in buffered writeback, the file position
     is needed to access the i_pages xarray in order to clean up the marks on
     it.  This means I don't need to go from my list to access each folio, but
     can look them up through the xarray instead.

 (3) Some network filesystems, e.g. ceph, allow discontiguous (sparse) writes
     to be made to the server in a single RPC operation.  This gives a means
     to convey that information to them, but then allows the data to be
     conveyed in a single blob to the socket (the mapping between blob offsets
     and file regions is tabulated separately within the RPC call).

Note that some of this also applies to reads too.

The last bit about filesystem block size alignment is because network
filesystems don't typically require any block alignment, doing RMW locally on
the server.  I should really have separated that from the discontiguity bit.

David