Re: [PATCH 5/9] cifs: Do not access tcon->cfids->cfid directly from is_path_accessible

From: Steve French <smfrench@gmail.com>
To: Paulo Alcantara <pc@cjr.nz>
Cc: Ronnie Sahlberg <lsahlber@redhat.com>,
	linux-cifs <linux-cifs@vger.kernel.org>
Subject: Re: [PATCH 5/9] cifs: Do not access tcon->cfids->cfid directly from is_path_accessible
Date: Thu, 11 Aug 2022 20:34:18 -0500	[thread overview]
Message-ID: <CAH2r5mtq=QjOR1_zHDjZ7rgQYGUiRv6GaGcupSQFjxBPKK46GQ@mail.gmail.com> (raw)
In-Reply-To: <CAH2r5mtEVXfhMva-5pz4GmjC0OTNoSVo0Pgr0n8S=kZFWBGj1Q@mail.gmail.com>

And one more obvious thing ... this also fixes the problem where safe
caching gets unintionally turned off (deferred close, handle leases)
by users who disable actimeo (which is unsafe metadata caching).  So
setting actimeo=0 which some workloads do to make sure no stale
mtime/filesize would end up unintentionally hurting safe handle
caching (deferred close).

This timer should be controlled separately than actimeo

On Thu, Aug 11, 2022 at 8:26 PM Steve French <smfrench@gmail.com> wrote:
>
> forgot to mention the obvious - the way to tell that this would be
> helpful is simply looking at "/proc/fs/cifs/Stats" to see number of
> opens vs. other operations.   Can also see pretty easily from simple
> trace commands ("trace-cmd record -e smb3_open* smb3_close*") etc
>
> On Thu, Aug 11, 2022 at 7:57 PM Steve French <smfrench@gmail.com> wrote:
> >
> > updated patch attached
> >
> >
> > On Thu, Aug 11, 2022 at 7:56 PM Steve French <smfrench@gmail.com> wrote:
> > >
> > > Tentatively merged this patch (after rebasing it and fixing a minor
> > > checkpatch problem).  Added Paulo's RB
> > >
> > > Skipped adding patch 4 though (cifs: Make tcon contain a wrapper
> > > structure cached_fids instead of cached_fid) - let me know if problem
> > > skipping that patch in the short term
> > >
> > > On Thu, Aug 11, 2022 at 8:20 AM Paulo Alcantara <pc@cjr.nz> wrote:
> > > >
> > > > Ronnie Sahlberg <lsahlber@redhat.com> writes:
> > > >
> > > > > cfids will soon keep a list of cached fids so we should not access this
> > > > > directly from outside of cached_dir.c
> > > > >
> > > > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > > > > ---
> > > > >  fs/cifs/cached_dir.c | 10 ++++++----
> > > > >  fs/cifs/cached_dir.h |  2 +-
> > > > >  fs/cifs/readdir.c    |  4 ++--
> > > > >  fs/cifs/smb2inode.c  |  2 +-
> > > > >  fs/cifs/smb2ops.c    | 18 ++++++++++++++----
> > > > >  5 files changed, 24 insertions(+), 12 deletions(-)
> > > >
> > > > Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> > >
> > >
> > >
> > > --
> > > Thanks,
> > >
> > > Steve
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve
>
>
>
> --
> Thanks,
>
> Steve

-- 
Thanks,

Steve