Whamcloud - gitweb
LU-1876 hsm: Avoid deadlock on i_mutex and layout change
authorJinshan Xiong <jinshan.xiong@intel.com>
Thu, 24 Jan 2013 21:28:21 +0000 (14:28 -0700)
committerOleg Drokin <green@whamcloud.com>
Thu, 31 Jan 2013 16:17:16 +0000 (11:17 -0500)
The deadlock is as follows:
Proc 1 (write): start an IO -> try to acquire i_mutex in
            generic_file_aio_write()
Proc 2 (setattr): Acquire i_mutex in ll_setattr_raw -> initialise IO
            -> ll_layout_refresh() -> wait for IO above.

It's actually not necessary to acquire i_mutex in ll_setattr_raw()
until setattr really starts.

Signed-off-by: Jinshan Xiong <jinshan.xiong@intel.com>
Change-Id: I1988d67eab8bcac2ee63b9ec0ecab06d0b7cfc66
Reviewed-on: http://review.whamcloud.com/5159
Tested-by: Hudson
Reviewed-by: jacques-Charles Lafoucriere <jacques-charles.lafoucriere@cea.fr>
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Johann Lombardi <johann.lombardi@intel.com>
lustre/include/lclient.h
lustre/llite/llite_lib.c
lustre/llite/vvp_io.c

index 8124254..4a2631b 100644 (file)
@@ -99,7 +99,6 @@ struct ccc_io {
 
         union {
                 struct {
-                        int                        cui_locks_released;
                         enum ccc_setattr_lock_type cui_local_lock;
                 } setattr;
         } u;
index a2c6370..a7cb341 100644 (file)
@@ -1475,9 +1475,6 @@ int ll_setattr_raw(struct dentry *dentry, struct iattr *attr)
                        inode_dio_write_done(inode);
                mutex_unlock(&inode->i_mutex);
                down_write(&lli->lli_trunc_sem);
-               mutex_lock(&inode->i_mutex);
-               if (ia_valid & ATTR_SIZE)
-                       inode_dio_wait(inode);
        }
 
        /* We need a steady stripe configuration for setattr to avoid
@@ -1535,13 +1532,17 @@ out:
                 }
                 ll_finish_md_op_data(op_data);
         }
-        if (!S_ISDIR(inode->i_mode))
+       if (!S_ISDIR(inode->i_mode)) {
                up_write(&lli->lli_trunc_sem);
+               mutex_lock(&inode->i_mutex);
+               if (ia_valid & ATTR_SIZE)
+                       inode_dio_wait(inode);
+       }
 
-        ll_stats_ops_tally(ll_i2sbi(inode), (ia_valid & ATTR_SIZE) ?
-                           LPROC_LL_TRUNC : LPROC_LL_SETATTR, 1);
+       ll_stats_ops_tally(ll_i2sbi(inode), (ia_valid & ATTR_SIZE) ?
+                       LPROC_LL_TRUNC : LPROC_LL_SETATTR, 1);
 
-        return rc;
+       return rc;
 }
 
 int ll_setattr(struct dentry *de, struct iattr *attr)
index 401c4ad..18e30b6 100644 (file)
@@ -289,21 +289,6 @@ static int vvp_io_write_lock(const struct lu_env *env,
 static int vvp_io_setattr_iter_init(const struct lu_env *env,
                                    const struct cl_io_slice *ios)
 {
-       struct ccc_io *cio   = ccc_env_io(env);
-       struct inode  *inode = ccc_object_inode(ios->cis_obj);
-
-       /*
-        * We really need to get our PW lock before we change inode->i_size.
-        * If we don't we can race with other i_size updaters on our node,
-        * like ll_file_read.  We can also race with i_size propogation to
-        * other nodes through dirtying and writeback of final cached pages.
-        * This last one is especially bad for racing o_append users on other
-        * nodes.
-        */
-       if (cl_io_is_trunc(ios->cis_io))
-               inode_dio_write_done(inode);
-       mutex_unlock(&inode->i_mutex);
-       cio->u.setattr.cui_locks_released = 1;
        return 0;
 }
 
@@ -386,15 +371,10 @@ static int vvp_io_setattr_time(const struct lu_env *env,
 static int vvp_io_setattr_start(const struct lu_env *env,
                                const struct cl_io_slice *ios)
 {
-       struct ccc_io   *cio   = cl2ccc_io(env, ios);
        struct cl_io    *io    = ios->cis_io;
        struct inode    *inode = ccc_object_inode(io->ci_obj);
 
-       LASSERT(cio->u.setattr.cui_locks_released);
-
        mutex_lock(&inode->i_mutex);
-       cio->u.setattr.cui_locks_released = 0;
-
        if (cl_io_is_trunc(io))
                return vvp_io_setattr_trunc(env, ios, inode,
                                            io->u.ci_setattr.sa_attr.lvb_size);
@@ -405,30 +385,21 @@ static int vvp_io_setattr_start(const struct lu_env *env,
 static void vvp_io_setattr_end(const struct lu_env *env,
                                const struct cl_io_slice *ios)
 {
-        struct cl_io         *io    = ios->cis_io;
-        struct inode         *inode = ccc_object_inode(io->ci_obj);
-
-        if (!cl_io_is_trunc(io))
-                return;
+       struct cl_io *io    = ios->cis_io;
+       struct inode *inode = ccc_object_inode(io->ci_obj);
 
-       /* Truncate in memory pages - they must be clean pages because osc
-        * has already notified to destroy osc_extents. */
-       vvp_do_vmtruncate(inode, io->u.ci_setattr.sa_attr.lvb_size);
+       if (cl_io_is_trunc(io)) {
+               /* Truncate in memory pages - they must be clean pages
+                * because osc has already notified to destroy osc_extents. */
+               vvp_do_vmtruncate(inode, io->u.ci_setattr.sa_attr.lvb_size);
+               inode_dio_write_done(inode);
+       }
+       mutex_unlock(&inode->i_mutex);
 }
 
 static void vvp_io_setattr_fini(const struct lu_env *env,
                                const struct cl_io_slice *ios)
 {
-       struct ccc_io *cio   = ccc_env_io(env);
-       struct cl_io  *io    = ios->cis_io;
-       struct inode  *inode = ccc_object_inode(ios->cis_io->ci_obj);
-
-       if (cio->u.setattr.cui_locks_released) {
-               mutex_lock(&inode->i_mutex);
-               if (cl_io_is_trunc(io))
-                       inode_dio_wait(inode);
-               cio->u.setattr.cui_locks_released = 0;
-       }
        vvp_io_fini(env, ios);
 }