Whamcloud - gitweb
LU-1438 quota: fix race in quota_chk_acq_common()
authorNiu Yawei <niu@whamcloud.com>
Fri, 1 Jun 2012 06:28:45 +0000 (23:28 -0700)
committerOleg Drokin <green@whamcloud.com>
Mon, 4 Jun 2012 07:21:48 +0000 (03:21 -0400)
quota_check_common() & qctxt_adjust_qunit() uses different way
to check if quota is enforced on certain ID, which could result
in infinite loop in quota_chk_acq_common() when the QB/QI_SET
flag is cleared just after checking.

This patch used a non-instrusive way to fix this rare race.

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

index 9e4cac2..511f0f5 100644 (file)
@@ -1137,8 +1137,31 @@ qctxt_adjust_qunit(struct obd_device *obd, struct lustre_quota_ctxt *qctxt,
         struct qunit_data qdata[MAXQUOTAS];
         ENTRY;
 
         struct qunit_data qdata[MAXQUOTAS];
         ENTRY;
 
-        if (quota_is_set(obd, id, isblk ? QB_SET : QI_SET) == 0)
-                RETURN(0);
+       /* XXX In quota_chk_acq_common(), we do something like:
+        *
+        *     while (quota_check_common() & QUOTA_RET_ACQUOTA) {
+        *            rc = qctxt_adjust_qunit();
+        *     }
+        *
+        *     to make sure the slave acquired enough quota from master.
+        *
+        *     Unfortunately, qctxt_adjust_qunit() checks QB/QI_SET to
+        *     decide if do real DQACQ or not, but quota_check_common()
+        *     doesn't check QB/QI_SET flags. This inconsistence could
+        *     lead into an infinite loop.
+        *
+        *     We can't fix it by simply adding QB/QI_SET checking in the
+        *     quota_check_common(), since we must guarantee that the
+        *     paried quota_pending_commit() has same QB/QI_SET, but the
+        *     flags can be actually cleared at any time...
+        *
+        *     A quick non-intrusive solution is to just skip the
+        *     QB/QI_SET checking here when the @wait is non-zero.
+        *     (If the @wait is non-zero, the caller must have already
+        *     checked the QB/QI_SET).
+        */
+       if (!wait && quota_is_set(obd, id, isblk ? QB_SET : QI_SET) == 0)
+               RETURN(0);
 
         for (i = 0; i < MAXQUOTAS; i++) {
                 qdata[i].qd_id = id[i];
 
         for (i = 0; i < MAXQUOTAS; i++) {
                 qdata[i].qd_id = id[i];