LU-8174 llite: restore fd_och when putting lease

fd_och was not restored when putting back a file lease, preventing
from getting a lease, putting it back and taking it again on a FD.

Expand regression tests accordingly.

Signed-off-by: Henri Doreau <henri.doreau@cea.fr>
Change-Id: I1ad63c5332ee14bdac780f1d74794ac8827186cf
Reviewed-on: http://review.whamcloud.com/20331
Reviewed-by: Jinshan Xiong <jinshan.xiong@intel.com>
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Jean-Baptiste Riaux <riaux.jb@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/llite/file.c b/lustre/llite/file.c
index a163098..99b247a 100644 (file)
--- a/lustre/llite/file.c
+++ b/lustre/llite/file.c
@@ -693,6 +693,95 @@ static int ll_md_blocking_lease_ast(struct ldlm_lock *lock,
 }
 
 /**
+ * When setting a lease on a file, we take ownership of the lli_mds_*_och
+ * and save it as fd->fd_och so as to force client to reopen the file even
+ * if it has an open lock in cache already.
+ */
+static int ll_lease_och_acquire(struct inode *inode, struct file *file,
+                               struct lustre_handle *old_handle)
+{
+       struct ll_inode_info *lli = ll_i2info(inode);
+       struct ll_file_data *fd = LUSTRE_FPRIVATE(file);
+       struct obd_client_handle **och_p;
+       __u64 *och_usecount;
+       int rc = 0;
+       ENTRY;
+
+       /* Get the openhandle of the file */
+       mutex_lock(&lli->lli_och_mutex);
+       if (fd->fd_lease_och != NULL)
+               GOTO(out_unlock, rc = -EBUSY);
+
+       if (fd->fd_och == NULL) {
+               if (file->f_mode & FMODE_WRITE) {
+                       LASSERT(lli->lli_mds_write_och != NULL);
+                       och_p = &lli->lli_mds_write_och;
+                       och_usecount = &lli->lli_open_fd_write_count;
+               } else {
+                       LASSERT(lli->lli_mds_read_och != NULL);
+                       och_p = &lli->lli_mds_read_och;
+                       och_usecount = &lli->lli_open_fd_read_count;
+               }
+
+               if (*och_usecount > 1)
+                       GOTO(out_unlock, rc = -EBUSY);
+
+               fd->fd_och = *och_p;
+               *och_usecount = 0;
+               *och_p = NULL;
+       }
+
+       *old_handle = fd->fd_och->och_fh;
+
+       EXIT;
+out_unlock:
+       mutex_unlock(&lli->lli_och_mutex);
+       return rc;
+}
+
+/**
+ * Release ownership on lli_mds_*_och when putting back a file lease.
+ */
+static int ll_lease_och_release(struct inode *inode, struct file *file)
+{
+       struct ll_inode_info *lli = ll_i2info(inode);
+       struct ll_file_data *fd = LUSTRE_FPRIVATE(file);
+       struct obd_client_handle **och_p;
+       struct obd_client_handle *old_och = NULL;
+       __u64 *och_usecount;
+       int rc = 0;
+       ENTRY;
+
+       mutex_lock(&lli->lli_och_mutex);
+       if (file->f_mode & FMODE_WRITE) {
+               och_p = &lli->lli_mds_write_och;
+               och_usecount = &lli->lli_open_fd_write_count;
+       } else {
+               och_p = &lli->lli_mds_read_och;
+               och_usecount = &lli->lli_open_fd_read_count;
+       }
+
+       /* The file may have been open by another process (broken lease) so
+        * *och_p is not NULL. In this case we should simply increase usecount
+        * and close fd_och.
+        */
+       if (*och_p != NULL) {
+               old_och = fd->fd_och;
+               (*och_usecount)++;
+       } else {
+               *och_p = fd->fd_och;
+               *och_usecount = 1;
+       }
+       fd->fd_och = NULL;
+       mutex_unlock(&lli->lli_och_mutex);
+
+       if (old_och != NULL)
+               rc = ll_close_inode_openhandle(inode, old_och, 0, NULL);
+
+       RETURN(rc);
+}
+
+/**
  * Acquire a lease and open the file.
  */
 static struct obd_client_handle *
@@ -713,45 +802,12 @@ ll_lease_open(struct inode *inode, struct file *file, fmode_t fmode,
                RETURN(ERR_PTR(-EINVAL));
 
        if (file != NULL) {
-               struct ll_inode_info *lli = ll_i2info(inode);
-               struct ll_file_data *fd = LUSTRE_FPRIVATE(file);
-               struct obd_client_handle **och_p;
-               __u64 *och_usecount;
-
                if (!(fmode & file->f_mode) || (file->f_mode & FMODE_EXEC))
                        RETURN(ERR_PTR(-EPERM));
 
-               /* Get the openhandle of the file */
-               rc = -EBUSY;
-               mutex_lock(&lli->lli_och_mutex);
-               if (fd->fd_lease_och != NULL) {
-                       mutex_unlock(&lli->lli_och_mutex);
-                       RETURN(ERR_PTR(rc));
-               }
-
-               if (fd->fd_och == NULL) {
-                       if (file->f_mode & FMODE_WRITE) {
-                               LASSERT(lli->lli_mds_write_och != NULL);
-                               och_p = &lli->lli_mds_write_och;
-                               och_usecount = &lli->lli_open_fd_write_count;
-                       } else {
-                               LASSERT(lli->lli_mds_read_och != NULL);
-                               och_p = &lli->lli_mds_read_och;
-                               och_usecount = &lli->lli_open_fd_read_count;
-                       }
-                       if (*och_usecount == 1) {
-                               fd->fd_och = *och_p;
-                               *och_p = NULL;
-                               *och_usecount = 0;
-                               rc = 0;
-                       }
-               }
-               mutex_unlock(&lli->lli_och_mutex);
-               if (rc < 0) /* more than 1 opener */
+               rc = ll_lease_och_acquire(inode, file, &old_handle);
+               if (rc)
                        RETURN(ERR_PTR(rc));
-
-               LASSERT(fd->fd_och != NULL);
-               old_handle = fd->fd_och->och_fh;
        }
 
        OBD_ALLOC_PTR(och);
@@ -914,10 +970,11 @@ static int ll_lease_close(struct obd_client_handle *och, struct inode *inode,
        }
 
        CDEBUG(D_INODE, "lease for "DFID" broken? %d\n",
-               PFID(&ll_i2info(inode)->lli_fid), cancelled);
+              PFID(&ll_i2info(inode)->lli_fid), cancelled);
 
        if (!cancelled)
                ldlm_cli_cancel(&och->och_lease_handle, 0);
