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: <1657441.1744189529@warthog.procyon.org.uk>
Date: Wed, 09 Apr 2025 10:05:29 +0100
From: David Howells <dhowells@...hat.com>
To: Bagas Sanjaya <bagasdotme@...il.com>
Cc: dhowells@...hat.com, 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

Bagas Sanjaya <bagasdotme@...il.com> 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