diff mbox series

cifs: drop the lease for cached directories on rmdir or rename

Message ID 20221018073910.1732992-2-lsahlber@redhat.com (mailing list archive)
State New, archived
Headers show
Series cifs: drop the lease for cached directories on rmdir or rename | expand

Commit Message

Ronnie Sahlberg Oct. 18, 2022, 7:39 a.m. UTC
When we delete or rename a directory we must also drop any cached lease we have
on the directory.

Fixes: a350d6e73f5e ("cifs: enable caching of directories for which a lease
is held")
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/cached_dir.c | 21 +++++++++++++++++++++
 fs/cifs/cached_dir.h |  4 ++++
 fs/cifs/smb2inode.c  |  2 ++
 3 files changed, 27 insertions(+)

Comments

Tom Talpey Oct. 18, 2022, 2:22 p.m. UTC | #1
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?

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@redhat.com>
> ---
>   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,
ronnie sahlberg Oct. 18, 2022, 11:47 p.m. UTC | #2
On Wed, 19 Oct 2022 at 00:27, Tom Talpey <tom@talpey.com> 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@redhat.com>
> > ---
> >   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,
Tom Talpey Oct. 19, 2022, 4:02 p.m. UTC | #3
On 10/18/2022 7:47 PM, ronnie sahlberg wrote:
> On Wed, 19 Oct 2022 at 00:27, Tom Talpey <tom@talpey.com> 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)

Do both Windows and Samba servers break the lease on the renamed
directory? I'd certainly expect the parent lease to break, so that
much isn't surprising.

Tom.

>> 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@redhat.com>
>>> ---
>>>    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,
>
diff mbox series

Patch

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,