2.6.26-mmotm-0724 - linux-next.git loses /proc/sys/fs/quota, breaks disk quotas

2.6.26-mmotm-0724 - linux-next.git loses /proc/sys/fs/quota, breaks disk quotas

[Valdis.Kletnieks]

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

I bisected the problem to somewhere in here:

repeatable-slab-corruption-with-ltp-msgctl08.patch GOOD
#
linux-next.patch
linux-next-git-rejects.patch
linux-next-fixup.patch BAD

I have no idea how to go bisecting through linux-next.patch

It works in 26-mmotm-0715, but is broken in -0724, so that will hopefully
narrow the suspect commits down a *little* bit...

The problem is that the userspace does this:

        /* Detect new kernel interface; Assume generic interface unless we can p
rove there is not one... */
        if (!stat("/proc/sys/fs/quota", &st) || errno != ENOENT) {
                kernel_iface = IFACE_GENERIC;
                kernel_formats |= (1 << QF_VFSOLD) | (1 << QF_VFSV0);
        }
        else {
                struct v2_dqstats v2_stats;

and /proc/sys/fs/quota has mysteriously vanished in -0724. As a result,
the userspace code goes looking for the quotav1 files and doesn't find them,
since the actual files on disk are quotav2 (with a different name).

fs/dquot.c, which contains the code for that, appears to be identical in -0715
and -0724, so something *else* is breaking it.

Anybody know where /proc/sys/fs/quota went?

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

Re: 2.6.26-mmotm-0724 - linux-next.git loses /proc/sys/fs/quota, breaks disk quotas

[Andrew Morton]

On Sat, 26 Jul 2008 22:22:46 -0400 Valdis.Kletnieks@vt.edu wrote:

> I bisected the problem to somewhere in here:
> 
> repeatable-slab-corruption-with-ltp-msgctl08.patch GOOD
> #
> linux-next.patch
> linux-next-git-rejects.patch
> linux-next-fixup.patch BAD
> 
> I have no idea how to go bisecting through linux-next.patch

You need to grab the git tree and use git-bisect.  Then wade through
all the runtime and compile-time bisection breakage which people
nowadays seem to find acceptable because git collapses in an
ignominious heap when these are fixed.

> It works in 26-mmotm-0715, but is broken in -0724, so that will hopefully
> narrow the suspect commits down a *little* bit...
> 
> The problem is that the userspace does this:
> 
>         /* Detect new kernel interface; Assume generic interface unless we can p
> rove there is not one... */
>         if (!stat("/proc/sys/fs/quota", &st) || errno != ENOENT) {
>                 kernel_iface = IFACE_GENERIC;
>                 kernel_formats |= (1 << QF_VFSOLD) | (1 << QF_VFSV0);
>         }
>         else {
>                 struct v2_dqstats v2_stats;
> 
> and /proc/sys/fs/quota has mysteriously vanished in -0724. As a result,
> the userspace code goes looking for the quotav1 files and doesn't find them,
> since the actual files on disk are quotav2 (with a different name).
> 
> fs/dquot.c, which contains the code for that, appears to be identical in -0715
> and -0724, so something *else* is breaking it.
> 
> Anybody know where /proc/sys/fs/quota went?
> 

Nope.  There are pretty extensive changes to procfs core in the vfs
tree - perhaps something there made /proc/sys/fs/quota vanish.

<checks>

Yup, it happens in the most recent linux-next too.

Ah, it seems to be taking a little vacation over in
/proc/sys/kernel/.  Curious.

Re: 2.6.26-mmotm-0724 - linux-next.git loses /proc/sys/fs/quota, breaks disk quotas

[Valdis.Kletnieks]

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

On Sat, 26 Jul 2008 20:25:25 PDT, Andrew Morton said:

> You need to grab the git tree and use git-bisect.  Then wade through
> all the runtime and compile-time bisection breakage which people
> nowadays seem to find acceptable because git collapses in an
> ignominious heap when these are fixed.

I've bisected through Linus' tree more than once - ISTR that there was
some special funkiness in dealing with trying to bisect through linux-next
because the tree gets redone every night.

If it *was* as easy as Linus's tree to bisect, I'd go ahead and do it.

> > Anybody know where /proc/sys/fs/quota went?

> Nope.  There are pretty extensive changes to procfs core in the vfs
> tree - perhaps something there made /proc/sys/fs/quota vanish.
> 
> <checks>
> 
> Yup, it happens in the most recent linux-next too.
> 
> Ah, it seems to be taking a little vacation over in
> /proc/sys/kernel/.  Curious.

Wow. I never *thought* to look for it there, and neither did my userspace :)




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

Re: 2.6.26-mmotm-0724 - linux-next.git loses /proc/sys/fs/quota, breaks disk quotas

[Roland Dreier]

 > I've bisected through Linus' tree more than once - ISTR that there was
 > some special funkiness in dealing with trying to bisect through linux-next
 > because the tree gets redone every night.

 > If it *was* as easy as Linus's tree to bisect, I'd go ahead and do it.

For a single day's linux-next tree, it should be just like Linus's
tree... just clone

    git://git.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git

and bisect as usual.

The funkiness is that if you say, "linux-next from July 24 worked,
linux-next from July 25 didn't," then it's pretty much hopeless, exactly
because those two trees were built independently.

 - R.

Re: 2.6.26-mmotm-0724 - linux-next.git loses /proc/sys/fs/quota, breaks disk quotas

[Al Viro]

On Sat, Jul 26, 2008 at 11:43:49PM -0400, Valdis.Kletnieks@vt.edu wrote:

> > Ah, it seems to be taking a little vacation over in
> > /proc/sys/kernel/.  Curious.
> 
> Wow. I never *thought* to look for it there, and neither did my userspace :)

