Whamcloud - gitweb
LU-952 quota: follow locking order of quota code
authorNiu Yawei <niu@whamcloud.com>
Fri, 6 Jan 2012 09:18:35 +0000 (01:18 -0800)
committerOleg Drokin <green@whamcloud.com>
Mon, 16 Jan 2012 17:10:00 +0000 (12:10 -0500)
The locking order of quota code is: i_mutex > dqonoff_sem >
journal_lock > dqptr_sem > dquot->dq_lock > dqio_mutex, so we
should call the ll_vfs_dq_init() after journal started to avoid
deadlock.

Signed-off-by: Niu Yawei <niu@whamcloud.com>
Change-Id: Ia88a2eb8c9dc3827afd4828e0160ee376a1f041e
Reviewed-on: http://review.whamcloud.com/1923
Reviewed-by: Johann Lombardi <johann@whamcloud.com>
Tested-by: Hudson
Reviewed-by: Fan Yong <yong.fan@whamcloud.com>
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/obdfilter/filter.c
lustre/obdfilter/filter_io_26.c

index 9ba4d36..a31ecf3 100644 (file)
@@ -1596,9 +1596,7 @@ int filter_vfs_unlink(struct inode *dir, struct dentry *dentry,
             IS_IMMUTABLE(dentry->d_inode))
                 GOTO(out, rc = -EPERM);
 
-        /* NOTE: This might need to go outside i_mutex, though it isn't clear if
-         *       that was done because of journal_start (which is already done
-         *       here) or some other ordering issue. */
+        /* Locking order: i_mutex -> journal_lock -> dqptr_sem. LU-952 */
         ll_vfs_dq_init(dir);
 
         rc = ll_security_inode_unlink(dir, dentry, mnt);
@@ -3243,7 +3241,6 @@ int filter_setattr_internal(struct obd_export *exp, struct dentry *dentry,
         }
         if (ia_valid & (ATTR_SIZE | ATTR_UID | ATTR_GID)) {
                 unsigned long now = jiffies;
-                ll_vfs_dq_init(inode);
                 /* Filter truncates and writes are serialized by
                  * i_alloc_sem, see the comment in
                  * filter_preprw_write.*/
@@ -3313,6 +3310,11 @@ int filter_setattr_internal(struct obd_export *exp, struct dentry *dentry,
                 if (IS_ERR(handle))
                         GOTO(out_unlock, rc = PTR_ERR(handle));
         }
+
+        /* Locking order: i_mutex -> journal_lock -> dqptr_sem. LU-952 */
+        if (ia_valid & (ATTR_SIZE | ATTR_UID | ATTR_GID))
+                ll_vfs_dq_init(inode);
+
         if (oa->o_valid & OBD_MD_FLFLAGS) {
                 rc = fsfilt_iocontrol(exp->exp_obd, dentry,
                                       FSFILT_IOC_SETFLAGS, (long)&oa->o_flags);
@@ -4173,7 +4175,6 @@ int filter_destroy(struct obd_export *exp, struct obdo *oa,
                 if (fcc != NULL)
                         *fcc = oa->o_lcookie;
         }
-        ll_vfs_dq_init(dchild->d_inode);
 
         /* we're gonna truncate it first in order to avoid possible deadlock:
          *      P1                      P2
@@ -4208,6 +4209,9 @@ int filter_destroy(struct obd_export *exp, struct obdo *oa,
                 GOTO(cleanup, rc = PTR_ERR(handle));
         }
 
+        /* Locking order: i_mutex -> journal_lock -> dqptr_sem. LU-952 */
+        ll_vfs_dq_init(dchild->d_inode);
+
         iattr.ia_valid = ATTR_SIZE;
         iattr.ia_size = 0;
         rc = fsfilt_setattr(obd, dchild, handle, &iattr, 1);
index cc0d72c..e2e49ea 100644 (file)
@@ -697,7 +697,6 @@ int filter_commitrw_write(struct obd_export *exp, struct obdo *oa,
         push_ctxt(&saved, &obd->obd_lvfs_ctxt, NULL);
         cleanup_phase = 2;
 
-        ll_vfs_dq_init(inode);
         fsfilt_check_slow(obd, now, "quota init");
 
 retry:
@@ -717,6 +716,9 @@ retry:
 
         fsfilt_check_slow(obd, now, "brw_start");
 
+        /* Locking order: i_mutex -> journal_lock -> dqptr_sem. LU-952 */
+        ll_vfs_dq_init(inode);
+
         i = OBD_MD_FLATIME | OBD_MD_FLMTIME | OBD_MD_FLCTIME;
 
         /* If the inode still has SUID+SGID bits set (see filter_precreate())