linux-kernel - Re: [PATCH] netfs: Update main API document

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z_Z8KVgaH-ksEKog@archie.me>
Date: Wed, 9 Apr 2025 20:54:49 +0700
From: Bagas Sanjaya <bagasdotme@...il.com>
To: David Howells <dhowells@...hat.com>
Cc: Christian Brauner <brauner@...nel.org>,
	Paulo Alcantara <pc@...guebit.com>,
	Jeff Layton <jlayton@...nel.org>,
	Viacheslav Dubeyko <slava@...eyko.com>,
	Alex Markuze <amarkuze@...hat.com>, Timothy Day <timday@...zon.com>,
	Jonathan Corbet <corbet@....net>, netfs@...ts.linux.dev,
	linux-doc@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] netfs: Update main API document

On Wed, Apr 09, 2025 at 02:24:23PM +0100, David Howells wrote:
> Bagas Sanjaya <bagasdotme@...il.com> 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.

I prefer that above too as it is more explicit.

> 
> > > > > +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?

Oh, that's what you mean. I'm leaning toward that.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)