cifs: add lease tracking to the cached root fid

Message ID 20180507231530.28478-2-lsahlber@redhat.com
State New
Headers show
Series
  • cifs: add lease tracking to the cached root fid
Related show

Commit Message

Ronnie Sahlberg May 7, 2018, 11:15 p.m.
Use a read lease for the cached root fid so that we can detect
when the fid changes and thus we should not longer keep it cached.

We detect the lease break in the context of the demultiplex loop
when we receive an unsolicited lease break that matches the lease key
for the root fid and setting a flag to indicate the fid is no longer
valid.

On next call to open_shroot() we forcibly close the fid and eventually
re-open it, getting a new lease.

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/cifsglob.h |  1 +
 fs/cifs/smb2misc.c |  9 +++++++++
 fs/cifs/smb2ops.c  | 10 +++++++++-
 3 files changed, 19 insertions(+), 1 deletion(-)

Comments

Pavel Shilovsky May 8, 2018, 10:02 p.m. | #1
2018-05-07 16:15 GMT-07:00 Ronnie Sahlberg <lsahlber@redhat.com>:
> Use a read lease for the cached root fid so that we can detect
> when the fid changes and thus we should not longer keep it cached.
>
> We detect the lease break in the context of the demultiplex loop
> when we receive an unsolicited lease break that matches the lease key
> for the root fid and setting a flag to indicate the fid is no longer
> valid.

The handle is not broken but the lease is. Let's have a flag
indicating if we have a lease or not: e.g. "tcon->root_handle_lease".

Also it means that we are not going to acknowledge a lease break if we
don't touch a share root for a while. I would suggest to handle a
lease break for the root handle the same way we do it for other
handles.

In general, I think we need to do a proper refcounting for the root
handle (see my replay to "smb3: prevent redundant opens on root"
thread). We can use cifsFileInfo structure to hold a root handle with
a reference inside tcon. We hold it for 15 sec after the last usage
and then dereference it (the special root handle cleanup thread will
do that by timer). If the root handle doesn't have a lease at that
time, we can safely close the handle.

Every time we dereferencing the handle with a handle lease, instead of
closing it we move it to a separate list of cached handles with
leases. For this list there will be another cleanup thread that closes
handles that remain in the list untouched for 2-3 minutes. If we
receive a lease break for a cached handle, we just remove it from the
list and close it. Such a generic infrastructure can be further
adapted for caching other directory or file handles.