Wow, indeed...  Very odd, since there shouldn't be any way for that
to end up there.  Off to try and reproduce it...

[PATCH] Re: 2.6.26-mmotm-0724 - linux-next.git loses /proc/sys/fs/quota, breaks disk quotas

[Al Viro]

On Sun, Jul 27, 2008 at 05:09:08AM +0100, Al Viro wrote:
> On Sat, Jul 26, 2008 at 11:43:49PM -0400, Valdis.Kletnieks@vt.edu wrote:
> 
> > > Ah, it seems to be taking a little vacation over in
> > > /proc/sys/kernel/.  Curious.
> > 
> > Wow. I never *thought* to look for it there, and neither did my userspace :)
> 
> Wow, indeed...  Very odd, since there shouldn't be any way for that
> to end up there.  Off to try and reproduce it...

Wait... I remember fixing exactly that, but the fix is not in there...
Bugger.  Botched "fold on reorder", fortunately still findable in old local
branch.

Anyway, I know what's going on; will test in a few, but I'm fairly sure what it
is.  The lost fix is to have try_attach() walk into the right subdirectory
of what we are attaching, not the first one.

Fun with reordering ;-/  The missing delta, rediffed to the tip of Linus'
tree follows and definitely needs to be applied; will test if that's all
there is as soon as the kernel build finishes...

Sigh... Why does that stuff show up after 10 days in linux-next, about
5 minutes after seeing it merged into mainline?  Murphy's fun...

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 911d846..fe47133 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1680,43 +1680,45 @@ static __init int sysctl_init(void)
 
 core_initcall(sysctl_init);
 
