Re: [PATCH] quota: add per-inode reservaton space sanity checks.

[Dmitry Monakhov <dmonakhov@openvz.org>]

[-- Attachment #1: Type: text/plain, Size: 2013 bytes --]

dmonakhov@openvz.org writes:

> Jan Kara <jack@suse.cz> writes:
>
>> On Tue 30-03-10 18:25:28, Dmitry Monakhov wrote:
>>> We already has per-dquot sanity checks, but with per-inode checks
>>> quota leakage investigation becomes much easier.
>>> 
>>> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
>>> ---
>>>  fs/quota/dquot.c |    4 ++++
>>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>> 
>>> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
>>> index e0b870f..4db57b7 100644
>>> --- a/fs/quota/dquot.c
>>> +++ b/fs/quota/dquot.c
>>> @@ -1428,6 +1428,8 @@ EXPORT_SYMBOL(inode_add_rsv_space);
>>>  void inode_claim_rsv_space(struct inode *inode, qsize_t number)
>>>  {
>>>  	spin_lock(&inode->i_lock);
>>> +	if (*inode_reserved_space(inode) < number)
>>> +		WARN_ON_ONCE(1);
>>   Maybe just: WARN_ON_ONCE(*inode_reserved_space(inode) < number)
Ohh. While writing testcase for automatic-quota consistency check.
I've failed to reliably detect condition where quota goes inconsistent.
And it appears to be not what easy. We have hided real problems too deeply.
The only reliable way is to grep dmesg for hard-coded "fs/quota/dquot.c"
string which produced by WARN_ON_ONCE.

After some thoughts (sometimes it happens with me too)
i'm think what it is reasonable rewrite all quota error messages logic.
Make it similar to ext4
quota_error(...) {
   printk("QUOTA error (dev:%s)", ...);
   handle_quota_error(sb) /* let sb to know what quota is corrupted */
}
quota_warn(...) {
   printk("QUOTA warn (dev:%s)", ...);
}
In fact it is very important to let filesystem to now that it's quota
is corrupted(quota == fs-metadata). It may set fsck_needed flag on
super block for later correction.
So please ignore this patch for now. I'll prepare another one which
redesign error conditions handling logic.

BTW: I've attached my testcase. I hope it will be useful for you.
It able to catch quota inconsistency caused by incorrect symlink
handling, but it is not reliable for writepage/fallocate bug in ext4.


[-- Attachment #2: 0001-xfstests-add-quota-stress-test.patch --]
[-- Type: text/plain, Size: 5857 bytes --]

>From fa70a77403f21a871decc0c61d665a82ae492f7c Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Tue, 30 Mar 2010 20:59:20 +0400
Subject: [PATCH] xfstests: add quota stress test

Run fsstress on filesystem with quota enabled, then recheckquota
and comare with old value.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 500      |  179 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 500.full |    4 ++
 500.out  |    5 ++
 group    |    1 +
 4 files changed, 189 insertions(+), 0 deletions(-)
 create mode 100755 500
 create mode 100644 500.full
 create mode 100644 500.out

diff --git a/500 b/500
new file mode 100755
index 0000000..47999d6
--- /dev/null
+++ b/500
@@ -0,0 +1,179 @@
+#! /bin/bash
+# FS QA Test No. 500
+#
+# Quota accounting stress test
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2010 Dmitry Monakhov.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+#-----------------------------------------------------------------------
+#
+# creator
+owner=dmonakhov@openvz.org
+
+seq=`basename $0`
+echo "QA output created by $seq"
+killall="/usr/bin/killall"
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+quota_supported=0
+
+_cleanup()
+{
+    rm -f $tmp.*
+}
+
+require_quota()
+{
+    case $FSTYP in
+	ext2|reiserfs|jfs)
+	    export MOUNT_OPTIONS="$MOUNT_OPTIONS -o usrquota,grpquota"
+	    quota_supported=1
+	    ;;
+	ext3|ext4)
+	    export MOUNT_OPTIONS="$MOUNT_OPTIONS -o jqfmt=vfsv0,usrjquota=aquota.user,grpjquota=aquota.group"
+	    quota_supported=1
+	    ;;
+	*)
+	    quota_supported=0
+	    ;;
+	esac
+}
+
+setup_quota()
+{
+    mountpoint=$1
+    case $FSTYP in
+	xfs)
+	    ;;
+	ext*|reiserfs|jfs)
+	    quotaoff $mountpoint &>/dev/null
+	    quotacheck -c -u -g $mountpoint
+	    quotaon $mountpoint
+	    ;;
+	*)
+	    _fail "Don't know how to turn on quota on $FSTYP"
+	    ;;
+    esac
+}
+
+check_quota()
+{
+    mountpoint=$1
+    case $FSTYP in
+	ext*|jfs|reiserfs)
+	    NAME=`mktemp -d $tmp_dir/XXXX`
+	    mkdir -p $NAME/orig $NAME/chk
+	    sync;
+	    repquota -n -u $mountpoint | sort > ${NAME}/orig/user
+	    repquota -n -g $mountpoint | sort > ${NAME}/orig/group
+
+	    # All writers was killed and fs was synced.
+	    # Now quota may be safely disabled for quota recalculation
+	    quotaoff $mountpoint
+	    quotacheck -c -u -g $mountpoint
+	    quotaon $mountpoint
+
+	    repquota -n -u $mountpoint | sort > ${NAME}/chk/user
+	    repquota -n -g $mountpoint | sort > ${NAME}/chk/group
+
+	    #remove this 
+	    cp -r ${NAME} /tmp/DIFF
+	    echo "Quota check " >> $seq.full
+	    diff -upr ${NAME}/{orig,chk} >> $seq.full
+	    local quotacheck_status=$?
+
+	    if [ $quotacheck_status -ne 0 ]; then
+		_fail " Quota check failed, see quota diff."
+	    fi
+	    ;;
+	*)
+	    _fail "Don't know how to check quota on $FSTYP"
+	    ;;
+    esac
+}
+
+workout()
+{
+    # Disable bash job controll, to prevent message about killed task.
+    set +m
+
+    #Timing parameters
+    nr_sec=15
+    kill_tries=10
+    echo Running fsstress. | tee -a $seq.full
+
+####################################################
+##    -f unresvsp=0 -f allocsp=0 -f freesp=0 \
+##    -f setxattr=0 -f attr_remove=0 -f attr_set=0 \
+######################################################
+    $FSSTRESS_PROG \
+	-d $SCRATCH_MNT/fsstress \
+	-p 100 -n 9999999 -s $seed > /dev/null 2>&1 &
+    sleep $((nr_sec - kill_tries))
+
+    for ((i = 0; i < $kill_tries; i++))
+    do
+	killall -r -q -TERM fsstress 2> /dev/null
+	sleep 1
+    done
+
+}
+
+trap "_cleanup ; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# This following sequance seed it is takes 5 sec to reproduce quota
+# inconsistency bug in ext4
+seed=1270493518
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+require_quota
+[ -x $killall ] || _notrun "$killall executable not found"
+if [ $quota_supported -ne 1 ] ;then 
+    _notrun "Don't know how to turn on quota on $FSTYP"
+fi
+
+rm -f $seq.full
+
+umount $TEST_DEV >/dev/null 2>&1
+umount $SCRATCH_DEV >/dev/null 2>&1
+echo "*** MKFS ***"                         >>$seq.full
+echo ""                                     >>$seq.full
+_scratch_mkfs    >/dev/null 2>&1 || _fail "mkfs failed"
+_scratch_mount   >/dev/null 2>&1 || _fail "mount failed"
+setup_quota $SCRATCH_MNT
+
+mkdir -p $SCRATCH_MNT/fsstress
+workout
+
+echo Checking quota
+check_quota $SCRATCH_MNT
+umount $SCRATCH_MNT
+
+echo 
+echo Checking filesystem
+_check_scratch_fs
+_scratch_mount
+status=$?
+exit
diff --git a/500.full b/500.full
new file mode 100644
index 0000000..8f41b34
--- /dev/null
+++ b/500.full
@@ -0,0 +1,4 @@
+*** MKFS ***
+
+Running fsstress.
+Quota check 
diff --git a/500.out b/500.out
new file mode 100644
index 0000000..b4614fc
--- /dev/null
+++ b/500.out
@@ -0,0 +1,5 @@
+QA output created by 500
+Running fsstress.
+Checking quota
+
+Checking filesystem
diff --git a/group b/group
index 8d4a83a..3164c37 100644
--- a/group
+++ b/group
@@ -339,3 +339,4 @@ deprecated
 223 auto quick
 224 auto
 225 auto quick
+500 auto quota
\ No newline at end of file
-- 
1.6.6