>
> On next call to open_shroot() we forcibly close the fid and eventually
> re-open it, getting a new lease.
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/cifsglob.h |  1 +
>  fs/cifs/smb2misc.c |  9 +++++++++
>  fs/cifs/smb2ops.c  | 10 +++++++++-
>  3 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 4f674b75bbc8..b9ec91ef3cba 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -982,6 +982,7 @@ struct cifs_tcon {
>  #endif
>         struct list_head pending_opens; /* list of incomplete opens */
>         bool valid_root_fid:1;  /* Do we have a useable root fid */
> +       bool broken_root_fid:1; /* Has the lease been broken? */
>         struct mutex prfid_mutex; /* prevents reopen race after dead ses*/
>         struct cifs_fid *prfid; /* handle to the directory at top of share */
>         /* BB add field for back pointer to sb struct(s)? */
> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
> index f7f3ad760401..33b82de50540 100644
> --- a/fs/cifs/smb2misc.c
> +++ b/fs/cifs/smb2misc.c
> @@ -608,6 +608,15 @@ smb2_is_valid_lease_break(char *buffer)
>                                         spin_unlock(&cifs_tcp_ses_lock);
>                                         return true;
>                                 }
> +                               if (tcon->valid_root_fid &&
> +                                   !memcmp(rsp->LeaseKey,
> +                                           tcon->prfid->lease_key,
> +                                           SMB2_LEASE_KEY_SIZE)) {
> +                                 tcon->broken_root_fid = true;
> +                                 spin_unlock(&tcon->open_file_lock);
> +                                 spin_unlock(&cifs_tcp_ses_lock);
> +                                 return true;
> +                               }
>                                 spin_unlock(&tcon->open_file_lock);
>                         }
>                 }
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index ceaa358723f0..c6edc99faac7 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -331,9 +331,16 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, struct cifs_fid *pfid)
>         struct cifs_open_parms oparams;
>         int rc;
>         __le16 srch_path = 0; /* Null - since an open of top of share */
> -       u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
> +       u8 oplock = SMB2_OPLOCK_LEVEL_II;
>
>         mutex_lock(&tcon->prfid_mutex);
> +       if (tcon->valid_root_fid && tcon->broken_root_fid) {
> +               cifs_dbg(FYI, "clear cached root file handle\n");
> +               SMB2_close(0, tcon, pfid->persistent_fid, pfid->volatile_fid);
> +               tcon->valid_root_fid = false;
> +               tcon->broken_root_fid = false;
> +       }
> +
>         if (tcon->valid_root_fid) {
>                 cifs_dbg(FYI, "found a cached root file handle\n");
>                 memcpy(pfid, tcon->prfid, sizeof(struct cifs_fid));
> @@ -352,6 +359,7 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, struct cifs_fid *pfid)
>         if (rc == 0) {
>                 memcpy(tcon->prfid, pfid, sizeof(struct cifs_fid));
>                 tcon->valid_root_fid = true;
> +               tcon->broken_root_fid = false;
>         }
>         mutex_unlock(&tcon->prfid_mutex);
>         return rc;
> --
> 2.13.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
Best regards,
Pavel Shilovsky
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 4f674b75bbc8..b9ec91ef3cba 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -982,6 +982,7 @@  struct cifs_tcon {
 #endif
 	struct list_head pending_opens;	/* list of incomplete opens */
 	bool valid_root_fid:1;	/* Do we have a useable root fid */
+	bool broken_root_fid:1;	/* Has the lease been broken? */
 	struct mutex prfid_mutex; /* prevents reopen race after dead ses*/
 	struct cifs_fid *prfid;	/* handle to the directory at top of share */
 	/* BB add field for back pointer to sb struct(s)? */
diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
index f7f3ad760401..33b82de50540 100644
--- a/fs/cifs/smb2misc.c
+++ b/fs/cifs/smb2misc.c
@@ -608,6 +608,15 @@  smb2_is_valid_lease_break(char *buffer)
 					spin_unlock(&cifs_tcp_ses_lock);
 					return true;
 				}
+				if (tcon->valid_root_fid &&
+				    !memcmp(rsp->LeaseKey,
+					    tcon->prfid->lease_key,
+					    SMB2_LEASE_KEY_SIZE)) {
+				  tcon->broken_root_fid = true;
+				  spin_unlock(&tcon->open_file_lock);
+				  spin_unlock(&cifs_tcp_ses_lock);
+				  return true;
+				}
 				spin_unlock(&tcon->open_file_lock);
 			}
 		}
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index ceaa358723f0..c6edc99faac7 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -331,9 +331,16 @@  int open_shroot(unsigned int xid, struct cifs_tcon *tcon, struct cifs_fid *pfid)
 	struct cifs_open_parms oparams;
 	int rc;
 	__le16 srch_path = 0; /* Null - since an open of top of share */
-	u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
+	u8 oplock = SMB2_OPLOCK_LEVEL_II;
 
 	mutex_lock(&tcon->prfid_mutex);
+	if (tcon->valid_root_fid && tcon->broken_root_fid) {
+		cifs_dbg(FYI, "clear cached root file handle\n");
+		SMB2_close(0, tcon, pfid->persistent_fid, pfid->volatile_fid);
+		tcon->valid_root_fid = false;
+		tcon->broken_root_fid = false;
+	}
+
 	if (tcon->valid_root_fid) {
 		cifs_dbg(FYI, "found a cached root file handle\n");
 		memcpy(pfid, tcon->prfid, sizeof(struct cifs_fid));
@@ -352,6 +359,7 @@  int open_shroot(unsigned int xid, struct cifs_tcon *tcon, struct cifs_fid *pfid)
 	if (rc == 0) {
 		memcpy(tcon->prfid, pfid, sizeof(struct cifs_fid));
 		tcon->valid_root_fid = true;
+		tcon->broken_root_fid = false;
 	}
 	mutex_unlock(&tcon->prfid_mutex);
 	return rc;