Re: [PATCH] netfs: Update main API document

[David Howells]

Bagas Sanjaya <bagasdotme@xxxxxxxxx> wrote:

> > > > +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.
> 
> That one sounds better to me.

I think I like this better:

	Further, if one or more contiguous cache-read subrequests fail, the
	library will pass them to the filesystem to perform instead,
	renegotiating and retiling them as necessary to fit with the
	filesystem's parameters rather than those of the cache.

> > > > +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?
> 
> I mean "However, this needs managing the inode (inode management)". Is it
> correct to you?

Um.  "However, this needs managing the inode (inode management)" isn't valid
English and "(inode management)" is superfluous with "managing the inode" also
in the sentence.

How about:

	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 pinning
	needs careful management.  To manage the pinning, the following sequence
	occurs:

	 1) An inode state flag ``I_PINNING_NETFS_WB`` is set by netfslib when the
	    pinning begins (when a folio is dirtied, for example) if the cache is
	    active to stop the cache structures from being discarded and the cache
	    space from being culled.  This also prevents re-getting of cache resources
	    if the flag is already set.

	 2) This flag then cleared inside the inode lock during inode writeback in the
	    VM - and the fact that it was set is transferred to ``->unpinned_netfs_wb``
	    in ``struct writeback_control``.

	 3) If ``->unpinned_netfs_wb`` is now set, the write_inode procedure is forced.

	 4) The filesystem's ``->write_inode()`` function is invoked to do the cleanup.

	 5) The filesystem invokes netfs to do its cleanup.

	To do the cleanup, netfslib provides a function to do the resource unpinning::

		int netfs_unpin_writeback(struct inode *inode, struct writeback_control *wbc);

	If the filesystem doesn't need to do anything else, this may be set as a its
	``.write_inode`` method.

	Further, if an inode is deleted, the filesystem's write_inode method may not
	get called, so::

		void netfs_clear_inode_writeback(struct inode *inode, const void *aux);

	must be called from ``->evict_inode()`` *before* ``clear_inode()`` is called.


instead?

Thanks,
David