On Wed, 19 Oct 2022 at 00:27, Tom Talpey <tom@xxxxxxxxxx> wrote:
>
> On 10/18/2022 3:39 AM, Ronnie Sahlberg wrote:
> > When we delete or rename a directory we must also drop any cached lease we have
> > on the directory.
>
> Just curious, why drop the lease on rename? I guess this is related
> to setting ReplaceIfExists, but that would apply to a lease on the
> existing (replaced) directory, which would then become deleted?
You might be right in that, but I think the lease will be broken by
the rename anyway
so by deliberately closing it saves half a roundtrip for the rename to complete.
(you get a lease break both for the directory you rename and also for
the parent directory as far as I can see in traces)
>
> I'm probably undercaffeinated, if not.
>
> Tom.
>
> > Fixes: a350d6e73f5e ("cifs: enable caching of directories for which a lease
> > is held")
> > Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> > ---
> > fs/cifs/cached_dir.c | 21 +++++++++++++++++++++
> > fs/cifs/cached_dir.h | 4 ++++
> > fs/cifs/smb2inode.c | 2 ++
> > 3 files changed, 27 insertions(+)
> >
> > diff --git a/fs/cifs/cached_dir.c b/fs/cifs/cached_dir.c
> > index ffc924296e59..6e689c4c8d1b 100644
> > --- a/fs/cifs/cached_dir.c
> > +++ b/fs/cifs/cached_dir.c
> > @@ -340,6 +340,27 @@ smb2_close_cached_fid(struct kref *ref)
> > free_cached_dir(cfid);
> > }
> >
> > +void drop_cached_dir_by_name(const unsigned int xid, struct cifs_tcon *tcon,
> > + const char *name, struct cifs_sb_info *cifs_sb)
> > +{
> > + struct cached_fid *cfid = NULL;
> > + int rc;
> > +
> > + rc = open_cached_dir(xid, tcon, name, cifs_sb, true, &cfid);
> > + if (rc) {
> > + cifs_dbg(FYI, "no cached dir found for rmdir(%s)\n", name);
> > + return;
> > + }
> > + spin_lock(&cfid->cfids->cfid_list_lock);
> > + if (cfid->has_lease) {
> > + cfid->has_lease = false;
> > + kref_put(&cfid->refcount, smb2_close_cached_fid);
> > + }
> > + spin_unlock(&cfid->cfids->cfid_list_lock);
> > + close_cached_dir(cfid);
> > +}
> > +
> > +
> > void close_cached_dir(struct cached_fid *cfid)
> > {
> > kref_put(&cfid->refcount, smb2_close_cached_fid);
> > diff --git a/fs/cifs/cached_dir.h b/fs/cifs/cached_dir.h
> > index e536304ca2ce..2f4e764c9ca9 100644
> > --- a/fs/cifs/cached_dir.h
> > +++ b/fs/cifs/cached_dir.h
> > @@ -69,6 +69,10 @@ extern int open_cached_dir_by_dentry(struct cifs_tcon *tcon,
> > struct dentry *dentry,
> > struct cached_fid **cfid);
> > extern void close_cached_dir(struct cached_fid *cfid);
> > +extern void drop_cached_dir_by_name(const unsigned int xid,
> > + struct cifs_tcon *tcon,
> > + const char *name,
> > + struct cifs_sb_info *cifs_sb);
> > extern void close_all_cached_dirs(struct cifs_sb_info *cifs_sb);
> > extern void invalidate_all_cached_dirs(struct cifs_tcon *tcon);
> > extern int cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16]);
> > diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
> > index a6640e6ea58b..68e08c85fbb8 100644
> > --- a/fs/cifs/smb2inode.c
> > +++ b/fs/cifs/smb2inode.c
> > @@ -655,6 +655,7 @@ int
> > smb2_rmdir(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
> > struct cifs_sb_info *cifs_sb)
> > {
> > + drop_cached_dir_by_name(xid, tcon, name, cifs_sb);
> > return smb2_compound_op(xid, tcon, cifs_sb, name, DELETE, FILE_OPEN,
> > CREATE_NOT_FILE, ACL_NO_MODE,
> > NULL, SMB2_OP_RMDIR, NULL, NULL, NULL);
> > @@ -698,6 +699,7 @@ smb2_rename_path(const unsigned int xid, struct cifs_tcon *tcon,
> > {
> > struct cifsFileInfo *cfile;
> >
> > + drop_cached_dir_by_name(xid, tcon, from_name, cifs_sb);
> > cifs_get_writable_path(tcon, from_name, FIND_WR_WITH_DELETE, &cfile);
> >
> > return smb2_set_path_attr(xid, tcon, from_name, to_name,
