linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* 2.6.15-mm3 - disk quotas apparently busticated..
@ 2006-01-12 21:16 Valdis.Kletnieks
  2006-01-14 21:19 ` Jan Kara
  0 siblings, 1 reply; 3+ messages in thread
From: Valdis.Kletnieks @ 2006-01-12 21:16 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

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

Disk quotas have been functioning for me for quite a while (including 2.6.15 and
2.6.15-rc*-mm* - haven't tried 15-mm1 or 15-mm2 yet).

Under the 2.6.15-mm3 kernel, 'quotaon /home' (or any other ext3 filesystem
that had functional quotas on it fails:

# quotaon /home
quotaon: using /home/aquota.group on /dev/mapper/rootvg-home [/home]: Invalid argument
quotaon: Maybe create new quota files with quotacheck(8)?
quotaon: using /home/aquota.user on /dev/mapper/rootvg-home [/home]: Invalid argument
quotaon: Maybe create new quota files with quotacheck(8)?
# quotacheck -u -g -m /home
# quotaon /home
quotaon: using /home/aquota.group on /dev/mapper/rootvg-home [/home]: Invalid argument
quotaon: Maybe create new quota files with quotacheck(8)?
quotaon: using /home/aquota.user on /dev/mapper/rootvg-home [/home]: Invalid argument
quotaon: Maybe create new quota files with quotacheck(8)?

Using quota-3.13.

Poking with strace finds:
...
quotactl(Q_QUOTAON|GRPQUOTA, "/dev/mapper/rootvg-home", 2, {8169863311701010479, 8030594534456192885, 141733949557, 8097873655606109231, 8390047167027504496, 28549297904379766, 1309965025280, 13254570172929388888}) = -1 EINVAL (Invalid argument)
...
quotactl(Q_QUOTAON|USRQUOTA, "/dev/mapper/rootvg-home", 2, {8169863311701010479, 7310315462216413045, 141733920882, 8097873655606109231, 8390047167027504496, 28549297904379766, 210453397504, 7018986666877744431}) = -1 EINVAL (Invalid argument)

And the following corresponding cryptic messages in dmesg:

[ 1833.288000] failed read
[ 1833.332000] failed read

(which is why I just submitted a patch to fs/quota_v2.c)

Further digging based on the info from that patch implicates ext3_quota_read()
(which itself seems potentially buggy - it has code that says:

                bh = ext3_bread(NULL, inode, blk, 0, &err);
                if (err)
                        return err;

which is returning 1 in my case.  Good thing that ext3_get_blocks_handle()
(which eventually gets called) can't return 8....

It's getting down to fs/ext3/inode.c ext3_getblk() and this code:

        *errp = ext3_get_blocks_handle(handle, inode, block, 1, &dummy, create, 1);
        if ((*errp == 1 ) && buffer_mapped(&dummy)) {

which sets *errp to 1. We then enter a 40-line block, which can return a valid
struct buffer_head, but never set another value into *errp.

Bug/question: Should that big 'if' statement reset *errp if things work out OK?

--- inode.c     2006-01-12 13:35:44.000000000 -0500
+++ inode.c.temp        2006-01-12 16:13:44.000000000 -0500
@@ -1048,14 +1048,16 @@
                } else {
                        BUFFER_TRACE(bh, "not a new buffer");
                }
                if (fatal) {
                        *errp = fatal;
                        brelse(bh);
                        bh = NULL;
+               } else {
+                       *errp = 0;
                }
                return bh;
        }
 err:
        return NULL;
 }


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: 2.6.15-mm3 - disk quotas apparently busticated..
  2006-01-12 21:16 2.6.15-mm3 - disk quotas apparently busticated Valdis.Kletnieks
@ 2006-01-14 21:19 ` Jan Kara
  2006-01-14 21:44   ` Andrew Morton
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Kara @ 2006-01-14 21:19 UTC (permalink / raw)
  To: Valdis.Kletnieks; +Cc: Andrew Morton, linux-kernel

  Hello,

> Disk quotas have been functioning for me for quite a while (including 2.6.15 and
> 2.6.15-rc*-mm* - haven't tried 15-mm1 or 15-mm2 yet).
> 
> Under the 2.6.15-mm3 kernel, 'quotaon /home' (or any other ext3 filesystem
> that had functional quotas on it fails:
> 
> # quotaon /home
> quotaon: using /home/aquota.group on /dev/mapper/rootvg-home [/home]: Invalid argument
> quotaon: Maybe create new quota files with quotacheck(8)?
> quotaon: using /home/aquota.user on /dev/mapper/rootvg-home [/home]: Invalid argument
> quotaon: Maybe create new quota files with quotacheck(8)?
> # quotacheck -u -g -m /home
> # quotaon /home
> quotaon: using /home/aquota.group on /dev/mapper/rootvg-home [/home]: Invalid argument
> quotaon: Maybe create new quota files with quotacheck(8)?
> quotaon: using /home/aquota.user on /dev/mapper/rootvg-home [/home]: Invalid argument
> quotaon: Maybe create new quota files with quotacheck(8)?
> 
> Using quota-3.13.
> 
> Poking with strace finds:
> ...
> quotactl(Q_QUOTAON|GRPQUOTA, "/dev/mapper/rootvg-home", 2, {8169863311701010479, 8030594534456192885, 141733949557, 8097873655606109231, 8390047167027504496, 28549297904379766, 1309965025280, 13254570172929388888}) = -1 EINVAL (Invalid argument)
> ...
> quotactl(Q_QUOTAON|USRQUOTA, "/dev/mapper/rootvg-home", 2, {8169863311701010479, 7310315462216413045, 141733920882, 8097873655606109231, 8390047167027504496, 28549297904379766, 210453397504, 7018986666877744431}) = -1 EINVAL (Invalid argument)
> 
> And the following corresponding cryptic messages in dmesg:
> 
> [ 1833.288000] failed read
> [ 1833.332000] failed read
> 
> (which is why I just submitted a patch to fs/quota_v2.c)
  Yes, thanks for that.

> Further digging based on the info from that patch implicates ext3_quota_read()
> (which itself seems potentially buggy - it has code that says:
> 
>                 bh = ext3_bread(NULL, inode, blk, 0, &err);
>                 if (err)
>                         return err;
> 
> which is returning 1 in my case.  Good thing that ext3_get_blocks_handle()
> (which eventually gets called) can't return 8....
> 
> It's getting down to fs/ext3/inode.c ext3_getblk() and this code:
> 
>         *errp = ext3_get_blocks_handle(handle, inode, block, 1, &dummy, create, 1);
>         if ((*errp == 1 ) && buffer_mapped(&dummy)) {
> 
> which sets *errp to 1. We then enter a 40-line block, which can return a valid
> struct buffer_head, but never set another value into *errp.
> 
> Bug/question: Should that big 'if' statement reset *errp if things work out OK?
> 
> --- inode.c     2006-01-12 13:35:44.000000000 -0500
> +++ inode.c.temp        2006-01-12 16:13:44.000000000 -0500
> @@ -1048,14 +1048,16 @@
>                 } else {
>                         BUFFER_TRACE(bh, "not a new buffer");
>                 }
>                 if (fatal) {
>                         *errp = fatal;
>                         brelse(bh);
>                         bh = NULL;
> +               } else {
> +                       *errp = 0;
>                 }
>                 return bh;
>         }
>  err:
>         return NULL;
>  }
  Yup, that patch seems to be correct. I'm not sure if Andrew has picked
up the patch as it is missing Signed-off-by and also directory of the
file being patched (see Documentation/SubmittingPatches for general
guidelines). So if Andrew has not responded yet, regenerate the patch
and send it to him please. Thanks for the fix.

								Honza
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: 2.6.15-mm3 - disk quotas apparently busticated..
  2006-01-14 21:19 ` Jan Kara