-static int is_branch_in(struct ctl_table *branch, struct ctl_table *table)
+static struct ctl_table *is_branch_in(struct ctl_table *branch,
+				      struct ctl_table *table)
 {
 	struct ctl_table *p;
 	const char *s = branch->procname;
 
 	/* branch should have named subdirectory as its first element */
 	if (!s || !branch->child)
-		return 0;
+		return NULL;
 
 	/* ... and nothing else */
 	if (branch[1].procname || branch[1].ctl_name)
-		return 0;
+		return NULL;
 
 	/* table should contain subdirectory with the same name */
 	for (p = table; p->procname || p->ctl_name; p++) {
 		if (!p->child)
 			continue;
 		if (p->procname && strcmp(p->procname, s) == 0)
-			return 1;
+			return p;
 	}
-	return 0;
+	return NULL;
 }
 
 /* see if attaching q to p would be an improvement */
 static void try_attach(struct ctl_table_header *p, struct ctl_table_header *q)
 {
 	struct ctl_table *to = p->ctl_table, *by = q->ctl_table;
+	struct ctl_table *next;
 	int is_better = 0;
 	int not_in_parent = !p->attached_by;
 
-	while (is_branch_in(by, to)) {
+	while ((next = is_branch_in(by, to)) != NULL) {
 		if (by == q->attached_by)
 			is_better = 1;
 		if (to == p->attached_by)
 			not_in_parent = 1;
 		by = by->child;
-		to = to->child;
+		to = next->child;
 	}
 
 	if (is_better && not_in_parent) {

[PATCH] lost sysctl fix

[Al Viro]

try_attach() should walk into the matching subdirectory, not the first one...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
[and yes, that's what that crap had been]

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 911d846..fe47133 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1680,43 +1680,45 @@ static __init int sysctl_init(void)
 
 core_initcall(sysctl_init);
 
-static int is_branch_in(struct ctl_table *branch, struct ctl_table *table)
+static struct ctl_table *is_branch_in(struct ctl_table *branch,
+				      struct ctl_table *table)
 {
 	struct ctl_table *p;
 	const char *s = branch->procname;
 
 	/* branch should have named subdirectory as its first element */
 	if (!s || !branch->child)
-		return 0;
+		return NULL;
 
 	/* ... and nothing else */
 	if (branch[1].procname || branch[1].ctl_name)
-		return 0;
+		return NULL;
 
 	/* table should contain subdirectory with the same name */
 	for (p = table; p->procname || p->ctl_name; p++) {
 		if (!p->child)
 			continue;
 		if (p->procname && strcmp(p->procname, s) == 0)
-			return 1;
+			return p;
 	}
-	return 0;
+	return NULL;
 }
 
 /* see if attaching q to p would be an improvement */
 static void try_attach(struct ctl_table_header *p, struct ctl_table_header *q)
 {
 	struct ctl_table *to = p->ctl_table, *by = q->ctl_table;
+	struct ctl_table *next;
 	int is_better = 0;
 	int not_in_parent = !p->attached_by;
 
-	while (is_branch_in(by, to)) {
+	while ((next = is_branch_in(by, to)) != NULL) {
 		if (by == q->attached_by)
 			is_better = 1;
 		if (to == p->attached_by)
 			not_in_parent = 1;
 		by = by->child;
-		to = to->child;
+		to = next->child;
 	}
 
 	if (is_better && not_in_parent) {

Re: [PATCH] Re: 2.6.26-mmotm-0724 - linux-next.git loses /proc/sys/fs/quota, breaks disk quotas

[Valdis.Kletnieks]

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

On Sun, 27 Jul 2008 06:17:02 BST, Al Viro said:
> Wait... I remember fixing exactly that, but the fix is not in there...
> Bugger.  Botched "fold on reorder", fortunately still findable in old local
> branch.

I guess we're fortunate that you remembered and found the fix - else this
would quickly have degenerated into a "How did it *ever* work?"..

> Sigh... Why does that stuff show up after 10 days in linux-next, about
> 5 minutes after seeing it merged into mainline?  Murphy's fun...

I have to conclude that I'm the first person who actually tried the quota
support against either a linux-next or -mmotm kernel in the last 2 weeks,
which says something about our testing coverage.

Looking at the patch, I have to wonder what *else* was broken and nobody
noticed...

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

Re: [PATCH] lost sysctl fix

[Ingo Molnar]

* Al Viro <viro@ZenIV.linux.org.uk> wrote:

> try_attach() should walk into the matching subdirectory, not the first one...
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> [and yes, that's what that crap had been]

does this explain the following crash -tip testing just triggered with 
latest -git:

[   12.293289] calling  butterfly_init+0x0/0x14
[   12.298036] BUG: unable to handle kernel NULL pointer dereference at 00000004
[   12.302065] IP: [<c012c169>] __register_sysctl_paths+0x1af/0x253
[   12.308058] *pdpt = 0000000000b81001 *pde = 0000000000000000 
[   12.314809] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[   12.323850] 
[   12.326808] Pid: 1, comm: swapper Not tainted (2.6.26-tip-01710-g55a4e73-dirty #22360)
[   12.340001] EIP: 0060:[<c012c169>] EFLAGS: 00010246 CPU: 1
[   12.350001] EIP is at __register_sysctl_paths+0x1af/0x253
[   12.350001] EAX: 00000001 EBX: f5b7bb60 ECX: c0a40e7e EDX: 00000000
[   12.360002] ESI: 00000000 EDI: f5b8660c EBP: f6c25ed4 ESP: f6c25e98
[   12.370002]  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
[   12.370002] Process swapper (pid: 1, ti=f6c25000 task=f6c40000 task.ti=f6c25000)
[   12.380003] Stack: c0a80b4c c0a7f17c f5b7bb60 f5b7bb90 00000000 c0a7f184 f5aba4d0 f5b7bb64 
[   12.390003]        00000001 00000001 c0a40e7e 00000000 f5b1e840 c082448c f5b475e0 f6c25ee0 
[   12.400003]        c012c230 f5b8676c f6c25ee8 c012c246 f6c25f08 c03d2213 f5b865b0 f5b475e0 
[   12.410004] Call Trace:
[   12.410004]  [<c012c230>] ? register_sysctl_paths+0x23/0x25
[   12.420004]  [<c012c246>] ? register_sysctl_table+0x14/0x16
[   12.430005]  [<c03d2213>] ? parport_device_proc_register+0xb3/0xda
[   12.430005]  [<c03d1863>] ? parport_register_device+0x17e/0x1be
[   12.440005]  [<c05d9661>] ? butterfly_attach+0x96/0x1e9
[   12.450005]  [<c03d1416>] ? parport_register_driver+0x25/0x65
[   12.450005]  [<c0b4f6f9>] ? butterfly_init+0x12/0x14
[   12.459919]  [<c0101042>] ? do_one_initcall+0x42/0x11d
[   12.460006]  [<c0b4f6e7>] ? butterfly_init+0x0/0x14
[   12.469746]  [<c01338d6>] ? start_workqueue_thread+0x22/0x25
[   12.480007]  [<c0133bd2>] ? __create_workqueue_key+0xa9/0x14a
[   12.480007]  [<c0b26607>] ? kernel_init+0x17c/0x1e3
[   12.489747]  [<c0b2648b>] ? kernel_init+0x0/0x1e3
[   12.490007]  [<c01048b7>] ? kernel_thread_helper+0x7/0x10
[   12.500007]  =======================
[   12.500007] Code: 00 75 34 83 7f 2c 00 75 2e 8b 75 f0 eb 1d 83 7e 14 00 74 14 85 c0 74 10 8b 55 ec e8 0d e0 20 00 85 c0 0f 84 87 00 00 00 83 c6 2c <8b> 46 04 85 c0 75 dc 83 3e 00 75 d7 83 7d e8 00 74 15 83 7d e4 
[   12.530008] EIP: [<c012c169>] __register_sysctl_paths+0x1af/0x253 SS:ESP 0068:f6c25e98
[   12.530008] Kernel panic - not syncing: Fatal exception
[   12.535690] Pid: 1, comm: swapper Tainted: G      D   2.6.26-tip-01710-g55a4e73-dirty #22360
[   12.544884]  [<c0126077>] panic+0x44/0xe4
[   12.550001]  [<c0104bff>] oops_end+0x61/0x85
[   12.552430]  [<c010510d>] die+0x5c/0x64
[   12.554614]  [<c0114850>] do_page_fault+0x67f/0x73f
[   12.560635]  [<c013afbb>] ? sysctl_check_table+0x445/0x46e
[   12.567413]  [<c01141d1>] ? do_page_fault+0x0/0x73f
[   12.576820]  [<c07a96b2>] error_code+0x72/0x80
[   12.581092]  [<c012c169>] ? __register_sysctl_paths+0x1af/0x253
[   12.584465]  [<c012c230>] register_sysctl_paths+0x23/0x25
[   12.591447]  [<c012c246>] register_sysctl_table+0x14/0x16
[   12.601870]  [<c03d2213>] parport_device_proc_register+0xb3/0xda
[   12.609130]  [<c03d1863>] parport_register_device+0x17e/0x1be
[   12.615959]  [<c05d9661>] butterfly_attach+0x96/0x1e9
[   12.620001]  [<c03d1416>] parport_register_driver+0x25/0x65
[   12.623174]  [<c0b4f6f9>] butterfly_init+0x12/0x14
[   12.627408]  [<c0101042>] do_one_initcall+0x42/0x11d
[   12.633537]  [<c0b4f6e7>] ? butterfly_init+0x0/0x14
[   12.641475]  [<c01338d6>] ? start_workqueue_thread+0x22/0x25
[   12.650001]  [<c0133bd2>] ? __create_workqueue_key+0xa9/0x14a
[   12.653274]  [<c0b26607>] kernel_init+0x17c/0x1e3
[   12.657343]  [<c0b2648b>] ? kernel_init+0x0/0x1e3
[   12.663027]  [<c01048b7>] kernel_thread_helper+0x7/0x10
[   12.668949]  =======================
[   12.672750] Rebooting in 1 seconds..Press any key to enter the menu

?

	Ingo

Re: [PATCH] lost sysctl fix

[Valdis.Kletnieks]

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

On Sun, 27 Jul 2008 06:31:22 BST, Al Viro said:
> try_attach() should walk into the matching subdirectory, not the first one...
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> [and yes, that's what that crap had been]
> 
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 911d846..fe47133 100644

I just built 26-mmotm-0727 with this patch on top of it, and can confirm
that disk quotas are functioning again.  Thanks, Al.  Feel free to
stick a Tested-By: on there...

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

Re: [PATCH] lost sysctl fix

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Al Viro <viro@ZenIV.linux.org.uk> wrote:
> 
> > try_attach() should walk into the matching subdirectory, not the first one...
> > 
> > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> > ---
> > [and yes, that's what that crap had been]
> 
> does this explain the following crash -tip testing just triggered with 
> latest -git:
> 
> [   12.293289] calling  butterfly_init+0x0/0x14
> [   12.298036] BUG: unable to handle kernel NULL pointer dereference at 00000004
> [   12.302065] IP: [<c012c169>] __register_sysctl_paths+0x1af/0x253
> [   12.308058] *pdpt = 0000000000b81001 *pde = 0000000000000000 
> [   12.314809] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC

your patch fixes it.

Tested-by: Ingo Molnar <mingo@elte.hu>

	Ingo