Whamcloud - gitweb
LU-4429 llite: fix open lock matching in ll_md_blocking_ast() 18/8718/2
authorJohn L. Hammond <john.hammond@intel.com>
Fri, 3 Jan 2014 23:31:53 +0000 (17:31 -0600)
committerOleg Drokin <oleg.drokin@intel.com>
Sat, 11 Jan 2014 03:53:24 +0000 (03:53 +0000)
In ll_md_blocking_ast() match open locks before all others, ensuring
that MDS_INODELOCK_OPEN is not cleared from bits by another open lock
with a different mode. Change the int flags parameter of
ll_md_real_close() to fmode_t fmode. Clean up verious style issues in
both functions.

Signed-off-by: John L. Hammond <john.hammond@intel.com>
Change-Id: I790cbbdbab75b25016c938b5f6340b20e09fc82e
Reviewed-on: http://review.whamcloud.com/8718
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Niu Yawei <yawei.niu@intel.com>
Reviewed-by: Jinshan Xiong <jinshan.xiong@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
lustre/llite/file.c
lustre/llite/llite_internal.h
lustre/llite/namei.c

index c805abf..f726ff2 100644 (file)
@@ -218,44 +218,47 @@ out:
         return rc;
 }
 
         return rc;
 }
 
-int ll_md_real_close(struct inode *inode, int flags)
+int ll_md_real_close(struct inode *inode, fmode_t fmode)
 {
 {
-        struct ll_inode_info *lli = ll_i2info(inode);
-        struct obd_client_handle **och_p;
-        struct obd_client_handle *och;
-        __u64 *och_usecount;
-        int rc = 0;
-        ENTRY;
+       struct ll_inode_info *lli = ll_i2info(inode);
+       struct obd_client_handle **och_p;
+       struct obd_client_handle *och;
+       __u64 *och_usecount;
+       int rc = 0;
+       ENTRY;
 
 
-        if (flags & FMODE_WRITE) {
-                och_p = &lli->lli_mds_write_och;
-                och_usecount = &lli->lli_open_fd_write_count;
-        } else if (flags & FMODE_EXEC) {
-                och_p = &lli->lli_mds_exec_och;
-                och_usecount = &lli->lli_open_fd_exec_count;
-        } else {
-                LASSERT(flags & FMODE_READ);
-                och_p = &lli->lli_mds_read_och;
-                och_usecount = &lli->lli_open_fd_read_count;
-        }
+       if (fmode & FMODE_WRITE) {
+               och_p = &lli->lli_mds_write_och;
+               och_usecount = &lli->lli_open_fd_write_count;
+       } else if (fmode & FMODE_EXEC) {
+               och_p = &lli->lli_mds_exec_och;
+               och_usecount = &lli->lli_open_fd_exec_count;
+       } else {
+               LASSERT(fmode & FMODE_READ);
+               och_p = &lli->lli_mds_read_och;
+               och_usecount = &lli->lli_open_fd_read_count;
+       }
 
        mutex_lock(&lli->lli_och_mutex);
 
        mutex_lock(&lli->lli_och_mutex);
-        if (*och_usecount) { /* There are still users of this handle, so
-                                skip freeing it. */
+       if (*och_usecount > 0) {
+               /* There are still users of this handle, so skip
+                * freeing it. */
                mutex_unlock(&lli->lli_och_mutex);
                mutex_unlock(&lli->lli_och_mutex);
-                RETURN(0);
-        }
-        och=*och_p;
-        *och_p = NULL;
+               RETURN(0);
+       }
+
+       och = *och_p;
+       *och_p = NULL;
        mutex_unlock(&lli->lli_och_mutex);
 
        mutex_unlock(&lli->lli_och_mutex);
 
-        if (och) { /* There might be a race and somebody have freed this och
-                      already */
-                rc = ll_close_inode_openhandle(ll_i2sbi(inode)->ll_md_exp,
+       if (och != NULL) {
+               /* There might be a race and this handle may already
+                * be closed. */
+               rc = ll_close_inode_openhandle(ll_i2sbi(inode)->ll_md_exp,
                                               inode, och, NULL);
                                               inode, och, NULL);
-        }
+       }
 
 
-        RETURN(rc);
+       RETURN(rc);
 }
 
 int ll_md_close(struct obd_export *md_exp, struct inode *inode,
 }
 
 int ll_md_close(struct obd_export *md_exp, struct inode *inode,
index e4482f5..da60dd3 100644 (file)
@@ -795,7 +795,7 @@ int ll_local_open(struct file *file,
 int ll_release_openhandle(struct dentry *, struct lookup_intent *);
 int ll_md_close(struct obd_export *md_exp, struct inode *inode,
                 struct file *file);
 int ll_release_openhandle(struct dentry *, struct lookup_intent *);
 int ll_md_close(struct obd_export *md_exp, struct inode *inode,
                 struct file *file);
-int ll_md_real_close(struct inode *inode, int flags);
+int ll_md_real_close(struct inode *inode, fmode_t fmode);
 void ll_ioepoch_close(struct inode *inode, struct md_op_data *op_data,
                       struct obd_client_handle **och, unsigned long flags);
 void ll_done_writing_attr(struct inode *inode, struct md_op_data *op_data);
 void ll_ioepoch_close(struct inode *inode, struct md_op_data *op_data,
                       struct obd_client_handle **och, unsigned long flags);
 void ll_done_writing_attr(struct inode *inode, struct md_op_data *op_data);
index 5051c5e..b3780e2 100644 (file)
@@ -196,114 +196,120 @@ static void ll_invalidate_negative_children(struct inode *dir)
 }
 
 int ll_md_blocking_ast(struct ldlm_lock *lock, struct ldlm_lock_desc *desc,
 }
 
 int ll_md_blocking_ast(struct ldlm_lock *lock, struct ldlm_lock_desc *desc,
-                       void *data, int flag)
+                      void *data, int flag)
 {
 {
-        int rc;
-        struct lustre_handle lockh;
-        ENTRY;
+       struct lustre_handle lockh;
+       int rc;
+       ENTRY;
 
 
-        switch (flag) {
-        case LDLM_CB_BLOCKING:
-                ldlm_lock2handle(lock, &lockh);
+       switch (flag) {
+       case LDLM_CB_BLOCKING:
+               ldlm_lock2handle(lock, &lockh);
                rc = ldlm_cli_cancel(&lockh, LCF_ASYNC);
                rc = ldlm_cli_cancel(&lockh, LCF_ASYNC);
-                if (rc < 0) {
-                        CDEBUG(D_INODE, "ldlm_cli_cancel: %d\n", rc);
-                        RETURN(rc);
-                }
-                break;
-        case LDLM_CB_CANCELING: {
+               if (rc < 0) {
+                       CDEBUG(D_INODE, "ldlm_cli_cancel: rc = %d\n", rc);
+                       RETURN(rc);
+               }
+               break;
+       case LDLM_CB_CANCELING: {
                struct inode *inode = ll_inode_from_resource_lock(lock);
                struct inode *inode = ll_inode_from_resource_lock(lock);
-                struct ll_inode_info *lli;
-                __u64 bits = lock->l_policy_data.l_inodebits.bits;
-                struct lu_fid *fid;
-                ldlm_mode_t mode = lock->l_req_mode;
+               __u64 bits = lock->l_policy_data.l_inodebits.bits;
 
                /* Inode is set to lock->l_resource->lr_lvb_inode
                 * for mdc - bug 24555 */
                LASSERT(lock->l_ast_data == NULL);
 
 
                /* Inode is set to lock->l_resource->lr_lvb_inode
                 * for mdc - bug 24555 */
                LASSERT(lock->l_ast_data == NULL);
 
-               /* Invalidate all dentries associated with this inode */
                if (inode == NULL)
                        break;
 
                if (inode == NULL)
                        break;
 
+               /* Invalidate all dentries associated with this inode */
                LASSERT(ldlm_is_canceling(lock));
 
                LASSERT(ldlm_is_canceling(lock));
 
-               if (bits & MDS_INODELOCK_XATTR)
+               if (!fid_res_name_eq(ll_inode2fid(inode),
+                                    &lock->l_resource->lr_name)) {
+                       LDLM_ERROR(lock, "data mismatch with object "DFID"(%p)",
+                                  PFID(ll_inode2fid(inode)), inode);
+                       LBUG();
+               }
+
+               if (bits & MDS_INODELOCK_XATTR) {
                        ll_xattr_cache_destroy(inode);
                        ll_xattr_cache_destroy(inode);
+                       bits &= ~MDS_INODELOCK_XATTR;
+               }
 
                /* For OPEN locks we differentiate between lock modes
                 * LCK_CR, LCK_CW, LCK_PR - bug 22891 */
 
                /* For OPEN locks we differentiate between lock modes
                 * LCK_CR, LCK_CW, LCK_PR - bug 22891 */
+               if (bits & MDS_INODELOCK_OPEN)
+                       ll_have_md_lock(inode, &bits, lock->l_req_mode);
+
+               if (bits & MDS_INODELOCK_OPEN) {
+                       fmode_t fmode;
+
+                       switch (lock->l_req_mode) {
+                       case LCK_CW:
+                               fmode = FMODE_WRITE;
+                               break;
+                       case LCK_PR:
+                               fmode = FMODE_EXEC;
+                               break;
+                       case LCK_CR:
+                               fmode = FMODE_READ;
+                               break;
+                       default:
+                               LDLM_ERROR(lock, "bad lock mode for OPEN lock");
+                               LBUG();
+                       }
+
+                       ll_md_real_close(inode, fmode);
+
+                       bits &= ~MDS_INODELOCK_OPEN;
+               }
+
                if (bits & (MDS_INODELOCK_LOOKUP | MDS_INODELOCK_UPDATE |
                            MDS_INODELOCK_LAYOUT | MDS_INODELOCK_PERM))
                if (bits & (MDS_INODELOCK_LOOKUP | MDS_INODELOCK_UPDATE |
                            MDS_INODELOCK_LAYOUT | MDS_INODELOCK_PERM))
-                        ll_have_md_lock(inode, &bits, LCK_MINMODE);
-
-                if (bits & MDS_INODELOCK_OPEN)
-                        ll_have_md_lock(inode, &bits, mode);
-
-               fid = ll_inode2fid(inode);
-               if (!fid_res_name_eq(fid, &lock->l_resource->lr_name))
-                       LDLM_ERROR(lock, "data mismatch with object "
-                                  DFID" (%p)", PFID(fid), inode);
-
-                if (bits & MDS_INODELOCK_OPEN) {
-                        int flags = 0;
-                        switch (lock->l_req_mode) {
-                        case LCK_CW:
-                                flags = FMODE_WRITE;
-                                break;
-                        case LCK_PR:
-                                flags = FMODE_EXEC;
-                                break;
-                        case LCK_CR:
-                                flags = FMODE_READ;
-                                break;
-                        default:
-                               CERROR("%s: unexpected lock mode for OPEN lock"
-                                      " %d, inode "DFID"\n",
-                                      ll_get_fsname(inode->i_sb, NULL, 0),
-                                      lock->l_req_mode,
-                                      PFID(ll_inode2fid(inode)));
-                        }
-                        ll_md_real_close(inode, flags);
-                }
+                       ll_have_md_lock(inode, &bits, LCK_MINMODE);
 
 
-               lli = ll_i2info(inode);
                if (bits & MDS_INODELOCK_LAYOUT) {
                if (bits & MDS_INODELOCK_LAYOUT) {
-                       struct cl_object_conf conf = { { 0 } };
+                       struct cl_object_conf conf = {
+                               .coc_opc = OBJECT_CONF_INVALIDATE,
+                               .coc_inode = inode,
+                       };
 
 
-                       conf.coc_opc = OBJECT_CONF_INVALIDATE;
-                       conf.coc_inode = inode;
                        rc = ll_layout_conf(inode, &conf);
                        rc = ll_layout_conf(inode, &conf);
-                       if (rc)
-                               CDEBUG(D_INODE, "invaliding layout %d.\n", rc);
+                       if (rc < 0)
+                               CDEBUG(D_INODE, "cannot invalidate layout of "
+                                      DFID": rc = %d\n",
+                                      PFID(ll_inode2fid(inode)), rc);
                }
 
                if (bits & MDS_INODELOCK_UPDATE) {
                }
 
                if (bits & MDS_INODELOCK_UPDATE) {
+                       struct ll_inode_info *lli = ll_i2info(inode);
+
                        spin_lock(&lli->lli_lock);
                        spin_lock(&lli->lli_lock);
-                        lli->lli_flags &= ~LLIF_MDS_SIZE_LOCK;
+                       lli->lli_flags &= ~LLIF_MDS_SIZE_LOCK;
                        spin_unlock(&lli->lli_lock);
                }
 
                        spin_unlock(&lli->lli_lock);
                }
 
-                if (S_ISDIR(inode->i_mode) &&
-                     (bits & MDS_INODELOCK_UPDATE)) {
+               if ((bits & MDS_INODELOCK_UPDATE) && S_ISDIR(inode->i_mode)) {
                        CDEBUG(D_INODE, "invalidating inode "DFID"\n",
                               PFID(ll_inode2fid(inode)));
                        CDEBUG(D_INODE, "invalidating inode "DFID"\n",
                               PFID(ll_inode2fid(inode)));
-                        truncate_inode_pages(inode->i_mapping, 0);
+                       truncate_inode_pages(inode->i_mapping, 0);
                        ll_invalidate_negative_children(inode);
                }
 
                        ll_invalidate_negative_children(inode);
                }
 
-               if (inode->i_sb->s_root &&
-                   inode != inode->i_sb->s_root->d_inode &&
-                   (bits & (MDS_INODELOCK_LOOKUP | MDS_INODELOCK_PERM)))
+               if ((bits & (MDS_INODELOCK_LOOKUP | MDS_INODELOCK_PERM)) &&
+                   inode->i_sb->s_root != NULL &&
+                   inode != inode->i_sb->s_root->d_inode)
                        ll_invalidate_aliases(inode);
                        ll_invalidate_aliases(inode);
-                iput(inode);
-                break;
-        }
-        default:
-                LBUG();
-        }
 
 
-        RETURN(0);
+               iput(inode);
+               break;
+       }
+       default:
+               LBUG();
+       }
+
+       RETURN(0);
 }
 
 __u32 ll_i2suppgid(struct inode *i)
 }
 
 __u32 ll_i2suppgid(struct inode *i)