+
        if (lease_broken != NULL)
                *lease_broken = cancelled;
 
@@ -2532,6 +2589,10 @@ out:
                        if (rc < 0)
                                RETURN(rc);
 
+                       rc = ll_lease_och_release(inode, file);
+                       if (rc < 0)
+                               RETURN(rc);
+
                        if (lease_broken)
                                fmode = 0;
 

diff --git a/lustre/tests/swap_lock_test.c b/lustre/tests/swap_lock_test.c
index fed55e7..5ec64e5 100644 (file)
--- a/lustre/tests/swap_lock_test.c
+++ b/lustre/tests/swap_lock_test.c
@@ -437,11 +437,8 @@ static void test15(void)
        ASSERTF(rc == 0,
                "invalid lease type on '%s': %s", filename, strerror(-rc));
 
-#if 0
-       /* BUG! This returns EBUSY but there's no lease. */
        rc = llapi_lease_get(fd, LL_LEASE_RDLCK);
        ASSERTF(rc == 0, "cannot get lease '%s': %s", filename, strerror(-rc));
-#endif
 
        close(fd);
 
@@ -464,11 +461,8 @@ static void test15(void)
        ASSERTF(rc == 0,
                "invalid lease type on '%s': %s", filename, strerror(-rc));
 
-#if 0
-       /* BUG! This returns EBUSY but there's no lease. */
        rc = llapi_lease_get(fd, LL_LEASE_WRLCK);
        ASSERTF(rc == 0, "cannot get lease '%s': %s", filename, strerror(-rc));
-#endif
 
        close(fd);
 
@@ -477,8 +471,6 @@ static void test15(void)
        ASSERTF(fd >= 0, "open failed for '%s': %s", filename, strerror(errno));
 
        for (i = 0; i < 1000; i++) {
-#if 0
-               /* BUG! Same as above */
                rc = llapi_lease_get(fd, LL_LEASE_WRLCK);
                ASSERTF(rc == 0, "cannot get lease '%s': %s",
                        filename, strerror(-rc));
@@ -496,11 +488,58 @@ static void test15(void)
                ASSERTF(rc == LL_LEASE_RDLCK,
                        "was not able to put back lease '%s': %s",
                        filename, strerror(-rc));
-#endif
        }
 
        close(fd);
 
+       /* Get a write lease, release and take a read one */
+       fd = open(filename, O_RDWR);
+       ASSERTF(fd >= 0, "open failed for '%s': %s", filename, strerror(errno));
+
+       rc = llapi_lease_get(fd, LL_LEASE_WRLCK);
+       ASSERTF(rc == 0, "cannot get lease '%s': %s", filename, strerror(-rc));
+
+       rc = llapi_lease_check(fd);
+       ASSERTF(rc == LL_LEASE_WRLCK,
+               "invalid lease type on '%s': %s", filename, strerror(-rc));
+
+       rc = llapi_lease_put(fd);
+       ASSERTF(rc == LL_LEASE_WRLCK, "was not able to put back lease '%s': %s",
+               filename, strerror(-rc));
+
+       rc = llapi_lease_check(fd);
+       ASSERTF(rc == 0,
+               "invalid lease type on '%s': %s", filename, strerror(-rc));
+
+       rc = llapi_lease_get(fd, LL_LEASE_RDLCK);
+       ASSERTF(rc == 0, "cannot get lease '%s': %s", filename, strerror(-rc));
+
+       close(fd);
+
+       /* Get a read lease, release and take a write one */
+       fd = open(filename, O_RDWR);
+       ASSERTF(fd >= 0, "open failed for '%s': %s", filename, strerror(errno));
+
+       rc = llapi_lease_get(fd, LL_LEASE_RDLCK);
+       ASSERTF(rc == 0, "cannot get lease '%s': %s", filename, strerror(-rc));
+
+       rc = llapi_lease_check(fd);
+       ASSERTF(rc == LL_LEASE_RDLCK,
+               "invalid lease type on '%s': %s", filename, strerror(-rc));
+
+       rc = llapi_lease_put(fd);
+       ASSERTF(rc == LL_LEASE_RDLCK, "was not able to put back lease '%s': %s",
+               filename, strerror(-rc));
+
+       rc = llapi_lease_check(fd);
+       ASSERTF(rc == 0,
+               "invalid lease type on '%s': %s", filename, strerror(-rc));
+
+       rc = llapi_lease_get(fd, LL_LEASE_WRLCK);
+       ASSERTF(rc == 0, "cannot get lease '%s': %s", filename, strerror(-rc));
+
+       close(fd);
+
        free(filename);
 }