Bagas Sanjaya <bagasdotme@xxxxxxxxx> wrote:
> > + * For writeback, it is unknown how much there will be to write until the
> "... will be written ..."
> > + pagecache is walked, so no limit is set by the library.
No, I mean "how much there will be to write" - ie. how much dirty data there
is in the pagecache.
> > +Further, if a read from the cache fails, the library will ask the filesystem to
> > +do the read instead, renegotiating and retiling the subrequests as necessary.
> Read from the filesystem itself or direct read?
I'm not sure what you mean. Here, I'm talking about read subrequests - i.e. a
subrequest that corresponds to a BIO issued to the cache or a single RPC
issued to the server. Things like DIO and pagecache are at a higher level and
not directly exposed to the filesystem.
Maybe I should amend the text to read:
Further, if one or more subrequests issued to read from the cache
fail, the library will issue them to the filesystem instead,
renegotiating and retiling the subrequests as necessary.
> > +Netfslib will pin resources on an inode for future writeback (such as pinning
> > +use of an fscache cookie) when an inode is dirtied. However, this needs
> > +managing. Firstly, a function is provided to unpin the writeback in
> inode management?
> > +``->write_inode()``::
Is "inode management" meant to be a suggested insertion or an alternative for
the subsection title?
> > -The above fields are the ones the netfs can use. They are:
> > +They are:
> "These fields are, in detail:"
It feels unnecessarily repetitive to say "these fields", but "they are" also
sounds stilted. How about I rearrange things a little.
The request structure manages the request as a whole, holding some resources
and state on behalf of the filesystem and tracking the collection of results::
struct netfs_io_request {
enum netfs_io_origin origin;
struct inode *inode;
struct address_space *mapping;
struct netfs_group *group;
struct netfs_io_stream io_streams[];
void *netfs_priv;
void *netfs_priv2;
unsigned long long start;
unsigned long long len;
unsigned long long i_size;
unsigned int debug_id;
unsigned long flags;
...
};
Many of the fields are for internal use, but the fields shown here are of
interest to the filesystem:
* ``origin``
...
And then put the bit about wrapping the struct after the field explanation:
If the filesystem wants more private data than is afforded by this structure,
then it should wrap it and provide its own allocator.
> > + This is not permitted to return an error. In the event of failure,
> > + ``netfs_prepare_write_failed()`` must be called.
> "This method is not permitted to return an error. Instead, in the event of
> failure, ..."
Seems superfluous, but okay.
(Btw, can you put a blank line before your "> <snipped>..." to make it easier
to go through your reply?)
Thanks,
David
