From 4f3941b090d170c704e01c37ee76fb987bab9562 Mon Sep 17 00:00:00 2001 From: Johann Lombardi Date: Fri, 8 Jan 2010 12:26:33 +0100 Subject: [PATCH] b=18630 fix race during quota release on the slave --- lustre/ChangeLog | 5 ++++ lustre/include/obd_support.h | 1 + lustre/quota/quota_context.c | 57 ++++++++++++++++++++++++++++++++------------ lustre/tests/sanity-quota.sh | 48 +++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 15 deletions(-) diff --git a/lustre/ChangeLog b/lustre/ChangeLog index a4e6519..ab88ab1 100644 --- a/lustre/ChangeLog +++ b/lustre/ChangeLog @@ -23,6 +23,11 @@ Description: BUG: soft lockup - CPU#1 stuck for 10s! [ll_mdt_07:4523] Details : add cond_resched() calls in lustre_hash_for_each_empty() to prevent hogging the CPU. +Severity : normal +Bugzilla : 18630 +Description: LustreError: 9153:0:(quota_context.c:622:dqacq_completion()) LBUG +Details : fix race with quota release requests. + 2009-10-16 Sun Microsystems, Inc. * version 1.8.1.1 * Support for kernels: diff --git a/lustre/include/obd_support.h b/lustre/include/obd_support.h index 60dc91b..3e78155 100644 --- a/lustre/include/obd_support.h +++ b/lustre/include/obd_support.h @@ -337,6 +337,7 @@ extern unsigned int obd_alloc_fail_rate; #define OBD_FAIL_QUOTA_RET_QDATA 0xA02 #define OBD_FAIL_QUOTA_DELAY_REL 0xA03 +#define OBD_FAIL_QUOTA_DELAY_SD 0xA04 #define OBD_FAIL_LPROC_REMOVE 0xB00 diff --git a/lustre/quota/quota_context.c b/lustre/quota/quota_context.c index 3f19a9e..15f1baa 100644 --- a/lustre/quota/quota_context.c +++ b/lustre/quota/quota_context.c @@ -325,11 +325,17 @@ check_cur_qunit(struct obd_device *obd, ret = 2; /* if there are other pending writes for this uid/gid, releasing * quota is put off until the last pending write b=16645 */ - if (ret == 2 && pending_write) { + /* if there is an ongoing quota request, a releasing request is aborted. + * That ongoing quota request will call this function again when + * it returned b=18630 */ + if (pending_write || record) { CDEBUG(D_QUOTA, "delay quota release\n"); ret = 0; } } + if (ret > 0) + quota_compute_lqs(qdata, lqs, 1, (ret == 1) ? 1 : 0); + CDEBUG(D_QUOTA, "type: %c, limit: "LPU64", usage: "LPU64 ", pending_write: "LPU64", record: %llu" ", qunit_sz: %lu, tune_sz: %lu, ret: %d.\n", @@ -902,6 +908,16 @@ static int got_qunit(struct lustre_qunit *qunit, int is_master) RETURN(rc); } +static inline void +revoke_lqs_rec(struct lustre_qunit_size *lqs, struct qunit_data *qdata, int opc) +{ + /* revoke lqs_xxx_rec which is computed in check_cur_qunit + * b=18630 */ + spin_lock(&lqs->lqs_lock); + quota_compute_lqs(qdata, lqs, 0, (opc == QUOTA_DQACQ) ? 1 : 0); + spin_unlock(&lqs->lqs_lock); +} + static int schedule_dqacq(struct obd_device *obd, struct lustre_quota_ctxt *qctxt, struct qunit_data *qdata, int opc, int wait, @@ -923,8 +939,22 @@ schedule_dqacq(struct obd_device *obd, struct lustre_quota_ctxt *qctxt, LASSERT(opc == QUOTA_DQACQ || opc == QUOTA_DQREL); do_gettimeofday(&work_start); - if ((empty = alloc_qunit(qctxt, qdata, opc)) == NULL) + + lqs = quota_search_lqs(LQS_KEY(QDATA_IS_GRP(qdata), qdata->qd_id), + qctxt, 0); + if (lqs == NULL || IS_ERR(lqs)) { + CERROR("Can't find the lustre qunit size!\n"); + RETURN(-EPERM); + } + + if ((empty = alloc_qunit(qctxt, qdata, opc)) == NULL) { + revoke_lqs_rec(lqs, qdata, opc); + /* this is for quota_search_lqs */ + lqs_putref(lqs); RETURN(-ENOMEM); + } + + OBD_FAIL_TIMEOUT(OBD_FAIL_QUOTA_DELAY_SD, 5); spin_lock(&qunit_hash_lock); qunit = dqacq_in_flight(qctxt, qdata); @@ -932,6 +962,9 @@ schedule_dqacq(struct obd_device *obd, struct lustre_quota_ctxt *qctxt, spin_unlock(&qunit_hash_lock); qunit_put(empty); + revoke_lqs_rec(lqs, qdata, opc); + /* this is for quota_search_lqs */ + lqs_putref(lqs); goto wait_completion; } qunit = empty; @@ -939,22 +972,16 @@ schedule_dqacq(struct obd_device *obd, struct lustre_quota_ctxt *qctxt, insert_qunit_nolock(qctxt, qunit); spin_unlock(&qunit_hash_lock); - lqs = quota_search_lqs(LQS_KEY(QDATA_IS_GRP(qdata), qdata->qd_id), - qctxt, 0); - if (lqs && !IS_ERR(lqs)) { - spin_lock(&lqs->lqs_lock); - quota_compute_lqs(qdata, lqs, 1, (opc == QUOTA_DQACQ) ? 1 : 0); - /* when this qdata returned from mds, it will call lqs_putref */ - lqs_getref(lqs); - spin_unlock(&lqs->lqs_lock); - /* this is for quota_search_lqs */ - lqs_putref(lqs); - } else { - CDEBUG(D_ERROR, "Can't find the lustre qunit size!\n"); - } + /* From here, the quota request will be sent anyway. + * When this qdata request returned or is cancelled, + * lqs_putref will be called at that time */ + lqs_getref(lqs); + /* this is for quota_search_lqs */ + lqs_putref(lqs); QDATA_DEBUG(qdata, "obd(%s): send %s quota req\n", obd->obd_name, (opc == QUOTA_DQACQ) ? "acq" : "rel"); + /* master is going to dqacq/dqrel from itself */ if (ismaster) { int rc2; diff --git a/lustre/tests/sanity-quota.sh b/lustre/tests/sanity-quota.sh index 1b0ad38..c013698 100644 --- a/lustre/tests/sanity-quota.sh +++ b/lustre/tests/sanity-quota.sh @@ -166,6 +166,54 @@ run_test_with_stat() { fi } +# test duplicate quota releases b=18630 +test_31() { + mkdir -p $DIR/$tdir + chmod 0777 $DIR/$tdir + + LIMIT=$(( $BUNIT_SZ * $(($OSTCOUNT + 1)) * 10)) # 10 bunits each sever + TESTFILE="$DIR/$tdir/$tfile-0" + TESTFILE2="$DIR/$tdir/$tfile-1" + + wait_delete_completed + + log " User quota (limit: $LIMIT kbytes)" + $LFS setquota -u $TSTUSR -b 0 -B $LIMIT -i 0 -I 0 $DIR + + $LFS setstripe $TESTFILE -i 0 -c 1 + chown $TSTUSR.$TSTUSR $TESTFILE + $LFS setstripe $TESTFILE2 -i 0 -c 1 + chown $TSTUSR.$TSTUSR $TESTFILE2 + + log " step1: write out of block quota ..." + $RUNAS dd if=/dev/zero of=$TESTFILE bs=$BLK_SZ count=5120 + $RUNAS dd if=/dev/zero of=$TESTFILE2 bs=$BLK_SZ count=5120 + + #define OBD_FAIL_QUOTA_DELAY_SD 0xA04 + #define OBD_FAIL_SOME 0x10000000 /* fail N times */ + lustre_fail ost $((0x00000A04 | 0x10000000)) 1 + + log " step2: delete two files so that triggering duplicate quota release ..." + rm -f $TESTFILE $TESTFILE2 + sync; sleep 5; sync # OBD_FAIL_QUOTA_DELAY_SD will delay for 5 seconds + wait_delete_completed + + log " step3: verify if the ost failed" + do_facet ost1 dmesg > $TMP/lustre-log-${TESTNAME}.log + watchdog=`awk '/test 31/ {start = 1;} + /release quota error/ { + if (start) { + print; + } + }' $TMP/lustre-log-${TESTNAME}.log` + [ "$watchdog" ] && error "$watchdog" + rm -f $TMP/lustre-log-${TESTNAME}.log + + lustre_fail ost 0 + resetquota -u $TSTUSR +} +run_test_with_stat 31 "test duplicate quota releases ===" + # # clear quota limits for a user or a group # usage: resetquota -u username -- 1.8.3.1