Received-SPF: pass (google.com: domain of linux-kernel+bounces-147269-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
	Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
	Kingdom.
	Registered in England and Wales under Company Registration No. 3798903
From: David Howells <dhowells@redhat.com>
In-Reply-To: <14e66691a65e3d05d3d8d50e74dfb366@manguebit.com>
References: <14e66691a65e3d05d3d8d50e74dfb366@manguebit.com> <3756406.1712244064@warthog.procyon.org.uk>
To: Paulo Alcantara <pc@manguebit.com>
Cc: dhowells@redhat.com, Steve French <sfrench@samba.org>,
    Shyam Prasad N <sprasad@microsoft.com>, linux-cifs@vger.kernel.org,
    linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] cifs: Fix reacquisition of volume cookie on still-live connection
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2713339.1713286722.1@warthog.procyon.org.uk>
Date: Tue, 16 Apr 2024 17:58:42 +0100
Message-ID: <2713340.1713286722@warthog.procyon.org.uk>

Paulo Alcantara <pc@manguebit.com> wrote:

> Can't we just move the cookie acquisition to cifs_get_tcon() before it
> gets added to list @ses->tcon_list.  This way we'll guarantee that the
> cookie is set only once for the new tcon.

cifs_get_tcon() is used from more than one place and I'm not sure the second
place (__cifs_construct_tcon()) actually wants a cookie.  I'm not sure what
that path is for.  Could all the (re)setting up being done in
cifs_mount_get_tcon() be pushed back into cifs_get_tcon()?

> Besides, do we want to share a tcon with two different superblocks that
> have 'fsc' and 'nofsc', respectively?  If not, it would be better to fix
> match_tcon() as well to handle such case.

Maybe?  What does a tcon *actually* represent?  I note that in
cifs_match_super(), it's not the only criterion matched upon - so you can, at
least in apparent theory, get different superblocks for the same tcon anyway.

This suggests that the tcon might not be the best place for the fscache volume
cookie as you can have multiple inodes wishing to use the same file cookie if
there are multiple mounts mounting the same tcon but, say, with different
mount parameters.

I'm not sure what the right way around this is.  The root of the problem is
coherency management.  If we make a change to an inode on one mounted
superblock and this bounces a change notification over to the server that then
pokes an inode in another mounted superblock on the same machine and causes it
to be invalidated, you lose your local cache if both inodes refer to the same
fscache cookie.

Remember: fscache does not do this for you!  It's just a facility by which
which data can be stored and retrieved.  The netfs is responsible for telling
it when to invalidate and handling coherency.

That said, it might be possible to time-share a cookie on cifs with leases,
but the local superblocks would have to know about each other - in which case,
why are they separate superblocks?

David