[linux-cifs-client] Re: hardlink patch series

Steve French smfrench at gmail.com
Thu Jun 25 22:05:12 GMT 2009


On Thu, Jun 25, 2009 at 4:43 PM, Jeff Layton<jlayton at redhat.com> wrote:
> On Thu, 25 Jun 2009 15:55:09 -0500
> Steve French <smfrench at gmail.com> wrote:
>
>> I have been reviewing Jeff's series of (large) patches for hardlink
>> fixup.   The only problem I have noticed so far is the rename error
>> path - we have cached inode information which could have stale inode
>> numbers for source and target (they could be up to 1 second old) so we
>> may incorrectly think that source and target are different inodes in
>> the EEXIST case (Jeff removed the logic which checks if source/target
>> inode numbers are the same on EEXIST).  It may not matter to current
>> Samba (more recent POSIX extensions) but still trying to check that
>> ...
> I don't know that there's much we can do about that. This sort of thing
> is always racy with a netfs like CIFS or NFS. The VFS does do a lookup
> prior to calling cifs_rename. One thing we could do is force a
> revalidation when LOOKUP_RENAME_TARGET is set (for the target dentry),
> but even that won't be sufficient to eliminate races 100%, and that
> doesn't help the case where the inode number of the source has changed.
>
> IMO, that's not a case to worry overly much about. I doubt it happens
> much in practice and we can consider how best to clean it up later.

The case of source and target recently hardlinked (even if revalidate
is called prior to rename, revalidate is a noop when info is cached
for less than a second) makes sense to continue to check since it is
possible to current Samba.  Since it only costs us an extra lookup
when we get an error it doesn't hurt performance to continue to do the
check - an easier way might be to simply force revalidate (set the
cifs inode time to zero).

It would be nice if Samba (at least) didn't do this, but it is
certainly possible for an app to do a create, hardlink and rename in 1
second, and I think it makes sense to continue to check for that.

-- 
Thanks,

Steve


More information about the linux-cifs-client mailing list