Patchwork Regression with ext4 in kernel 2.6.39-rc7? (Was: testing ext4 master branch)

login
register
mail settings
Submitter Jan Kara
Date May 16, 2011, 9:43 a.m.
Message ID <20110516094324.GA5344@quack.suse.cz>
Download mbox | patch
Permalink /patch/95702/
State Not Applicable
Headers show

Comments

Jan Kara - May 16, 2011, 9:43 a.m.
On Sat 14-05-11 13:16:25, Amir Goldstein wrote:
> Sent from my iPhone
> 
> On 14/05/2011, at 10:11, Amir Goldstein <amir73il@gmail.com> wrote:
> 
> >On Sat, May 14, 2011 at 1:49 AM, Ted Ts'o <tytso@mit.edu> wrote:
> >>On Fri, May 13, 2011 at 08:25:02PM +0300, Amir Goldstein wrote:
> >>>
> >>>After xfstests failed to build on Ubuntu 10.10, I followed the
> >>>advice omitted by the build script to run "make install-qa", to
> >>>solve the problem.  It took me a while to figure exactly where I
> >>>should run the command, but in the end I pulled the xfsprogs tree,
> >>>ran "make; make install; make install-qa" and from there on things
> >>>were looking better.
> >>
> >>I did that, but fsstress doesn't pull in the needed xfs/xfs_fs.h
> >>header file.  So it still dies.
> >>
> >>                                               - Ted
> >>
> >
> >Well, anyway, the regression has to be from commit  21f97697:
> >
> >ext4: remove unnecessary [cm]time update of quota file
> >
> >because before that commit ext4_quota_off() was too short to
> >have a bug at ext4_quota_off+0x42/0xd0.
> >
> >Jan, where are you? don't make me debug this...
> 
> So I guess that Jan's patch is missing
> If (!inode)
>     goto out;
  Exactly, that is it. I cannot trigger the problem anymore with the patch.
I just wonder how come you've spotted the problem because test 232 does not
trigger the problem for me - it is triggered when you run quotaoff without
running quotaon and that does not happen with test 232.. Anyway, with the
attached patch running quotaoff on filesystem without quotas turned on
works fine whereas previously it oopsed.

									Honza
Amir Goldstein - May 16, 2011, 9:59 a.m.
On Mon, May 16, 2011 at 12:43 PM, Jan Kara <jack@suse.cz> wrote:
> On Sat 14-05-11 13:16:25, Amir Goldstein wrote:
>> Sent from my iPhone
>>
>> On 14/05/2011, at 10:11, Amir Goldstein <amir73il@gmail.com> wrote:
>>
>> >On Sat, May 14, 2011 at 1:49 AM, Ted Ts'o <tytso@mit.edu> wrote:
>> >>On Fri, May 13, 2011 at 08:25:02PM +0300, Amir Goldstein wrote:
>> >>>
>> >>>After xfstests failed to build on Ubuntu 10.10, I followed the
>> >>>advice omitted by the build script to run "make install-qa", to
>> >>>solve the problem.  It took me a while to figure exactly where I
>> >>>should run the command, but in the end I pulled the xfsprogs tree,
>> >>>ran "make; make install; make install-qa" and from there on things
>> >>>were looking better.
>> >>
>> >>I did that, but fsstress doesn't pull in the needed xfs/xfs_fs.h
>> >>header file.  So it still dies.
>> >>
>> >>                                               - Ted
>> >>
>> >
>> >Well, anyway, the regression has to be from commit  21f97697:
>> >
>> >ext4: remove unnecessary [cm]time update of quota file
>> >
>> >because before that commit ext4_quota_off() was too short to
>> >have a bug at ext4_quota_off+0x42/0xd0.
>> >
>> >Jan, where are you? don't make me debug this...
>>
>> So I guess that Jan's patch is missing
>> If (!inode)
>>     goto out;
>  Exactly, that is it. I cannot trigger the problem anymore with the patch.
> I just wonder how come you've spotted the problem because test 232 does not
> trigger the problem for me -

It does not surprise me.
As I wrote earlier, my system (Ubuntu 11.4) is behaving strange.
Some tests fail the fsck at the end, because the filesystem fails to umount
immediately after the test (lsof shows nothing and manual umount succeeds).
It's probably the cause for 232 test to fail (even after the crash was fixed)
and the cause for quotaoff to be called without quotaon in test 232 on
my system.

> it is triggered when you run quotaoff without
> running quotaon and that does not happen with test 232.. Anyway, with the
> attached patch running quotaoff on filesystem without quotas turned on
> works fine whereas previously it oopsed.
>
>                                                                        Honza
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

From 91c77c4f066a9b905538bbeb1da46dc47c44855c Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Mon, 16 May 2011 10:49:39 +0200
Subject: [PATCH] ext4: Fix oops in ext4_quota_off()

When quota is not enabled when ext4_quota_off() is called, we must not
dereference quota file inode since it is NULL. Check properly for this.

Debugged-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/super.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index a86e693..2a4a9b2 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4652,6 +4652,8 @@  static int ext4_quota_off(struct super_block *sb, int type)
 	if (test_opt(sb, DELALLOC))
 		sync_filesystem(sb);
 
+	if (!inode)
+		goto out;
 	/* Update modification times of quota files when userspace can
 	 * start looking at them */
 	handle = ext4_journal_start(inode, 1);
-- 
1.7.1