netdev - Re: [PATCH v5 37/40] netfs: Optimise away reads above the point at which there can be no data

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: <2202548.1703245791@warthog.procyon.org.uk>
Date: Fri, 22 Dec 2023 11:49:51 +0000
From: David Howells <dhowells@...hat.com>
To: Nathan Chancellor <nathan@...nel.org>,
    Anna Schumaker <Anna.Schumaker@...app.com>,
    Trond Myklebust <trond.myklebust@...merspace.com>
Cc: dhowells@...hat.com, Jeff Layton <jlayton@...nel.org>,
    Steve French <smfrench@...il.com>,
    Matthew Wilcox <willy@...radead.org>,
    Marc Dionne <marc.dionne@...istor.com>,
    Paulo Alcantara <pc@...guebit.com>,
    Shyam Prasad N <sprasad@...rosoft.com>, Tom Talpey <tom@...pey.com>,
    Dominique Martinet <asmadeus@...ewreck.org>,
    Eric Van Hensbergen <ericvh@...nel.org>,
    Ilya Dryomov <idryomov@...il.com>,
    Christian Brauner <christian@...uner.io>, linux-cachefs@...hat.com,
    linux-afs@...ts.infradead.org, linux-cifs@...r.kernel.org,
    linux-nfs@...r.kernel.org, ceph-devel@...r.kernel.org,
    v9fs@...ts.linux.dev, linux-fsdevel@...r.kernel.org,
    linux-mm@...ck.org, netdev@...r.kernel.org,
    linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 37/40] netfs: Optimise away reads above the point at which there can be no data

Nathan Chancellor <nathan@...nel.org> wrote:

> It appears that ctx->inode.i_mapping is NULL in netfs_inode_init(). This
> patch appears to cure the problem for me but I am not sure if it is
> proper or not.

I'm not sure that's the best way.  It kind of indicates that
nfs_netfs_inode_init() is not being called in the right place - it should
really be called after alloc_inode() has called inode_init_always().

However, mapping_set_release_always() makes ->release_folio() and
->invalidate_folio() always called for an inode's folios, even if PG_private
is not set - the idea being that this allows netfslib to update the
"zero_point" when a page we've written to the server gets invalidated here,
thereby requiring us to go fetch it again.

Now, NFS doesn't make use of this feature and fscache and cachefiles don't use
it directly, so we might not want to call mapping_set_release_always() for
NFS.

I'm not sure NFS can even reliably make use of it unless it's using a lease
unless it gets change notifications from the server.

So I'm thinking of applying your patch but add a comment to say why we're
doing it.  A better way, though, is to move the call to nfs_netfs_inode_init()
and give it a flag to say whether or not we want the facility.

David