This does look wrong since it would affect use of acdirmax/acregmax.
Would like to dig deeper into the failure and see if more intuitive
way to fix it.
It also seems like the if ... nohandlecache check applies more to
whether it is worth calling open_cached_dir_by_dentry ... not to
whether we should leverage acdirmax/acregmax cached dentries
On Sat, Aug 31, 2024 at 11:36 PM Steve French <smfrench@xxxxxxxxx> wrote:
>
> tentatively merged to cifs-2.6.git for-next pending testing and
> additional review
>
> On Fri, Aug 30, 2024 at 7:10 PM Enzo Matsumiya <ematsumiya@xxxxxxx> wrote:
> >
> > Some operations return -EEXIST for a non-existing dir/file because of
> > cached attributes.
> >
> > Fix this by forcing dentry revalidation when nohandlecache is set.
> >
> > Bug/reproducer:
> > Azure Files share, attribute caching timeout is 30s (as
> > suggested by Azure), 2 clients mounting the same share.
> >
> > tcon->nohandlecache is set by cifs_get_tcon() because no directory
> > leasing capability is offered.
> >
> > # client 1 and 2
> > $ mount.cifs -o ...,actimeo=30 //server/share /mnt
> > $ cd /mnt
> >
> > # client 1
> > $ mkdir dir1
> >
> > # client 2
> > $ ls
> > dir1
> >
> > # client 1
> > $ mv dir1 dir2
> >
> > # client 2
> > $ mkdir dir1
> > mkdir: cannot create directory ‘dir1’: File exists
> > $ ls
> > dir2
> > $ mkdir dir1
> > mkdir: cannot create directory ‘dir1’: File exists
> >
> > "mkdir dir1" eventually works after 30s (when attribute cache
> > expired).
> >
> > The same behaviour can be observed with files if using some
> > non-overwritting operation (e.g. "rm -i").
> >
> > Signed-off-by: Enzo Matsumiya <ematsumiya@xxxxxxx>
> > Tested-by: Henrique Carvalho <henrique.carvalho@xxxxxxxx>
> > ---
> > fs/smb/client/inode.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> > index dd0afa23734c..5f9c5525385f 100644
> > --- a/fs/smb/client/inode.c
> > +++ b/fs/smb/client/inode.c
> > @@ -2427,6 +2427,9 @@ cifs_dentry_needs_reval(struct dentry *dentry)
> > if (!lookupCacheEnabled)
> > return true;
> >
> > + if (tcon->nohandlecache)
> > + return true;
> > +
> > if (!open_cached_dir_by_dentry(tcon, dentry->d_parent, &cfid)) {
> > spin_lock(&cfid->fid_lock);
> > if (cfid->time && cifs_i->time > cfid->time) {
> > --
> > 2.46.0
> >
>
>
> --
> Thanks,
>
> Steve
--
Thanks,
Steve