@ 2006-01-14 21:44   ` Andrew Morton
  0 siblings, 0 replies; 3+ messages in thread
From: Andrew Morton @ 2006-01-14 21:44 UTC (permalink / raw)
  To: Jan Kara; +Cc: Valdis.Kletnieks, linux-kernel

Jan Kara <jack@suse.cz> wrote:
>
> > +               } else {
>  > +                       *errp = 0;
>  >                 }
>  >                 return bh;
>  >         }
>  >  err:
>  >         return NULL;
>  >  }
>    Yup, that patch seems to be correct. I'm not sure if Andrew has picked
>  up the patch as it is missing Signed-off-by and also directory of the
>  file being patched (see Documentation/SubmittingPatches for general
>  guidelines). So if Andrew has not responded yet, regenerate the patch
>  and send it to him please. Thanks for the fix.

We ended up with this:

--- devel/fs/ext3/inode.c~ext3-get-blocks-maping-multiple-blocks-at-a-once-ext3_getblk-fix	2006-01-13 12:34:37.000000000 -0800
+++ devel-akpm/fs/ext3/inode.c	2006-01-13 12:35:39.000000000 -0800
@@ -899,8 +899,16 @@ struct buffer_head *ext3_getblk(handle_t
 	dummy.b_state = 0;
 	dummy.b_blocknr = -1000;
 	buffer_trace_init(&dummy.b_history);
-	*errp = ext3_get_blocks_handle(handle, inode, block, 1, &dummy, create, 1);
-	if ((*errp == 1 ) && buffer_mapped(&dummy)) {
+	err = ext3_get_blocks_handle(handle, inode, block, 1,
+					&dummy, create, 1);
+	if (err == 1) {
+		err = 0;
+	} else if (err >= 0) {
+		WARN_ON(1);
+		err = -EIO;
+	}
+	*errp = err;
+	if (!err && buffer_mapped(&dummy)) {
 		struct buffer_head *bh;
 		bh = sb_getblk(inode->i_sb, dummy.b_blocknr);
 		if (!bh) {
_


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2006-01-14 21:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-01-12 21:16 2.6.15-mm3 - disk quotas apparently busticated Valdis.Kletnieks
2006-01-14 21:19 ` Jan Kara
2006-01-14 21:44   ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).