smb3: retrying on failed server close

In the current implementation, CIFS close sends a close to the
server and does not check for the success of the server close.
This patch adds functionality to check for server close return
status and retries in case of an EBUSY or EAGAIN error.

This can help avoid handle leaks

Cc: stable@vger.kernel.org
Signed-off-by: Ritvik Budhiraja <rbudhiraja@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index a0017724d5239312c14644bd15d2867880337d91..13a9d7acf8f8ec151323d18d44a346e060bf0ce2 100644 (file)
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -417,6 +417,7 @@ smb2_close_cached_fid(struct kref *ref)
 {
        struct cached_fid *cfid = container_of(ref, struct cached_fid,
                                               refcount);
+       int rc;
 
        spin_lock(&cfid->cfids->cfid_list_lock);
        if (cfid->on_list) {
@@ -430,9 +431,10 @@ smb2_close_cached_fid(struct kref *ref)
        cfid->dentry = NULL;
 
        if (cfid->is_open) {
-               SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid,
+               rc = SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid,
                           cfid->fid.volatile_fid);
-               atomic_dec(&cfid->tcon->num_remote_opens);
+               if (rc != -EBUSY && rc != -EAGAIN)
+                       atomic_dec(&cfid->tcon->num_remote_opens);
        }
 
        free_cached_dir(cfid);

diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index aa6f1ecb7c0e8fc11f9b1fb830d8c0ca8071b631..d41eedbff674abb0e62e52ae6cc585aaa5d83d77 100644 (file)
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -156,6 +156,7 @@ struct workqueue_struct     *decrypt_wq;
 struct workqueue_struct        *fileinfo_put_wq;
 struct workqueue_struct        *cifsoplockd_wq;
 struct workqueue_struct        *deferredclose_wq;
+struct workqueue_struct        *serverclose_wq;
 __u32 cifs_lock_secret;
 
 /*
@@ -1888,6 +1889,13 @@ init_cifs(void)
                goto out_destroy_cifsoplockd_wq;
        }
 
+       serverclose_wq = alloc_workqueue("serverclose",
+                                          WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
+       if (!serverclose_wq) {
+               rc = -ENOMEM;
+               goto out_destroy_serverclose_wq;
+       }
+
        rc = cifs_init_inodecache();
        if (rc)
                goto out_destroy_deferredclose_wq;
@@ -1962,6 +1970,8 @@ out_destroy_decrypt_wq:
        destroy_workqueue(decrypt_wq);
 out_destroy_cifsiod_wq:
        destroy_workqueue(cifsiod_wq);
+out_destroy_serverclose_wq:
+       destroy_workqueue(serverclose_wq);
 out_clean_proc:
        cifs_proc_clean();
        return rc;
@@ -1991,6 +2001,7 @@ exit_cifs(void)
        destroy_workqueue(cifsoplockd_wq);
        destroy_workqueue(decrypt_wq);
        destroy_workqueue(fileinfo_put_wq);
+       destroy_workqueue(serverclose_wq);
        destroy_workqueue(cifsiod_wq);
        cifs_proc_clean();
 }

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 286afbe346be0aba68e78ee0a9cb0fa42e43bb58..77ca7861a2cc84fa4d980a8089c9d13f36eb7e3f 100644 (file)
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -442,10 +442,10 @@ struct smb_version_operations {
        /* set fid protocol-specific info */
        void (*set_fid)(struct cifsFileInfo *, struct cifs_fid *, __u32);
        /* close a file */
-       void (*close)(const unsigned int, struct cifs_tcon *,
+       int (*close)(const unsigned int, struct cifs_tcon *,
                      struct cifs_fid *);
        /* close a file, returning file attributes and timestamps */
-       void (*close_getattr)(const unsigned int xid, struct cifs_tcon *tcon,
+       int (*close_getattr)(const unsigned int xid, struct cifs_tcon *tcon,
                      struct cifsFileInfo *pfile_info);
        /* send a flush request to the server */
        int (*flush)(const unsigned int, struct cifs_tcon *, struct cifs_fid *);
@@ -1439,6 +1439,7 @@ struct cifsFileInfo {
        bool swapfile:1;
        bool oplock_break_cancelled:1;
        bool status_file_deleted:1; /* file has been deleted */
+       bool offload:1; /* offload final part of _put to a wq */
        unsigned int oplock_epoch; /* epoch from the lease break */
        __u32 oplock_level; /* oplock/lease level from the lease break */
        int count;
@@ -1447,6 +1448,7 @@ struct cifsFileInfo {
        struct cifs_search_info srch_inf;
        struct work_struct oplock_break; /* work for oplock breaks */
        struct work_struct put; /* work for the final part of _put */
+       struct work_struct serverclose; /* work for serverclose */
        struct delayed_work deferred;
        bool deferred_close_scheduled; /* Flag to indicate close is scheduled */
        char *symlink_target;
@@ -2103,6 +2105,7 @@ extern struct workqueue_struct *decrypt_wq;
 extern struct workqueue_struct *fileinfo_put_wq;
 extern struct workqueue_struct *cifsoplockd_wq;
 extern struct workqueue_struct *deferredclose_wq;
+extern struct workqueue_struct *serverclose_wq;
 extern __u32 cifs_lock_secret;
 
 extern mempool_t *cifs_sm_req_poolp;

diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index ab536ce8a04a5f759359d55d36a3e00bc2f96901..9be37d0fe724e90af1981c109512a64f01d86a0a 100644 (file)
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -459,6 +459,7 @@ cifs_down_write(struct rw_semaphore *sem)
 }
 
 static void cifsFileInfo_put_work(struct work_struct *work);
+void serverclose_work(struct work_struct *work);
 
 struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
                                       struct tcon_link *tlink, __u32 oplock,
@@ -505,6 +506,7 @@ struct cifsFileInfo *cifs_new_fileinfo(struct cifs_fid *fid, struct file *file,
        cfile->tlink = cifs_get_tlink(tlink);
        INIT_WORK(&cfile->oplock_break, cifs_oplock_break);
        INIT_WORK(&cfile->put, cifsFileInfo_put_work);
+       INIT_WORK(&cfile->serverclose, serverclose_work);
        INIT_DELAYED_WORK(&cfile->deferred, smb2_deferred_work_close);
        mutex_init(&cfile->fh_mutex);
        spin_lock_init(&cfile->file_info_lock);
@@ -596,6 +598,40 @@ static void cifsFileInfo_put_work(struct work_struct *work)
        cifsFileInfo_put_final(cifs_file);
 }
 
+void serverclose_work(struct work_struct *work)
+{
+       struct cifsFileInfo *cifs_file = container_of(work,
+                       struct cifsFileInfo, serverclose);
+
+       struct cifs_tcon *tcon = tlink_tcon(cifs_file->tlink);
+
+       struct TCP_Server_Info *server = tcon->ses->server;
+       int rc = 0;
+       int retries = 0;
+       int MAX_RETRIES = 4;
+
+       do {
+               if (server->ops->close_getattr)
+                       rc = server->ops->close_getattr(0, tcon, cifs_file);
+               else if (server->ops->close)
+                       rc = server->ops->close(0, tcon, &cifs_file->fid);
+
+               if (rc == -EBUSY || rc == -EAGAIN) {
+                       retries++;
+                       msleep(250);
+               }
+       } while ((rc == -EBUSY || rc == -EAGAIN) && (retries < MAX_RETRIES)
+       );
+
+       if (retries == MAX_RETRIES)
+               pr_warn("Serverclose failed %d times, giving up\n", MAX_RETRIES);
+
+       if (cifs_file->offload)
+               queue_work(fileinfo_put_wq, &cifs_file->put);
+       else
+               cifsFileInfo_put_final(cifs_file);
+}
+
 /**
  * cifsFileInfo_put - release a reference of file priv data
  *
@@ -636,10 +672,13 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file,
        struct cifs_fid fid = {};
        struct cifs_pending_open open;
        bool oplock_break_cancelled;
+       bool serverclose_offloaded = false;
 
        spin_lock(&tcon->open_file_lock);
        spin_lock(&cifsi->open_file_lock);
        spin_lock(&cifs_file->file_info_lock);
+
+       cifs_file->offload = offload;
        if (--cifs_file->count > 0) {
                spin_unlock(&cifs_file->file_info_lock);
                spin_unlock(&cifsi->open_file_lock);
@@ -681,13 +720,20 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file,
        if (!tcon->need_reconnect && !cifs_file->invalidHandle) {
                struct TCP_Server_Info *server = tcon->ses->server;
                unsigned int xid;
+               int rc = 0;
 
                xid = get_xid();
                if (server->ops->close_getattr)
-                       server->ops->close_getattr(xid, tcon, cifs_file);
+                       rc = server->ops->close_getattr(xid, tcon, cifs_file);
                else if (server->ops->close)
-                       server->ops->close(xid, tcon, &cifs_file->fid);
+                       rc = server->ops->close(xid, tcon, &cifs_file->fid);
                _free_xid(xid);
+
+               if (rc == -EBUSY || rc == -EAGAIN) {
+                       // Server close failed, hence offloading it as an async op
+                       queue_work(serverclose_wq, &cifs_file->serverclose);
+                       serverclose_offloaded = true;
+               }
        }
 
        if (oplock_break_cancelled)
@@ -695,10 +741,15 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file,
 
        cifs_del_pending_open(&open);
 
-       if (offload)
-               queue_work(fileinfo_put_wq, &cifs_file->put);
-       else
-               cifsFileInfo_put_final(cifs_file);
+       // if serverclose has been offloaded to wq (on failure), it will
+       // handle offloading put as well. If serverclose not offloaded,
+       // we need to handle offloading put here.
+       if (!serverclose_offloaded) {
+               if (offload)
+                       queue_work(fileinfo_put_wq, &cifs_file->put);
+               else
+                       cifsFileInfo_put_final(cifs_file);
+       }
 }
 
 int cifs_open(struct inode *inode, struct file *file)

diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index a9eaba8083b0d6b2745ebedb1d3d4705c0f4809d..212ec6f66ec65b15f50275803bbc38b0153c9645 100644 (file)
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -753,11 +753,11 @@ cifs_set_fid(struct cifsFileInfo *cfile, struct cifs_fid *fid, __u32 oplock)
        cinode->can_cache_brlcks = CIFS_CACHE_WRITE(cinode);
 }
 
-static void
+static int
 cifs_close_file(const unsigned int xid, struct cifs_tcon *tcon,
                struct cifs_fid *fid)
 {
-       CIFSSMBClose(xid, tcon, fid->netfid);
+       return CIFSSMBClose(xid, tcon, fid->netfid);
 }
 
 static int

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 35bf7eb315cdfc1ce92643d7950f8a8dc50a261b..87b63f6ad2e2e9e49cf8988891e8cb7745b7d8d7 100644 (file)
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -1412,14 +1412,14 @@ smb2_set_fid(struct cifsFileInfo *cfile, struct cifs_fid *fid, __u32 oplock)
        memcpy(cfile->fid.create_guid, fid->create_guid, 16);
 }
 
-static void
+static int
 smb2_close_file(const unsigned int xid, struct cifs_tcon *tcon,
                struct cifs_fid *fid)
 {
-       SMB2_close(xid, tcon, fid->persistent_fid, fid->volatile_fid);
+       return SMB2_close(xid, tcon, fid->persistent_fid, fid->volatile_fid);
 }
 
-static void
+static int
 smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon,
                   struct cifsFileInfo *cfile)
 {
@@ -1430,7 +1430,7 @@ smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon,
        rc = __SMB2_close(xid, tcon, cfile->fid.persistent_fid,
                   cfile->fid.volatile_fid, &file_inf);
        if (rc)
-               return;
+               return rc;
 
        inode = d_inode(cfile->dentry);
 
@@ -1459,6 +1459,7 @@ smb2_close_getattr(const unsigned int xid, struct cifs_tcon *tcon,
 
        /* End of file and Attributes should not have to be updated on close */
        spin_unlock(&inode->i_lock);
+       return rc;
 }
 
 static int

diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 3ea688558e6c9b96f454e29197023ec74f274e2a..c0c4933af5fc386911922b4e23c7869bdea8098b 100644 (file)
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -3628,9 +3628,9 @@ replay_again:
                        memcpy(&pbuf->network_open_info,
                               &rsp->network_open_info,
                               sizeof(pbuf->network_open_info));
+               atomic_dec(&tcon->num_remote_opens);
        }
 
-       atomic_dec(&tcon->num_remote_opens);
 close_exit:
        SMB2_close_free(&rqst);
        free_rsp_buf(resp_buftype, rsp);