| Message ID | 1274095138-3386-2-git-send-email-jlayton@redhat.com |
|---|---|
| State | New |
| Headers | show |
Seems odd to turn off metadata caching completely for hardlinked files. Wouldn't it be easier, and less invasive, to simply set cifs_i->time to zero on source and destination inodes when the client does the hardlink (so we revalidate the first time after the link is done, but otherwise treat hardlinked files the same, and continue to allow limited metadata caching on them)? On Mon, May 17, 2010 at 6:18 AM, Jeff Layton <jlayton@redhat.com> wrote: > The old cifs_revalidate logic always revalidated hardlinked inodes. > This hack allowed CIFS to pass some connectathon tests when server inode > numbers aren't used (basic test7, in particular). > > Signed-off-by: Jeff Layton <jlayton@redhat.com> > --- > fs/cifs/inode.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c > index b35cb03..f52161a 100644 > --- a/fs/cifs/inode.c > +++ b/fs/cifs/inode.c > @@ -1511,6 +1511,10 @@ cifs_inode_needs_reval(struct inode *inode) > if (time_after_eq(jiffies, cifs_i->time + HZ)) > return true; > > + /* hardlinked files get "special" treatment */ > + if (S_ISREG(inode->i_mode) && inode->i_nlink != 1) > + return true; > + > return false; > } > > -- > 1.6.6.1 > >
On Mon, 17 May 2010 10:34:03 -0500 Steve French <smfrench@gmail.com> wrote: > Seems odd to turn off metadata caching completely for hardlinked > files. Wouldn't it be easier, and less invasive, to simply set > cifs_i->time to zero on source and destination inodes when the client > does the hardlink (so we revalidate the first time after the link is > done, but otherwise treat hardlinked files the same, and continue to > allow limited metadata caching on them)? > I do agree that this makes little sense, but what you suggest won't solve the problem. cifs_hardlink already does this: d_drop(direntry); /* force new lookup from server of target */ ...and: cifsInode = CIFS_I(old_file->d_inode); ... /* if not oplocked will force revalidate to get info on source file from srv */ cifsInode->time = 0; ...the reason the test doesn't pass is that the link count doesn't drop on the inode attached to one of the dentries when you unlink the other. What probably makes the most sense is to refuse to allow hardlinking of inodes when noserverino is set, but that might be considered a regression since older kernels allowed it. In short, it's a mess but this gets the behavior back to what it was originally.
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index b35cb03..f52161a 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -1511,6 +1511,10 @@ cifs_inode_needs_reval(struct inode *inode) if (time_after_eq(jiffies, cifs_i->time + HZ)) return true; + /* hardlinked files get "special" treatment */ + if (S_ISREG(inode->i_mode) && inode->i_nlink != 1) + return true; + return false; }
The old cifs_revalidate logic always revalidated hardlinked inodes. This hack allowed CIFS to pass some connectathon tests when server inode numbers aren't used (basic test7, in particular). Signed-off-by: Jeff Layton <jlayton@redhat.com> --- fs/cifs/inode.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)