Bug#587763: scary messages from JBD when manipulating quotas on ext4

2012-02-27 Thread Robert L Mathews

Jonathan Nieder wrote:


Thanks for the update, and no problem.  What filesystem options does
your test system use (as shown in /proc/mounts or by dumpe2fs -h
bdev)?  How do you set up quotas on it?


Both the test system (that doesn't show errors) and the live system 
(that does) have identical mount options:


 ext4 rw,noatime,errors=remount-ro,acl,barrier=1,data=ordered,usrquota

The initial quotas were enabled with quotaon -a, then by setting a 
100GB / 1 million file quota for each user on the system other than 
root, using setquota.


Don't know why I couldn't reproduce it. Perhaps something to do with the 
live server's (heavy and varied) disk write patterns.




Looking closer, straightforward testing of that one patch would
probably not reveal much anyway --- any subtle effects wouldn't
necessarily be triggered often.  Pointers that might help someone
review the code:


Hmmm. These changes to eliminate an erroneous -- but harmless -- warning 
gives me the heebie-jeebies! In your shoes, I'd mark this as won't fix 
and let it get fixed in the next real kernel version change


--
Robert L Mathews, Tiger Technologies, http://www.tigertech.net/



--
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/4f4c024b.2090...@tigertech.com



Bug#587763: scary messages from JBD when manipulating quotas on ext4

2012-02-16 Thread Jonathan Nieder
Hi,

Robert L Mathews wrote:

 Unfortunately, I'm not able to reproduce the original problem on a
 test system for some reason (and I tried quite hard). It only
 happens on one of my production servers, but I can't experiment on
 that one. Sorry about that.  :-(

Thanks for the update, and no problem.  What filesystem options does
your test system use (as shown in /proc/mounts or by dumpe2fs -h
bdev)?  How do you set up quotas on it?

Looking closer, straightforward testing of that one patch would
probably not reveal much anyway --- any subtle effects wouldn't
necessarily be triggered often.  Pointers that might help someone
review the code:

  http://thread.gmane.org/gmane.comp.file-systems.ext4/19421/focus=19462
  http://thread.gmane.org/gmane.comp.file-systems.ext4/19421/focus=19470

I suspect applying the patch to 2.6.32.y is safe even in edge cases.

Summary of the discussion: Ted said

|  In the long-term I
| think the quota file should become a special file much like the
| journal, and then this makes a huge amount of sense.

Jan addressed his fears by clarifying that the quota file is immutable
when quotas are on:

|   Ted, that's what generic quota code actually does for you (unless
| DQUOT_QUOTA_SYS_FILE flag is specified but that's not the case of
| ext?)
| - see vfs_load_quota_inode.

The basic rules about quota have not changed fundamentally between
2.6.32 and 2.6.36, so this looks like a good and safe patch for
squeeze.  To avoid tempting fate, here is a list including some other
fixes that could go with it.  I haven't tried testing this at all yet.
Comments (including test results) welcome.

 - v2.6.34-rc1~192^2~11 quota: Properly invalidate caches even for
   filesystems with blocksize  pagesize, 2010-02-22

 - v2.6.36-rc1~501^2~15 ext4: Always journal quota file
   modifications, 2010-07-27

 - v2.6.36-rc1~501^2~7 ext4: force block allocation on quota_off,
   2010-08-01, and v2.6.37-rc2~75^2~2 ext4: do not try to grab the
   s_umount semaphore in ext4_quota_off, 2010-11-08

 - v2.6.33-rc1~311^2~14 quota: Fix WARN_ON in lookup_one_len,
   2009-11-12

 - 6b6dc836a vfs: Provide function to get superblock and wait for it
   to thaw, 2012-02-10, and dcdbed853d9f quota: Fix deadlock with
   suspend and quotas, 2012-02-10

 fs/ext4/super.c|   32 +---
 fs/quota/dquot.c   |   14 +-
 fs/quota/quota.c   |   24 +---
 fs/super.c |   22 ++
 include/linux/fs.h |1 +
 5 files changed, 70 insertions(+), 23 deletions(-)
From: Jan Kara j...@suse.cz
Date: Mon, 22 Feb 2010 21:07:17 +0100
Subject: quota: Properly invalidate caches even for filesystems with blocksize 
 pagesize

commit ab94c39b6fa076d4f6d2903dcc54cda35d938776 upstream.

Sometimes invalidate_bdev() can fail to invalidate a part of block
device cache because of dirty data. If the filesystem has blocksize
smaller than page size, this can happen even for pages containing
quota files and thus kernel would operate on stale data. Fix the
issue by syncing the filesystem before invalidating the cache.

Reviewed-by: Christoph Hellwig h...@lst.de
Signed-off-by: Jan Kara j...@suse.cz
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
 fs/quota/dquot.c |   12 +++-
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index ce9a4f22c1dd..d06ee1f12c8b 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -2087,11 +2087,13 @@ static int vfs_load_quota_inode(struct inode *inode, 
int type, int format_id,
}
 
if (!(dqopt-flags  DQUOT_QUOTA_SYS_FILE)) {
-   /* As we bypass the pagecache we must now flush the inode so
-* that we see all the changes from userspace... */
-   write_inode_now(inode, 1);
-   /* And now flush the block cache so that kernel sees the
-* changes */
+   /* As we bypass the pagecache we must now flush all the
+* dirty data and invalidate caches so that kernel sees
+* changes from userspace. It is not enough to just flush
+* the quota file since if blocksize  pagesize, invalidation
+* of the cache could fail because of other unrelated dirty
+* data */
+   sync_filesystem(sb);
invalidate_bdev(sb-s_bdev);
}
mutex_lock(dqopt-dqonoff_mutex);
-- 
1.7.9

From: Jan Kara j...@suse.cz
Date: Tue, 27 Jul 2010 11:56:07 -0400
Subject: ext4: Always journal quota file modifications

commit 62d2b5f2dcd3707b070efb16bbfdf6947c38c194 upstream.

When journaled quota options are not specified, we do writes
to quota files just in data=ordered mode. This actually causes
warnings from JBD2 about dirty journaled buffer because ext4_getblk
unconditionally treats a block allocated by it as metadata. Since
quota actually is filesystem metadata, the easiest