David Howells <dhowells@xxxxxxxxxx> writes:
> Paulo Alcantara <pc@xxxxxxxxxxxxx> 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.
__cifs_construct_tcon() is used for creating sessions and tcons under
multiuser mounts. Whenever an user accesses a multiuser mount and the
client can't find a credential for it, a new session and tcon will be
created for the user accessing the mount -- new accesses from same user
will end up reusing the created session and tcon.
And yes, I don't think we'll need a cookie for those tcons as the client
seems to get the fscache cookie from master tcon (the one created from
mount credentials).
> Could all the (re)setting up being done in cifs_mount_get_tcon() be
> pushed back into cifs_get_tcon()?
AFAICT, yes. I'd need to look into it to make sure that's safe.
>> 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.
tcon simply represents a tree connected SMB share. It can be either an
IPC share (\\srv\IPC$) or the actual share (\\srv\share) we're accessing
the files from.
Consider the following example where a tcon is reused from different
CIFS superblocks:
mount.cifs //srv/share /mnt/1 -o ${opts} # new super, new tcon
mount.cifs //srv/share/dir /mnt/2 -o ${opts} # new super, reused tcon
So, /mnt/1/dir/foo and /mnt/2/foo will lead to different inodes.
The two mounts are accessing the same tcon (\\srv\share) but the new
superblock was created because the prefix path "\dir" didn't match in
cifs_match_super(). Trust me, that's a very common scenario.
> 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.
We're not supposed to allow mounts with different parameters reusing
servers, sessions or tcons, so that should be no issue.
> 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.
Yes, that could be a problem. Perhaps placing the fscache cookie in the
superblock would be a way to go, so we can handle the different fscache
cookies for the superblocks that contain different prefix paths and
access same tcon.
> 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.
ACK.
> 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?
See above why they could be separate superblocks.
