From 2fbec0f7156a4e8f565a4ab837fc82332b460b9c Mon Sep 17 00:00:00 2001 From: Henri Doreau Date: Thu, 19 May 2016 18:31:16 +0200 Subject: [PATCH] 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 Change-Id: I1ad63c5332ee14bdac780f1d74794ac8827186cf Reviewed-on: http://review.whamcloud.com/20331 Reviewed-by: Jinshan Xiong Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Jean-Baptiste Riaux Reviewed-by: Oleg Drokin --- lustre/llite/file.c | 133 ++++++++++++++++++++++++++++++------------ lustre/tests/swap_lock_test.c | 57 +++++++++++++++--- 2 files changed, 145 insertions(+), 45 deletions(-) diff --git a/lustre/llite/file.c b/lustre/llite/file.c index a163098..99b247a 100644 --- 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 --- 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); } -- 1.8.3.1