Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-12 Thread Michael Tokarev
Linus Torvalds wrote:
> 
> On Mon, 11 Dec 2006, Chuck Ebbert wrote:
>> Prevent oops when an app tries to create a pipe while pipefs
>> is not mounted.
> 
> Have you actually seen this, or is this just from looking at code?
> 
> Quite frankly, if "pipe_mnt" is ever NULL, we're dead for lots of other 
> reasons. 
> 
> In fact, pipe_mnt can't be NULL. The way it is initialized is:
> 
>   pipe_mnt = kern_mount(_fs_type);
> 
> and pipe_mnt doesn't even return NULL - it returns an error pointer, so if 
> "kern_mount()" were to have failed, pipe_mnt will be some random invalid 
> pointer that could only be tested with IS_ERR(), not by comparing against 
> NULL.

http://marc.theaimsgroup.com/?t=11651039061=1=2

/mjt
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-12 Thread Michael Tokarev
Linus Torvalds wrote:
 
 On Mon, 11 Dec 2006, Chuck Ebbert wrote:
 Prevent oops when an app tries to create a pipe while pipefs
 is not mounted.
 
 Have you actually seen this, or is this just from looking at code?
 
 Quite frankly, if pipe_mnt is ever NULL, we're dead for lots of other 
 reasons. 
 
 In fact, pipe_mnt can't be NULL. The way it is initialized is:
 
   pipe_mnt = kern_mount(pipe_fs_type);
 
 and pipe_mnt doesn't even return NULL - it returns an error pointer, so if 
 kern_mount() were to have failed, pipe_mnt will be some random invalid 
 pointer that could only be tested with IS_ERR(), not by comparing against 
 NULL.

http://marc.theaimsgroup.com/?t=11651039061r=1w=2

/mjt
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Linus Torvalds


On Mon, 11 Dec 2006, Andrew Morton wrote:
> 
> Looks like this might break pcmcia which for some reason does firmware
> requesting at fs_initcall level (drivers/pcmcia/ds.c).

Ok, that's just strange. 

I think it's fine to do init_pcmcia_bus early to make sure that the PCMCIA 
bus interface is there by the time the driver init stuff happens, but I 
really don't see the point of that firmware load to be there. And all that 
MATCH_FAKE_CIS stuff is about the _devices_, not the PCMCIA bus, so that 
whole thing looks pretty silly. It should be done by the device 
registration (which is obviously device_initcall), not by some bus layer.

Hopefully Dominik can fix whatever up (if it even needs it)

Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Dominik Brodowski
On Mon, Dec 11, 2006 at 06:08:22PM -0800, Andrew Morton wrote:
> > diff --git a/include/linux/init.h b/include/linux/init.h
> > index 5eb5d24..5a593a1 100644
> > --- a/include/linux/init.h
> > +++ b/include/linux/init.h
> > @@ -111,6 +111,7 @@ extern void setup_arch(char **);
> >  #define subsys_initcall_sync(fn)   __define_initcall("4s",fn,4s)
> >  #define fs_initcall(fn)__define_initcall("5",fn,5)
> >  #define fs_initcall_sync(fn)   __define_initcall("5s",fn,5s)
> > +#define rootfs_initcall(fn)
> > __define_initcall("rootfs",fn,rootfs)
> >  #define device_initcall(fn)__define_initcall("6",fn,6)
> >  #define device_initcall_sync(fn)   __define_initcall("6s",fn,6s)
> >  #define late_initcall(fn)  __define_initcall("7",fn,7)
> 
> Looks like this might break pcmcia which for some reason does firmware
> requesting at fs_initcall level (drivers/pcmcia/ds.c).

That codepath is not triggered before device_initcall()s occur. So it's a
non-issue for PCMCIA.

Thanks,
Dominik
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 08:01:40 -0800 (PST)
Linus Torvalds <[EMAIL PROTECTED]> wrote:

> 
> 
> On Mon, 11 Dec 2006, Al Viro wrote:
> 
> > On Mon, Dec 11, 2006 at 02:27:46AM -0800, Andrew Morton wrote:
> > > @@ -115,6 +115,11 @@ extern void setup_arch(char **);
> > >  #define device_initcall_sync(fn) __define_initcall("6s",fn,6s)
> > >  #define late_initcall(fn)__define_initcall("7",fn,7)
> > >  #define late_initcall_sync(fn)   __define_initcall("7s",fn,7s)
> > > +#define populate_rootfs_initcall(fn) __define_initcall("8",fn,8)
> > > +#define populate_rootfs_initcall_sync(fn) __define_initcall("8s",fn,8s)
> > > +#define rootfs_neeeded_initcall(fn)  __define_initcall("9",fn,9)
> > > +#define rootfs_neeeded_initcall_sync(fn) __define_initcall("9s",fn,9s)
> > 
> > Ewww  After module_init()?  Please, don't.  Come on, if it can
> > be a module, it _must_ be ready to run late in the game.
> 
> Yeah, I think you should just run "populate_rootfs()" just before 
> "module_init" (which is the same as "device_initcall()").
> 
> So perhaps somethign like this? (totally untested)
> 
> Btw, if the linker sorts sections some way (does it?) we could probably 
> just make the vmlinux.lds.S file do
> 
>   *(.initcall*.init)
> 
> or something, and then just let special cases like this use
> 
>   __initcall(myfn, 5.1);
> 
> to show that it's between levels 5 and 6. But that would depend on the 
> linker section beign sorted alphabetically. Does anybody know if the 
> linker sorts these things somehow?
> 
> This patch is totally untested, but it looks obvious. It just says that 
> we'll populate rootfs _after_ we've done the fs-level initcalls, but 
> before we do any actual "device" initcalls.
> 
> If any really core stuff needs user-land - tough titties, as they say.
> 
>   Linus
> 
> diff --git a/include/asm-generic/vmlinux.lds.h 
> b/include/asm-generic/vmlinux.lds.h
> index 6e9fceb..7437cca 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -242,6 +242,7 @@
>   *(.initcall4s.init) \
>   *(.initcall5.init)  \
>   *(.initcall5s.init) \
> + *(.initcallrootfs.init) \
>   *(.initcall6.init)  \
>   *(.initcall6s.init) \
>   *(.initcall7.init)  \
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 5eb5d24..5a593a1 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -111,6 +111,7 @@ extern void setup_arch(char **);
>  #define subsys_initcall_sync(fn) __define_initcall("4s",fn,4s)
>  #define fs_initcall(fn)  __define_initcall("5",fn,5)
>  #define fs_initcall_sync(fn) __define_initcall("5s",fn,5s)
> +#define rootfs_initcall(fn)  __define_initcall("rootfs",fn,rootfs)
>  #define device_initcall(fn)  __define_initcall("6",fn,6)
>  #define device_initcall_sync(fn) __define_initcall("6s",fn,6s)
>  #define late_initcall(fn)__define_initcall("7",fn,7)

Looks like this might break pcmcia which for some reason does firmware
requesting at fs_initcall level (drivers/pcmcia/ds.c).


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Benjamin Herrenschmidt

> So it makes perfect sense to say
> 
>"you won't be getting any notification by anything built-in, until 
> 'device_initcall' (which is the default module_init, of course)".
> 
> which in the case of certain drivers obviously _does_ mean that they had 
> better not try to use any early initcalls to load firmware.

And that will fix some other issues I think I've seen (a while ago, I
might have a memory mixup here) related to /sbin/hotplug being called
before /dev/null & /dev/zero are initialized (they are fs_initcall). At
least with that patch, it won't happen.

Ben.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Linus Torvalds


On Mon, 11 Dec 2006, Al Viro wrote:
> 
> FWIW, I really think that this sort of bugs ("oh, I call hotplug,
> rootfs is there but kernel is not ready, woe is me") clearly show
> that many, _many_ users of hotplug are BS.  The reason is simple -
> if we have a call of hotplug that early, we have a driver that lives
> with hotplug failures _and_ with different behaviour in built-in
> and modular cases.  I would argue that any such driver is broken.

Now, you are probably right that many of them are unnecessary, but I don't 
agree in _general_. 

It's perfectly normal behaviour to say "I discovered this device".

The fact that device discovery happens during early bootup and nobody even 
_cares_ (because early bootup ends up enumerating all discovered devices 
on its own) is a separate issue. The fact that many distros don't care 
during early bootup doesn't mean that they don't care later on.

Another reason for "unnecessary" hotplug events is that generic functions 
like "I added a bus" happen both for static buses _and_ for "hey, somebody 
loaded a module that found this bus". Again, the static case may be 
something that people don't _care_ about, but we should (a) use common 
code and (b) be consistent, so we do a hotplug event regardless.

That said, I definitely agree that people shouldn't expect hotplug events 
to be delievered too early. If it's something that runs in a 
core_initcall, and is compiled in, no way in *hell* should we actually 
expose that to anything - it's just too early.

So it makes perfect sense to say

   "you won't be getting any notification by anything built-in, until 
'device_initcall' (which is the default module_init, of course)".

which in the case of certain drivers obviously _does_ mean that they had 
better not try to use any early initcalls to load firmware.

Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 08:01:40AM -0800, Linus Torvalds wrote:
> 
> 
> On Mon, 11 Dec 2006, Al Viro wrote:
> 
> > On Mon, Dec 11, 2006 at 02:27:46AM -0800, Andrew Morton wrote:
> > > @@ -115,6 +115,11 @@ extern void setup_arch(char **);
> > >  #define device_initcall_sync(fn) __define_initcall("6s",fn,6s)
> > >  #define late_initcall(fn)__define_initcall("7",fn,7)
> > >  #define late_initcall_sync(fn)   __define_initcall("7s",fn,7s)
> > > +#define populate_rootfs_initcall(fn) __define_initcall("8",fn,8)
> > > +#define populate_rootfs_initcall_sync(fn) __define_initcall("8s",fn,8s)
> > > +#define rootfs_neeeded_initcall(fn)  __define_initcall("9",fn,9)
> > > +#define rootfs_neeeded_initcall_sync(fn) __define_initcall("9s",fn,9s)
> > 
> > Ewww  After module_init()?  Please, don't.  Come on, if it can
> > be a module, it _must_ be ready to run late in the game.
> 
> Yeah, I think you should just run "populate_rootfs()" just before 
> "module_init" (which is the same as "device_initcall()").
 
> If any really core stuff needs user-land - tough titties, as they say.

FWIW, I really think that this sort of bugs ("oh, I call hotplug,
rootfs is there but kernel is not ready, woe is me") clearly show
that many, _many_ users of hotplug are BS.  The reason is simple -
if we have a call of hotplug that early, we have a driver that lives
with hotplug failures _and_ with different behaviour in built-in
and modular cases.  I would argue that any such driver is broken.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Linus Torvalds


On Mon, 11 Dec 2006, Al Viro wrote:

> On Mon, Dec 11, 2006 at 02:27:46AM -0800, Andrew Morton wrote:
> > @@ -115,6 +115,11 @@ extern void setup_arch(char **);
> >  #define device_initcall_sync(fn)   __define_initcall("6s",fn,6s)
> >  #define late_initcall(fn)  __define_initcall("7",fn,7)
> >  #define late_initcall_sync(fn) __define_initcall("7s",fn,7s)
> > +#define populate_rootfs_initcall(fn)   __define_initcall("8",fn,8)
> > +#define populate_rootfs_initcall_sync(fn) __define_initcall("8s",fn,8s)
> > +#define rootfs_neeeded_initcall(fn)__define_initcall("9",fn,9)
> > +#define rootfs_neeeded_initcall_sync(fn) __define_initcall("9s",fn,9s)
> 
> Ewww  After module_init()?  Please, don't.  Come on, if it can
> be a module, it _must_ be ready to run late in the game.

Yeah, I think you should just run "populate_rootfs()" just before 
"module_init" (which is the same as "device_initcall()").

So perhaps somethign like this? (totally untested)

Btw, if the linker sorts sections some way (does it?) we could probably 
just make the vmlinux.lds.S file do

*(.initcall*.init)

or something, and then just let special cases like this use

__initcall(myfn, 5.1);

to show that it's between levels 5 and 6. But that would depend on the 
linker section beign sorted alphabetically. Does anybody know if the 
linker sorts these things somehow?

This patch is totally untested, but it looks obvious. It just says that 
we'll populate rootfs _after_ we've done the fs-level initcalls, but 
before we do any actual "device" initcalls.

If any really core stuff needs user-land - tough titties, as they say.

Linus

diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index 6e9fceb..7437cca 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -242,6 +242,7 @@
*(.initcall4s.init) \
*(.initcall5.init)  \
*(.initcall5s.init) \
+   *(.initcallrootfs.init) \
*(.initcall6.init)  \
*(.initcall6s.init) \
*(.initcall7.init)  \
diff --git a/include/linux/init.h b/include/linux/init.h
index 5eb5d24..5a593a1 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -111,6 +111,7 @@ extern void setup_arch(char **);
 #define subsys_initcall_sync(fn)   __define_initcall("4s",fn,4s)
 #define fs_initcall(fn)__define_initcall("5",fn,5)
 #define fs_initcall_sync(fn)   __define_initcall("5s",fn,5s)
+#define rootfs_initcall(fn)__define_initcall("rootfs",fn,rootfs)
 #define device_initcall(fn)__define_initcall("6",fn,6)
 #define device_initcall_sync(fn)   __define_initcall("6s",fn,6s)
 #define late_initcall(fn)  __define_initcall("7",fn,7)
diff --git a/init/initramfs.c b/init/initramfs.c
index 85f0403..4fa0f79 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -526,7 +526,7 @@ static void __init free_initrd(void)
 
 #endif
 
-void __init populate_rootfs(void)
+static int __init populate_rootfs(void)
 {
char *err = unpack_to_rootfs(__initramfs_start,
 __initramfs_end - __initramfs_start, 0);
@@ -544,7 +544,7 @@ void __init populate_rootfs(void)
unpack_to_rootfs((char *)initrd_start,
initrd_end - initrd_start, 0);
free_initrd();
-   return;
+   return 0;
}
printk("it isn't (%s); looks like an initrd\n", err);
fd = sys_open("/initrd.image", O_WRONLY|O_CREAT, 0700);
@@ -565,4 +565,6 @@ void __init populate_rootfs(void)
 #endif
}
 #endif
+   return 0;
 }
+rootfs_initcall(populate_rootfs);
diff --git a/init/main.c b/init/main.c
index 036f97c..ae12fa3 100644
--- a/init/main.c
+++ b/init/main.c
@@ -94,7 +94,6 @@ extern void pidmap_init(void);
 extern void prio_tree_init(void);
 extern void radix_tree_init(void);
 extern void free_initmem(void);
-extern void populate_rootfs(void);
 extern void driver_init(void);
 extern void prepare_namespace(void);
 #ifdef CONFIG_ACPI
@@ -739,12 +738,6 @@ static int init(void * unused)
 
cpuset_init_smp();
 
-   /*
-* Do this before initcalls, because some drivers want to access
-* firmware files.
-*/
-   populate_rootfs();
-
do_basic_setup();
 
/*
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Linus Torvalds


On Mon, 11 Dec 2006, Chuck Ebbert wrote:
>
> Prevent oops when an app tries to create a pipe while pipefs
> is not mounted.

Have you actually seen this, or is this just from looking at code?

Quite frankly, if "pipe_mnt" is ever NULL, we're dead for lots of other 
reasons. 

In fact, pipe_mnt can't be NULL. The way it is initialized is:

pipe_mnt = kern_mount(_fs_type);

and pipe_mnt doesn't even return NULL - it returns an error pointer, so if 
"kern_mount()" were to have failed, pipe_mnt will be some random invalid 
pointer that could only be tested with IS_ERR(), not by comparing against 
NULL.

But more fundamentally - we might as well oops. We need to panic or oops 
or do _something_ bad at some point anyway, because it's MUCH better to 
fail spectacularly than it would be to just silently fail without a pipe.

Hmm?

Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Jeff Garzik

Andrew Morton wrote:

A heck of a lot of things can trigger an /sbin/hotplug run.  It could well
be that Andrew's driver didn't want to run hotplug at all, but the kernel
did it anwyay.  But as soon as the script appeared at /sbin/hotplug, and it
happened to use foo|bar: boom.


In fact,  things run /sbin/hotplug but don't necessarily need to :/

I think bcrl used a counter one time, and saw over 1000 /sbin/hotplug 
executions during a single boot.


Jeff


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 11:48:57AM +0100, Olaf Hering wrote:
> On Mon, Dec 11, Chuck Ebbert wrote:
> 
> > Why not create a new initcall category for things that must run before
> > early userspace?
> 
> Why do you want to continue with papering over the root cause?
> Pick some janitor, let him write something that implements something
> like make style dependencies for initcalls.
> Then you can get rid of all that foo_initcall stuff.
> It surely needs work to get all that done.

I would argue that _this_ is papering over the root cause.  Namely,
too complex ordering requirements.  Growing a technics for allowing
them to fester is an odd solution...
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Olaf Hering
On Mon, Dec 11, Chuck Ebbert wrote:

> Why not create a new initcall category for things that must run before
> early userspace?

Why do you want to continue with papering over the root cause?
Pick some janitor, let him write something that implements something
like make style dependencies for initcalls.
Then you can get rid of all that foo_initcall stuff.
It surely needs work to get all that done.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 02:34:36AM -0800, Andrew Morton wrote:

> There are plenty of drivers in there using subsys_initcall, arch_initcall,
> postcore_initcall, core_initcall and even one pure_initcall.
> 
> Heaven knows why.  They're drivers :(
 
> A heck of a lot of things can trigger an /sbin/hotplug run.  It could well
> be that Andrew's driver didn't want to run hotplug at all, but the kernel
> did it anwyay.  But as soon as the script appeared at /sbin/hotplug, and it
> happened to use foo|bar: boom.

Again, I agree with moving pipes up, but keep in mind that any caller
of /sbin/hotplug either
* doesn't need it or
* really handles failure or
* really should _not_ be run before populate_rootfs()
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 02:27:46AM -0800, Andrew Morton wrote:
> @@ -115,6 +115,11 @@ extern void setup_arch(char **);
>  #define device_initcall_sync(fn) __define_initcall("6s",fn,6s)
>  #define late_initcall(fn)__define_initcall("7",fn,7)
>  #define late_initcall_sync(fn)   __define_initcall("7s",fn,7s)
> +#define populate_rootfs_initcall(fn) __define_initcall("8",fn,8)
> +#define populate_rootfs_initcall_sync(fn) __define_initcall("8s",fn,8s)
> +#define rootfs_neeeded_initcall(fn)  __define_initcall("9",fn,9)
> +#define rootfs_neeeded_initcall_sync(fn) __define_initcall("9s",fn,9s)

Ewww  After module_init()?  Please, don't.  Come on, if it can
be a module, it _must_ be ready to run late in the game.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 10:22:07 +
Al Viro <[EMAIL PROTECTED]> wrote:

> On Mon, Dec 11, 2006 at 02:17:18AM -0800, Andrew Morton wrote:
> > I think we should aim to have as many subsystems ready to go as possible -
> > ideally all of them.  Right now we can potentially run userspace before
> > AIO, posix-timers, message-queues, BIO, networking, etc are ready to run.
> > 
> > It looks to be pretty easy to fix...
> > 
> > > As for that example, I'd love to see specifics - which driver triggers
> > > hotplug?  Presumably it happens from an initcall, so we also have 
> > > something
> > > fishy here...
> > 
> > I don't know in this case - but firmware loading from a statically-linked
> > driver is a legit thing to do.
> 
> Umm... statically linked driver that might want firmware shouldn't precede
> the subsystems unless something is seriously wrong with priorities...

There are plenty of drivers in there using subsys_initcall, arch_initcall,
postcore_initcall, core_initcall and even one pure_initcall.

Heaven knows why.  They're drivers :(

> IOW, I still wonder what's really going on - pipes are fs_initcall() and
> any hardware stuff ought to be simple module_init().  So something fishy
> is going on, regardless of anything else.

A heck of a lot of things can trigger an /sbin/hotplug run.  It could well
be that Andrew's driver didn't want to run hotplug at all, but the kernel
did it anwyay.  But as soon as the script appeared at /sbin/hotplug, and it
happened to use foo|bar: boom.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 02:17:18 -0800
Andrew Morton <[EMAIL PROTECTED]> wrote:

> > Said that, I think that pipes should be initialized early.
> 
> Judging by the comment there, the only reason we prepare the rootfs prior
> to running initcalls is for firmware.  So the sequence
> 
>   run initcalls
>   populate rootfs
>   run initcalls which want to access files
> 
> fixes everything?

Like this...  The other part is hunting down all those drivers which want
to access the filesystem at init time.


 include/asm-generic/vmlinux.lds.h |6 +-
 include/linux/init.h  |5 +
 init/initramfs.c  |3 ++-
 init/main.c   |7 ---
 4 files changed, 12 insertions(+), 9 deletions(-)

diff -puN init/main.c~dont-run-userspace-until-initcalls-have-completed 
init/main.c
--- a/init/main.c~dont-run-userspace-until-initcalls-have-completed
+++ a/init/main.c
@@ -94,7 +94,6 @@ extern void pidmap_init(void);
 extern void prio_tree_init(void);
 extern void radix_tree_init(void);
 extern void free_initmem(void);
-extern void populate_rootfs(void);
 extern void driver_init(void);
 extern void prepare_namespace(void);
 #ifdef CONFIG_ACPI
@@ -739,12 +738,6 @@ static int init(void * unused)
 
cpuset_init_smp();
 
-   /*
-* Do this before initcalls, because some drivers want to access
-* firmware files.
-*/
-   populate_rootfs();
-
do_basic_setup();
 
/*
diff -puN 
include/linux/init.h~dont-run-userspace-until-initcalls-have-completed 
include/linux/init.h
--- a/include/linux/init.h~dont-run-userspace-until-initcalls-have-completed
+++ a/include/linux/init.h
@@ -115,6 +115,11 @@ extern void setup_arch(char **);
 #define device_initcall_sync(fn)   __define_initcall("6s",fn,6s)
 #define late_initcall(fn)  __define_initcall("7",fn,7)
 #define late_initcall_sync(fn) __define_initcall("7s",fn,7s)
+#define populate_rootfs_initcall(fn)   __define_initcall("8",fn,8)
+#define populate_rootfs_initcall_sync(fn) __define_initcall("8s",fn,8s)
+#define rootfs_neeeded_initcall(fn)__define_initcall("9",fn,9)
+#define rootfs_neeeded_initcall_sync(fn) __define_initcall("9s",fn,9s)
+
 
 #define __initcall(fn) device_initcall(fn)
 
diff -puN 
include/asm-generic/vmlinux.lds.h~dont-run-userspace-until-initcalls-have-completed
 include/asm-generic/vmlinux.lds.h
--- 
a/include/asm-generic/vmlinux.lds.h~dont-run-userspace-until-initcalls-have-completed
+++ a/include/asm-generic/vmlinux.lds.h
@@ -245,5 +245,9 @@
*(.initcall6.init)  \
*(.initcall6s.init) \
*(.initcall7.init)  \
-   *(.initcall7s.init)
+   *(.initcall7s.init) \
+   *(.initcall8.init)  \
+   *(.initcall8s.init) \
+   *(.initcall9.init)  \
+   *(.initcall9s.init)
 
diff -puN init/initramfs.c~dont-run-userspace-until-initcalls-have-completed 
init/initramfs.c
--- a/init/initramfs.c~dont-run-userspace-until-initcalls-have-completed
+++ a/init/initramfs.c
@@ -526,7 +526,7 @@ static void __init free_initrd(void)
 
 #endif
 
-void __init populate_rootfs(void)
+static int __init populate_rootfs(void)
 {
char *err = unpack_to_rootfs(__initramfs_start,
 __initramfs_end - __initramfs_start, 0);
@@ -566,3 +566,4 @@ void __init populate_rootfs(void)
}
 #endif
 }
+populate_rootfs_initcall(populate_rootfs);
_

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 02:17:18AM -0800, Andrew Morton wrote:
> I think we should aim to have as many subsystems ready to go as possible -
> ideally all of them.  Right now we can potentially run userspace before
> AIO, posix-timers, message-queues, BIO, networking, etc are ready to run.
> 
> It looks to be pretty easy to fix...
> 
> > As for that example, I'd love to see specifics - which driver triggers
> > hotplug?  Presumably it happens from an initcall, so we also have something
> > fishy here...
> 
> I don't know in this case - but firmware loading from a statically-linked
> driver is a legit thing to do.

Umm... statically linked driver that might want firmware shouldn't precede
the subsystems unless something is seriously wrong with priorities...

IOW, I still wonder what's really going on - pipes are fs_initcall() and
any hardware stuff ought to be simple module_init().  So something fishy
is going on, regardless of anything else.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 10:03:01 +
Al Viro <[EMAIL PROTECTED]> wrote:

> On Mon, Dec 11, 2006 at 01:47:27AM -0800, Andrew Morton wrote:
> > - populate_rootfs() puts stuff into the filesystem
> > 
> > - we then run initcalls.
> > 
> > - an initcall runs /sbin/hotplug.
> > 
> > We're now running userspace before all the initcalls have been executed. 
> > Hence we're trying to run userspace when potentially none of "grep
> > _initcall */*.c" has been executed.  It isn't a kernel yet...
> 
> That's... arguable.  We certainly don't need lots and lots of initcalls
> to be able to run userland code.  Which ones are missing in your opinion?

I think we should aim to have as many subsystems ready to go as possible -
ideally all of them.  Right now we can potentially run userspace before
AIO, posix-timers, message-queues, BIO, networking, etc are ready to run.

It looks to be pretty easy to fix...

> As for that example, I'd love to see specifics - which driver triggers
> hotplug?  Presumably it happens from an initcall, so we also have something
> fishy here...

I don't know in this case - but firmware loading from a statically-linked
driver is a legit thing to do.

> Said that, I think that pipes should be initialized early.

Judging by the comment there, the only reason we prepare the rootfs prior
to running initcalls is for firmware.  So the sequence

run initcalls
populate rootfs
run initcalls which want to access files

fixes everything?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 01:47:27AM -0800, Andrew Morton wrote:
> - populate_rootfs() puts stuff into the filesystem
> 
> - we then run initcalls.
> 
> - an initcall runs /sbin/hotplug.
> 
> We're now running userspace before all the initcalls have been executed. 
> Hence we're trying to run userspace when potentially none of "grep
> _initcall */*.c" has been executed.  It isn't a kernel yet...

That's... arguable.  We certainly don't need lots and lots of initcalls
to be able to run userland code.  Which ones are missing in your opinion?

As for that example, I'd love to see specifics - which driver triggers
hotplug?  Presumably it happens from an initcall, so we also have something
fishy here...

Said that, I think that pipes should be initialized early.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 09:33:15 +
Al Viro <[EMAIL PROTECTED]> wrote:

> On Mon, Dec 11, 2006 at 01:25:45AM -0800, Andrew Morton wrote:
> > On Mon, 11 Dec 2006 09:21:30 +
> > Al Viro <[EMAIL PROTECTED]> wrote:
> > 
> > > On Mon, Dec 11, 2006 at 01:13:27AM -0800, Andrew Morton wrote:
> > > > On Mon, 11 Dec 2006 00:55:57 -0800
> > > > Andrew Morton <[EMAIL PROTECTED]> wrote:
> > > > 
> > > > > I think the bug really is the running of populate_rootfs() before 
> > > > > running
> > > > > the initcalls, in init/main.c:init().  It's just more sensible to 
> > > > > start
> > > > > running userspace after the initcalls have been run.  
> > > > > Statically-linked
> > > > > drivers which want to load firmware files will lose.  To fix that 
> > > > > we'd need
> > > > > a new callback.  It could be with a new linker section or perhaps 
> > > > > simply a
> > > > > notifier chain.
> > > > 
> > > > hm, actually...  Add two new initcall levels, one for populate_rootfs() 
> > > > and
> > > > one for things which want to come after it (ie: drivers which want to
> > > > access the filesytem):
> > > 
> > > IMO we should just call pipe (and socket) initialization directly at
> > > the same level as slab, task, dcache, etc.
> > 
> > spose that would work.  But what other initcall-initialised things are not
> > yet available when populate_rootfs() runs?
> > 
> > 
> > 
> 
> Explain, please...

- populate_rootfs() puts stuff into the filesystem

- we then run initcalls.

- an initcall runs /sbin/hotplug.

We're now running userspace before all the initcalls have been executed. 
Hence we're trying to run userspace when potentially none of "grep
_initcall */*.c" has been executed.  It isn't a kernel yet...

See http://marc.theaimsgroup.com/?l=linux-kernel=116510389000878=2
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Chuck Ebbert
In-Reply-To: <[EMAIL PROTECTED]>

On Mon, 11 Dec 2006 00:55:57 -0800, Andrew Morton wrote:

> > Prevent oops when an app tries to create a pipe while pipefs
> > is not mounted.
> 
> That's pretty lame.  It means that pipes just won't work, so people who are
> using pipes in their initramfs setups will just get mysterious failures
> running userspace on a crippled kernel.

I know, I just wanted to keep the issue alive. :)

> I think the bug really is the running of populate_rootfs() before running
> the initcalls, in init/main.c:init().  It's just more sensible to start
> running userspace after the initcalls have been run.  Statically-linked
> drivers which want to load firmware files will lose.  To fix that we'd need
> a new callback.  It could be with a new linker section or perhaps simply a
> notifier chain.

Why not create a new initcall category for things that must run before
early userspace?

-- 
MBTI: IXTP

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Michael Tokarev
Al Viro wrote:
> On Mon, Dec 11, 2006 at 03:27:37AM -0500, Chuck Ebbert wrote:
>> Prevent oops when an app tries to create a pipe while pipefs
>> is not mounted.
[]
> That makes no sense at all.  pipe_mnt is not created by userland
> mount; it's created by init_pipe_fs() and we'd bloody better
> have it done before any applications get a chance to run.
> 
> Please, explain how the hell did you manage to get a perverted
> config where that is not true.  In the meanwhile the patch is
> NAKed.

See for example http://marc.theaimsgroup.com/?t=11651039061=1=2

Thanks.

/mjt
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 09:21:30 +
Al Viro <[EMAIL PROTECTED]> wrote:

> On Mon, Dec 11, 2006 at 01:13:27AM -0800, Andrew Morton wrote:
> > On Mon, 11 Dec 2006 00:55:57 -0800
> > Andrew Morton <[EMAIL PROTECTED]> wrote:
> > 
> > > I think the bug really is the running of populate_rootfs() before running
> > > the initcalls, in init/main.c:init().  It's just more sensible to start
> > > running userspace after the initcalls have been run.  Statically-linked
> > > drivers which want to load firmware files will lose.  To fix that we'd 
> > > need
> > > a new callback.  It could be with a new linker section or perhaps simply a
> > > notifier chain.
> > 
> > hm, actually...  Add two new initcall levels, one for populate_rootfs() and
> > one for things which want to come after it (ie: drivers which want to
> > access the filesytem):
> 
> IMO we should just call pipe (and socket) initialization directly at
> the same level as slab, task, dcache, etc.

spose that would work.  But what other initcall-initialised things are not
yet available when populate_rootfs() runs?




> This is basic stuff needed to get operational kernel.  We could try
> to put that on an additional initcall level, but then we ought to
> take the rest of basic setup there as well.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 01:25:45AM -0800, Andrew Morton wrote:
> On Mon, 11 Dec 2006 09:21:30 +
> Al Viro <[EMAIL PROTECTED]> wrote:
> 
> > On Mon, Dec 11, 2006 at 01:13:27AM -0800, Andrew Morton wrote:
> > > On Mon, 11 Dec 2006 00:55:57 -0800
> > > Andrew Morton <[EMAIL PROTECTED]> wrote:
> > > 
> > > > I think the bug really is the running of populate_rootfs() before 
> > > > running
> > > > the initcalls, in init/main.c:init().  It's just more sensible to start
> > > > running userspace after the initcalls have been run.  Statically-linked
> > > > drivers which want to load firmware files will lose.  To fix that we'd 
> > > > need
> > > > a new callback.  It could be with a new linker section or perhaps 
> > > > simply a
> > > > notifier chain.
> > > 
> > > hm, actually...  Add two new initcall levels, one for populate_rootfs() 
> > > and
> > > one for things which want to come after it (ie: drivers which want to
> > > access the filesytem):
> > 
> > IMO we should just call pipe (and socket) initialization directly at
> > the same level as slab, task, dcache, etc.
> 
> spose that would work.  But what other initcall-initialised things are not
> yet available when populate_rootfs() runs?
> 
> 
> 

Explain, please...
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 01:13:27AM -0800, Andrew Morton wrote:
> On Mon, 11 Dec 2006 00:55:57 -0800
> Andrew Morton <[EMAIL PROTECTED]> wrote:
> 
> > I think the bug really is the running of populate_rootfs() before running
> > the initcalls, in init/main.c:init().  It's just more sensible to start
> > running userspace after the initcalls have been run.  Statically-linked
> > drivers which want to load firmware files will lose.  To fix that we'd need
> > a new callback.  It could be with a new linker section or perhaps simply a
> > notifier chain.
> 
> hm, actually...  Add two new initcall levels, one for populate_rootfs() and
> one for things which want to come after it (ie: drivers which want to
> access the filesytem):

IMO we should just call pipe (and socket) initialization directly at
the same level as slab, task, dcache, etc.

This is basic stuff needed to get operational kernel.  We could try
to put that on an additional initcall level, but then we ought to
take the rest of basic setup there as well.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 00:55:57 -0800
Andrew Morton <[EMAIL PROTECTED]> wrote:

> I think the bug really is the running of populate_rootfs() before running
> the initcalls, in init/main.c:init().  It's just more sensible to start
> running userspace after the initcalls have been run.  Statically-linked
> drivers which want to load firmware files will lose.  To fix that we'd need
> a new callback.  It could be with a new linker section or perhaps simply a
> notifier chain.

hm, actually...  Add two new initcall levels, one for populate_rootfs() and
one for things which want to come after it (ie: drivers which want to
access the filesytem):

 #define core_initcall(fn)  __define_initcall("1",fn,1)
 #define core_initcall_sync(fn) __define_initcall("1s",fn,1s)
 #define postcore_initcall(fn)  __define_initcall("2",fn,2)
 #define postcore_initcall_sync(fn) __define_initcall("2s",fn,2s)
 #define arch_initcall(fn)  __define_initcall("3",fn,3)
 #define arch_initcall_sync(fn) __define_initcall("3s",fn,3s)
 #define subsys_initcall(fn)__define_initcall("4",fn,4)
 #define subsys_initcall_sync(fn)   __define_initcall("4s",fn,4s)
 #define fs_initcall(fn)__define_initcall("5",fn,5)
 #define fs_initcall_sync(fn)   __define_initcall("5s",fn,5s)
 #define device_initcall(fn)__define_initcall("6",fn,6)
 #define device_initcall_sync(fn)   __define_initcall("6s",fn,6s)
 #define late_initcall(fn)  __define_initcall("7",fn,7)
 #define late_initcall_sync(fn) __define_initcall("7s",fn,7s)
+#define populate_rootfs_initcall ...
+#define post_populate_rootfs_initcall ...

?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 03:27:37AM -0500, Chuck Ebbert wrote:
> Prevent oops when an app tries to create a pipe while pipefs
> is not mounted.
> 
> Signed-off-by: Chuck Ebbert <[EMAIL PROTECTED]>

That makes no sense at all.  pipe_mnt is not created by userland
mount; it's created by init_pipe_fs() and we'd bloody better
have it done before any applications get a chance to run.

Please, explain how the hell did you manage to get a perverted
config where that is not true.  In the meanwhile the patch is
NAKed.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 03:27:37 -0500
Chuck Ebbert <[EMAIL PROTECTED]> wrote:

> Prevent oops when an app tries to create a pipe while pipefs
> is not mounted.
> 
> Signed-off-by: Chuck Ebbert <[EMAIL PROTECTED]>
> 
> --- 2.6.19.1-pre1-32.orig/fs/pipe.c
> +++ 2.6.19.1-pre1-32/fs/pipe.c
> @@ -839,9 +839,11 @@ static struct dentry_operations pipefs_d
>  
>  static struct inode * get_pipe_inode(void)
>  {
> - struct inode *inode = new_inode(pipe_mnt->mnt_sb);
> + struct inode *inode = NULL;
>   struct pipe_inode_info *pipe;
>  
> + if (pipe_mnt)
> + inode = new_inode(pipe_mnt->mnt_sb);
>   if (!inode)
>   goto fail_inode;
>  

That's pretty lame.  It means that pipes just won't work, so people who are
using pipes in their initramfs setups will just get mysterious failures
running userspace on a crippled kernel.

I think the bug really is the running of populate_rootfs() before running
the initcalls, in init/main.c:init().  It's just more sensible to start
running userspace after the initcalls have been run.  Statically-linked
drivers which want to load firmware files will lose.  To fix that we'd need
a new callback.  It could be with a new linker section or perhaps simply a
notifier chain.



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 03:27:37 -0500
Chuck Ebbert [EMAIL PROTECTED] wrote:

 Prevent oops when an app tries to create a pipe while pipefs
 is not mounted.
 
 Signed-off-by: Chuck Ebbert [EMAIL PROTECTED]
 
 --- 2.6.19.1-pre1-32.orig/fs/pipe.c
 +++ 2.6.19.1-pre1-32/fs/pipe.c
 @@ -839,9 +839,11 @@ static struct dentry_operations pipefs_d
  
  static struct inode * get_pipe_inode(void)
  {
 - struct inode *inode = new_inode(pipe_mnt-mnt_sb);
 + struct inode *inode = NULL;
   struct pipe_inode_info *pipe;
  
 + if (pipe_mnt)
 + inode = new_inode(pipe_mnt-mnt_sb);
   if (!inode)
   goto fail_inode;
  

That's pretty lame.  It means that pipes just won't work, so people who are
using pipes in their initramfs setups will just get mysterious failures
running userspace on a crippled kernel.

I think the bug really is the running of populate_rootfs() before running
the initcalls, in init/main.c:init().  It's just more sensible to start
running userspace after the initcalls have been run.  Statically-linked
drivers which want to load firmware files will lose.  To fix that we'd need
a new callback.  It could be with a new linker section or perhaps simply a
notifier chain.



-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 03:27:37AM -0500, Chuck Ebbert wrote:
 Prevent oops when an app tries to create a pipe while pipefs
 is not mounted.
 
 Signed-off-by: Chuck Ebbert [EMAIL PROTECTED]

That makes no sense at all.  pipe_mnt is not created by userland
mount; it's created by init_pipe_fs() and we'd bloody better
have it done before any applications get a chance to run.

Please, explain how the hell did you manage to get a perverted
config where that is not true.  In the meanwhile the patch is
NAKed.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 00:55:57 -0800
Andrew Morton [EMAIL PROTECTED] wrote:

 I think the bug really is the running of populate_rootfs() before running
 the initcalls, in init/main.c:init().  It's just more sensible to start
 running userspace after the initcalls have been run.  Statically-linked
 drivers which want to load firmware files will lose.  To fix that we'd need
 a new callback.  It could be with a new linker section or perhaps simply a
 notifier chain.

hm, actually...  Add two new initcall levels, one for populate_rootfs() and
one for things which want to come after it (ie: drivers which want to
access the filesytem):

 #define core_initcall(fn)  __define_initcall(1,fn,1)
 #define core_initcall_sync(fn) __define_initcall(1s,fn,1s)
 #define postcore_initcall(fn)  __define_initcall(2,fn,2)
 #define postcore_initcall_sync(fn) __define_initcall(2s,fn,2s)
 #define arch_initcall(fn)  __define_initcall(3,fn,3)
 #define arch_initcall_sync(fn) __define_initcall(3s,fn,3s)
 #define subsys_initcall(fn)__define_initcall(4,fn,4)
 #define subsys_initcall_sync(fn)   __define_initcall(4s,fn,4s)
 #define fs_initcall(fn)__define_initcall(5,fn,5)
 #define fs_initcall_sync(fn)   __define_initcall(5s,fn,5s)
 #define device_initcall(fn)__define_initcall(6,fn,6)
 #define device_initcall_sync(fn)   __define_initcall(6s,fn,6s)
 #define late_initcall(fn)  __define_initcall(7,fn,7)
 #define late_initcall_sync(fn) __define_initcall(7s,fn,7s)
+#define populate_rootfs_initcall ...
+#define post_populate_rootfs_initcall ...

?
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 01:13:27AM -0800, Andrew Morton wrote:
 On Mon, 11 Dec 2006 00:55:57 -0800
 Andrew Morton [EMAIL PROTECTED] wrote:
 
  I think the bug really is the running of populate_rootfs() before running
  the initcalls, in init/main.c:init().  It's just more sensible to start
  running userspace after the initcalls have been run.  Statically-linked
  drivers which want to load firmware files will lose.  To fix that we'd need
  a new callback.  It could be with a new linker section or perhaps simply a
  notifier chain.
 
 hm, actually...  Add two new initcall levels, one for populate_rootfs() and
 one for things which want to come after it (ie: drivers which want to
 access the filesytem):

IMO we should just call pipe (and socket) initialization directly at
the same level as slab, task, dcache, etc.

This is basic stuff needed to get operational kernel.  We could try
to put that on an additional initcall level, but then we ought to
take the rest of basic setup there as well.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 01:25:45AM -0800, Andrew Morton wrote:
 On Mon, 11 Dec 2006 09:21:30 +
 Al Viro [EMAIL PROTECTED] wrote:
 
  On Mon, Dec 11, 2006 at 01:13:27AM -0800, Andrew Morton wrote:
   On Mon, 11 Dec 2006 00:55:57 -0800
   Andrew Morton [EMAIL PROTECTED] wrote:
   
I think the bug really is the running of populate_rootfs() before 
running
the initcalls, in init/main.c:init().  It's just more sensible to start
running userspace after the initcalls have been run.  Statically-linked
drivers which want to load firmware files will lose.  To fix that we'd 
need
a new callback.  It could be with a new linker section or perhaps 
simply a
notifier chain.
   
   hm, actually...  Add two new initcall levels, one for populate_rootfs() 
   and
   one for things which want to come after it (ie: drivers which want to
   access the filesytem):
  
  IMO we should just call pipe (and socket) initialization directly at
  the same level as slab, task, dcache, etc.
 
 spose that would work.  But what other initcall-initialised things are not
 yet available when populate_rootfs() runs?
 
 does  grep _initcall */*.c
 wonders why anything works at all

Explain, please...
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 09:21:30 +
Al Viro [EMAIL PROTECTED] wrote:

 On Mon, Dec 11, 2006 at 01:13:27AM -0800, Andrew Morton wrote:
  On Mon, 11 Dec 2006 00:55:57 -0800
  Andrew Morton [EMAIL PROTECTED] wrote:
  
   I think the bug really is the running of populate_rootfs() before running
   the initcalls, in init/main.c:init().  It's just more sensible to start
   running userspace after the initcalls have been run.  Statically-linked
   drivers which want to load firmware files will lose.  To fix that we'd 
   need
   a new callback.  It could be with a new linker section or perhaps simply a
   notifier chain.
  
  hm, actually...  Add two new initcall levels, one for populate_rootfs() and
  one for things which want to come after it (ie: drivers which want to
  access the filesytem):
 
 IMO we should just call pipe (and socket) initialization directly at
 the same level as slab, task, dcache, etc.

spose that would work.  But what other initcall-initialised things are not
yet available when populate_rootfs() runs?

does  grep _initcall */*.c
wonders why anything works at all

 This is basic stuff needed to get operational kernel.  We could try
 to put that on an additional initcall level, but then we ought to
 take the rest of basic setup there as well.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Chuck Ebbert
In-Reply-To: [EMAIL PROTECTED]

On Mon, 11 Dec 2006 00:55:57 -0800, Andrew Morton wrote:

  Prevent oops when an app tries to create a pipe while pipefs
  is not mounted.
 
 That's pretty lame.  It means that pipes just won't work, so people who are
 using pipes in their initramfs setups will just get mysterious failures
 running userspace on a crippled kernel.

I know, I just wanted to keep the issue alive. :)

 I think the bug really is the running of populate_rootfs() before running
 the initcalls, in init/main.c:init().  It's just more sensible to start
 running userspace after the initcalls have been run.  Statically-linked
 drivers which want to load firmware files will lose.  To fix that we'd need
 a new callback.  It could be with a new linker section or perhaps simply a
 notifier chain.

Why not create a new initcall category for things that must run before
early userspace?

-- 
MBTI: IXTP

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Michael Tokarev
Al Viro wrote:
 On Mon, Dec 11, 2006 at 03:27:37AM -0500, Chuck Ebbert wrote:
 Prevent oops when an app tries to create a pipe while pipefs
 is not mounted.
[]
 That makes no sense at all.  pipe_mnt is not created by userland
 mount; it's created by init_pipe_fs() and we'd bloody better
 have it done before any applications get a chance to run.
 
 Please, explain how the hell did you manage to get a perverted
 config where that is not true.  In the meanwhile the patch is
 NAKed.

See for example http://marc.theaimsgroup.com/?t=11651039061r=1w=2

Thanks.

/mjt
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 09:33:15 +
Al Viro [EMAIL PROTECTED] wrote:

 On Mon, Dec 11, 2006 at 01:25:45AM -0800, Andrew Morton wrote:
  On Mon, 11 Dec 2006 09:21:30 +
  Al Viro [EMAIL PROTECTED] wrote:
  
   On Mon, Dec 11, 2006 at 01:13:27AM -0800, Andrew Morton wrote:
On Mon, 11 Dec 2006 00:55:57 -0800
Andrew Morton [EMAIL PROTECTED] wrote:

 I think the bug really is the running of populate_rootfs() before 
 running
 the initcalls, in init/main.c:init().  It's just more sensible to 
 start
 running userspace after the initcalls have been run.  
 Statically-linked
 drivers which want to load firmware files will lose.  To fix that 
 we'd need
 a new callback.  It could be with a new linker section or perhaps 
 simply a
 notifier chain.

hm, actually...  Add two new initcall levels, one for populate_rootfs() 
and
one for things which want to come after it (ie: drivers which want to
access the filesytem):
   
   IMO we should just call pipe (and socket) initialization directly at
   the same level as slab, task, dcache, etc.
  
  spose that would work.  But what other initcall-initialised things are not
  yet available when populate_rootfs() runs?
  
  does  grep _initcall */*.c
  wonders why anything works at all
 
 Explain, please...

- populate_rootfs() puts stuff into the filesystem

- we then run initcalls.

- an initcall runs /sbin/hotplug.

We're now running userspace before all the initcalls have been executed. 
Hence we're trying to run userspace when potentially none of grep
_initcall */*.c has been executed.  It isn't a kernel yet...

See http://marc.theaimsgroup.com/?l=linux-kernelm=116510389000878w=2
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 01:47:27AM -0800, Andrew Morton wrote:
 - populate_rootfs() puts stuff into the filesystem
 
 - we then run initcalls.
 
 - an initcall runs /sbin/hotplug.
 
 We're now running userspace before all the initcalls have been executed. 
 Hence we're trying to run userspace when potentially none of grep
 _initcall */*.c has been executed.  It isn't a kernel yet...

That's... arguable.  We certainly don't need lots and lots of initcalls
to be able to run userland code.  Which ones are missing in your opinion?

As for that example, I'd love to see specifics - which driver triggers
hotplug?  Presumably it happens from an initcall, so we also have something
fishy here...

Said that, I think that pipes should be initialized early.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 10:03:01 +
Al Viro [EMAIL PROTECTED] wrote:

 On Mon, Dec 11, 2006 at 01:47:27AM -0800, Andrew Morton wrote:
  - populate_rootfs() puts stuff into the filesystem
  
  - we then run initcalls.
  
  - an initcall runs /sbin/hotplug.
  
  We're now running userspace before all the initcalls have been executed. 
  Hence we're trying to run userspace when potentially none of grep
  _initcall */*.c has been executed.  It isn't a kernel yet...
 
 That's... arguable.  We certainly don't need lots and lots of initcalls
 to be able to run userland code.  Which ones are missing in your opinion?

I think we should aim to have as many subsystems ready to go as possible -
ideally all of them.  Right now we can potentially run userspace before
AIO, posix-timers, message-queues, BIO, networking, etc are ready to run.

It looks to be pretty easy to fix...

 As for that example, I'd love to see specifics - which driver triggers
 hotplug?  Presumably it happens from an initcall, so we also have something
 fishy here...

I don't know in this case - but firmware loading from a statically-linked
driver is a legit thing to do.

 Said that, I think that pipes should be initialized early.

Judging by the comment there, the only reason we prepare the rootfs prior
to running initcalls is for firmware.  So the sequence

run initcalls
populate rootfs
run initcalls which want to access files

fixes everything?

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 02:17:18AM -0800, Andrew Morton wrote:
 I think we should aim to have as many subsystems ready to go as possible -
 ideally all of them.  Right now we can potentially run userspace before
 AIO, posix-timers, message-queues, BIO, networking, etc are ready to run.
 
 It looks to be pretty easy to fix...
 
  As for that example, I'd love to see specifics - which driver triggers
  hotplug?  Presumably it happens from an initcall, so we also have something
  fishy here...
 
 I don't know in this case - but firmware loading from a statically-linked
 driver is a legit thing to do.

Umm... statically linked driver that might want firmware shouldn't precede
the subsystems unless something is seriously wrong with priorities...

IOW, I still wonder what's really going on - pipes are fs_initcall() and
any hardware stuff ought to be simple module_init().  So something fishy
is going on, regardless of anything else.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 02:17:18 -0800
Andrew Morton [EMAIL PROTECTED] wrote:

  Said that, I think that pipes should be initialized early.
 
 Judging by the comment there, the only reason we prepare the rootfs prior
 to running initcalls is for firmware.  So the sequence
 
   run initcalls
   populate rootfs
   run initcalls which want to access files
 
 fixes everything?

Like this...  The other part is hunting down all those drivers which want
to access the filesystem at init time.


 include/asm-generic/vmlinux.lds.h |6 +-
 include/linux/init.h  |5 +
 init/initramfs.c  |3 ++-
 init/main.c   |7 ---
 4 files changed, 12 insertions(+), 9 deletions(-)

diff -puN init/main.c~dont-run-userspace-until-initcalls-have-completed 
init/main.c
--- a/init/main.c~dont-run-userspace-until-initcalls-have-completed
+++ a/init/main.c
@@ -94,7 +94,6 @@ extern void pidmap_init(void);
 extern void prio_tree_init(void);
 extern void radix_tree_init(void);
 extern void free_initmem(void);
-extern void populate_rootfs(void);
 extern void driver_init(void);
 extern void prepare_namespace(void);
 #ifdef CONFIG_ACPI
@@ -739,12 +738,6 @@ static int init(void * unused)
 
cpuset_init_smp();
 
-   /*
-* Do this before initcalls, because some drivers want to access
-* firmware files.
-*/
-   populate_rootfs();
-
do_basic_setup();
 
/*
diff -puN 
include/linux/init.h~dont-run-userspace-until-initcalls-have-completed 
include/linux/init.h
--- a/include/linux/init.h~dont-run-userspace-until-initcalls-have-completed
+++ a/include/linux/init.h
@@ -115,6 +115,11 @@ extern void setup_arch(char **);
 #define device_initcall_sync(fn)   __define_initcall(6s,fn,6s)
 #define late_initcall(fn)  __define_initcall(7,fn,7)
 #define late_initcall_sync(fn) __define_initcall(7s,fn,7s)
+#define populate_rootfs_initcall(fn)   __define_initcall(8,fn,8)
+#define populate_rootfs_initcall_sync(fn) __define_initcall(8s,fn,8s)
+#define rootfs_neeeded_initcall(fn)__define_initcall(9,fn,9)
+#define rootfs_neeeded_initcall_sync(fn) __define_initcall(9s,fn,9s)
+
 
 #define __initcall(fn) device_initcall(fn)
 
diff -puN 
include/asm-generic/vmlinux.lds.h~dont-run-userspace-until-initcalls-have-completed
 include/asm-generic/vmlinux.lds.h
--- 
a/include/asm-generic/vmlinux.lds.h~dont-run-userspace-until-initcalls-have-completed
+++ a/include/asm-generic/vmlinux.lds.h
@@ -245,5 +245,9 @@
*(.initcall6.init)  \
*(.initcall6s.init) \
*(.initcall7.init)  \
-   *(.initcall7s.init)
+   *(.initcall7s.init) \
+   *(.initcall8.init)  \
+   *(.initcall8s.init) \
+   *(.initcall9.init)  \
+   *(.initcall9s.init)
 
diff -puN init/initramfs.c~dont-run-userspace-until-initcalls-have-completed 
init/initramfs.c
--- a/init/initramfs.c~dont-run-userspace-until-initcalls-have-completed
+++ a/init/initramfs.c
@@ -526,7 +526,7 @@ static void __init free_initrd(void)
 
 #endif
 
-void __init populate_rootfs(void)
+static int __init populate_rootfs(void)
 {
char *err = unpack_to_rootfs(__initramfs_start,
 __initramfs_end - __initramfs_start, 0);
@@ -566,3 +566,4 @@ void __init populate_rootfs(void)
}
 #endif
 }
+populate_rootfs_initcall(populate_rootfs);
_

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 10:22:07 +
Al Viro [EMAIL PROTECTED] wrote:

 On Mon, Dec 11, 2006 at 02:17:18AM -0800, Andrew Morton wrote:
  I think we should aim to have as many subsystems ready to go as possible -
  ideally all of them.  Right now we can potentially run userspace before
  AIO, posix-timers, message-queues, BIO, networking, etc are ready to run.
  
  It looks to be pretty easy to fix...
  
   As for that example, I'd love to see specifics - which driver triggers
   hotplug?  Presumably it happens from an initcall, so we also have 
   something
   fishy here...
  
  I don't know in this case - but firmware loading from a statically-linked
  driver is a legit thing to do.
 
 Umm... statically linked driver that might want firmware shouldn't precede
 the subsystems unless something is seriously wrong with priorities...

There are plenty of drivers in there using subsys_initcall, arch_initcall,
postcore_initcall, core_initcall and even one pure_initcall.

Heaven knows why.  They're drivers :(

 IOW, I still wonder what's really going on - pipes are fs_initcall() and
 any hardware stuff ought to be simple module_init().  So something fishy
 is going on, regardless of anything else.

A heck of a lot of things can trigger an /sbin/hotplug run.  It could well
be that Andrew's driver didn't want to run hotplug at all, but the kernel
did it anwyay.  But as soon as the script appeared at /sbin/hotplug, and it
happened to use foo|bar: boom.


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 02:27:46AM -0800, Andrew Morton wrote:
 @@ -115,6 +115,11 @@ extern void setup_arch(char **);
  #define device_initcall_sync(fn) __define_initcall(6s,fn,6s)
  #define late_initcall(fn)__define_initcall(7,fn,7)
  #define late_initcall_sync(fn)   __define_initcall(7s,fn,7s)
 +#define populate_rootfs_initcall(fn) __define_initcall(8,fn,8)
 +#define populate_rootfs_initcall_sync(fn) __define_initcall(8s,fn,8s)
 +#define rootfs_neeeded_initcall(fn)  __define_initcall(9,fn,9)
 +#define rootfs_neeeded_initcall_sync(fn) __define_initcall(9s,fn,9s)

Ewww  After module_init()?  Please, don't.  Come on, if it can
be a module, it _must_ be ready to run late in the game.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 02:34:36AM -0800, Andrew Morton wrote:

 There are plenty of drivers in there using subsys_initcall, arch_initcall,
 postcore_initcall, core_initcall and even one pure_initcall.
 
 Heaven knows why.  They're drivers :(
 
 A heck of a lot of things can trigger an /sbin/hotplug run.  It could well
 be that Andrew's driver didn't want to run hotplug at all, but the kernel
 did it anwyay.  But as soon as the script appeared at /sbin/hotplug, and it
 happened to use foo|bar: boom.

Again, I agree with moving pipes up, but keep in mind that any caller
of /sbin/hotplug either
* doesn't need it or
* really handles failure or
* really should _not_ be run before populate_rootfs()
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Olaf Hering
On Mon, Dec 11, Chuck Ebbert wrote:

 Why not create a new initcall category for things that must run before
 early userspace?

Why do you want to continue with papering over the root cause?
Pick some janitor, let him write something that implements something
like make style dependencies for initcalls.
Then you can get rid of all that foo_initcall stuff.
It surely needs work to get all that done.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 11:48:57AM +0100, Olaf Hering wrote:
 On Mon, Dec 11, Chuck Ebbert wrote:
 
  Why not create a new initcall category for things that must run before
  early userspace?
 
 Why do you want to continue with papering over the root cause?
 Pick some janitor, let him write something that implements something
 like make style dependencies for initcalls.
 Then you can get rid of all that foo_initcall stuff.
 It surely needs work to get all that done.

I would argue that _this_ is papering over the root cause.  Namely,
too complex ordering requirements.  Growing a technics for allowing
them to fester is an odd solution...
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Jeff Garzik

Andrew Morton wrote:

A heck of a lot of things can trigger an /sbin/hotplug run.  It could well
be that Andrew's driver didn't want to run hotplug at all, but the kernel
did it anwyay.  But as soon as the script appeared at /sbin/hotplug, and it
happened to use foo|bar: boom.


In fact, many things run /sbin/hotplug but don't necessarily need to :/

I think bcrl used a counter one time, and saw over 1000 /sbin/hotplug 
executions during a single boot.


Jeff


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Linus Torvalds


On Mon, 11 Dec 2006, Chuck Ebbert wrote:

 Prevent oops when an app tries to create a pipe while pipefs
 is not mounted.

Have you actually seen this, or is this just from looking at code?

Quite frankly, if pipe_mnt is ever NULL, we're dead for lots of other 
reasons. 

In fact, pipe_mnt can't be NULL. The way it is initialized is:

pipe_mnt = kern_mount(pipe_fs_type);

and pipe_mnt doesn't even return NULL - it returns an error pointer, so if 
kern_mount() were to have failed, pipe_mnt will be some random invalid 
pointer that could only be tested with IS_ERR(), not by comparing against 
NULL.

But more fundamentally - we might as well oops. We need to panic or oops 
or do _something_ bad at some point anyway, because it's MUCH better to 
fail spectacularly than it would be to just silently fail without a pipe.

Hmm?

Linus
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Linus Torvalds


On Mon, 11 Dec 2006, Al Viro wrote:

 On Mon, Dec 11, 2006 at 02:27:46AM -0800, Andrew Morton wrote:
  @@ -115,6 +115,11 @@ extern void setup_arch(char **);
   #define device_initcall_sync(fn)   __define_initcall(6s,fn,6s)
   #define late_initcall(fn)  __define_initcall(7,fn,7)
   #define late_initcall_sync(fn) __define_initcall(7s,fn,7s)
  +#define populate_rootfs_initcall(fn)   __define_initcall(8,fn,8)
  +#define populate_rootfs_initcall_sync(fn) __define_initcall(8s,fn,8s)
  +#define rootfs_neeeded_initcall(fn)__define_initcall(9,fn,9)
  +#define rootfs_neeeded_initcall_sync(fn) __define_initcall(9s,fn,9s)
 
 Ewww  After module_init()?  Please, don't.  Come on, if it can
 be a module, it _must_ be ready to run late in the game.

Yeah, I think you should just run populate_rootfs() just before 
module_init (which is the same as device_initcall()).

So perhaps somethign like this? (totally untested)

Btw, if the linker sorts sections some way (does it?) we could probably 
just make the vmlinux.lds.S file do

*(.initcall*.init)

or something, and then just let special cases like this use

__initcall(myfn, 5.1);

to show that it's between levels 5 and 6. But that would depend on the 
linker section beign sorted alphabetically. Does anybody know if the 
linker sorts these things somehow?

This patch is totally untested, but it looks obvious. It just says that 
we'll populate rootfs _after_ we've done the fs-level initcalls, but 
before we do any actual device initcalls.

If any really core stuff needs user-land - tough titties, as they say.

Linus

diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index 6e9fceb..7437cca 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -242,6 +242,7 @@
*(.initcall4s.init) \
*(.initcall5.init)  \
*(.initcall5s.init) \
+   *(.initcallrootfs.init) \
*(.initcall6.init)  \
*(.initcall6s.init) \
*(.initcall7.init)  \
diff --git a/include/linux/init.h b/include/linux/init.h
index 5eb5d24..5a593a1 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -111,6 +111,7 @@ extern void setup_arch(char **);
 #define subsys_initcall_sync(fn)   __define_initcall(4s,fn,4s)
 #define fs_initcall(fn)__define_initcall(5,fn,5)
 #define fs_initcall_sync(fn)   __define_initcall(5s,fn,5s)
+#define rootfs_initcall(fn)__define_initcall(rootfs,fn,rootfs)
 #define device_initcall(fn)__define_initcall(6,fn,6)
 #define device_initcall_sync(fn)   __define_initcall(6s,fn,6s)
 #define late_initcall(fn)  __define_initcall(7,fn,7)
diff --git a/init/initramfs.c b/init/initramfs.c
index 85f0403..4fa0f79 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -526,7 +526,7 @@ static void __init free_initrd(void)
 
 #endif
 
-void __init populate_rootfs(void)
+static int __init populate_rootfs(void)
 {
char *err = unpack_to_rootfs(__initramfs_start,
 __initramfs_end - __initramfs_start, 0);
@@ -544,7 +544,7 @@ void __init populate_rootfs(void)
unpack_to_rootfs((char *)initrd_start,
initrd_end - initrd_start, 0);
free_initrd();
-   return;
+   return 0;
}
printk(it isn't (%s); looks like an initrd\n, err);
fd = sys_open(/initrd.image, O_WRONLY|O_CREAT, 0700);
@@ -565,4 +565,6 @@ void __init populate_rootfs(void)
 #endif
}
 #endif
+   return 0;
 }
+rootfs_initcall(populate_rootfs);
diff --git a/init/main.c b/init/main.c
index 036f97c..ae12fa3 100644
--- a/init/main.c
+++ b/init/main.c
@@ -94,7 +94,6 @@ extern void pidmap_init(void);
 extern void prio_tree_init(void);
 extern void radix_tree_init(void);
 extern void free_initmem(void);
-extern void populate_rootfs(void);
 extern void driver_init(void);
 extern void prepare_namespace(void);
 #ifdef CONFIG_ACPI
@@ -739,12 +738,6 @@ static int init(void * unused)
 
cpuset_init_smp();
 
-   /*
-* Do this before initcalls, because some drivers want to access
-* firmware files.
-*/
-   populate_rootfs();
-
do_basic_setup();
 
/*
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Al Viro
On Mon, Dec 11, 2006 at 08:01:40AM -0800, Linus Torvalds wrote:
 
 
 On Mon, 11 Dec 2006, Al Viro wrote:
 
  On Mon, Dec 11, 2006 at 02:27:46AM -0800, Andrew Morton wrote:
   @@ -115,6 +115,11 @@ extern void setup_arch(char **);
#define device_initcall_sync(fn) __define_initcall(6s,fn,6s)
#define late_initcall(fn)__define_initcall(7,fn,7)
#define late_initcall_sync(fn)   __define_initcall(7s,fn,7s)
   +#define populate_rootfs_initcall(fn) __define_initcall(8,fn,8)
   +#define populate_rootfs_initcall_sync(fn) __define_initcall(8s,fn,8s)
   +#define rootfs_neeeded_initcall(fn)  __define_initcall(9,fn,9)
   +#define rootfs_neeeded_initcall_sync(fn) __define_initcall(9s,fn,9s)
  
  Ewww  After module_init()?  Please, don't.  Come on, if it can
  be a module, it _must_ be ready to run late in the game.
 
 Yeah, I think you should just run populate_rootfs() just before 
 module_init (which is the same as device_initcall()).
 
 If any really core stuff needs user-land - tough titties, as they say.

FWIW, I really think that this sort of bugs (oh, I call hotplug,
rootfs is there but kernel is not ready, woe is me) clearly show
that many, _many_ users of hotplug are BS.  The reason is simple -
if we have a call of hotplug that early, we have a driver that lives
with hotplug failures _and_ with different behaviour in built-in
and modular cases.  I would argue that any such driver is broken.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Linus Torvalds


On Mon, 11 Dec 2006, Al Viro wrote:
 
 FWIW, I really think that this sort of bugs (oh, I call hotplug,
 rootfs is there but kernel is not ready, woe is me) clearly show
 that many, _many_ users of hotplug are BS.  The reason is simple -
 if we have a call of hotplug that early, we have a driver that lives
 with hotplug failures _and_ with different behaviour in built-in
 and modular cases.  I would argue that any such driver is broken.

Now, you are probably right that many of them are unnecessary, but I don't 
agree in _general_. 

It's perfectly normal behaviour to say I discovered this device.

The fact that device discovery happens during early bootup and nobody even 
_cares_ (because early bootup ends up enumerating all discovered devices 
on its own) is a separate issue. The fact that many distros don't care 
during early bootup doesn't mean that they don't care later on.

Another reason for unnecessary hotplug events is that generic functions 
like I added a bus happen both for static buses _and_ for hey, somebody 
loaded a module that found this bus. Again, the static case may be 
something that people don't _care_ about, but we should (a) use common 
code and (b) be consistent, so we do a hotplug event regardless.

That said, I definitely agree that people shouldn't expect hotplug events 
to be delievered too early. If it's something that runs in a 
core_initcall, and is compiled in, no way in *hell* should we actually 
expose that to anything - it's just too early.

So it makes perfect sense to say

   you won't be getting any notification by anything built-in, until 
'device_initcall' (which is the default module_init, of course).

which in the case of certain drivers obviously _does_ mean that they had 
better not try to use any early initcalls to load firmware.

Linus
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Benjamin Herrenschmidt

 So it makes perfect sense to say
 
you won't be getting any notification by anything built-in, until 
 'device_initcall' (which is the default module_init, of course).
 
 which in the case of certain drivers obviously _does_ mean that they had 
 better not try to use any early initcalls to load firmware.

And that will fix some other issues I think I've seen (a while ago, I
might have a memory mixup here) related to /sbin/hotplug being called
before /dev/null  /dev/zero are initialized (they are fs_initcall). At
least with that patch, it won't happen.

Ben.


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Andrew Morton
On Mon, 11 Dec 2006 08:01:40 -0800 (PST)
Linus Torvalds [EMAIL PROTECTED] wrote:

 
 
 On Mon, 11 Dec 2006, Al Viro wrote:
 
  On Mon, Dec 11, 2006 at 02:27:46AM -0800, Andrew Morton wrote:
   @@ -115,6 +115,11 @@ extern void setup_arch(char **);
#define device_initcall_sync(fn) __define_initcall(6s,fn,6s)
#define late_initcall(fn)__define_initcall(7,fn,7)
#define late_initcall_sync(fn)   __define_initcall(7s,fn,7s)
   +#define populate_rootfs_initcall(fn) __define_initcall(8,fn,8)
   +#define populate_rootfs_initcall_sync(fn) __define_initcall(8s,fn,8s)
   +#define rootfs_neeeded_initcall(fn)  __define_initcall(9,fn,9)
   +#define rootfs_neeeded_initcall_sync(fn) __define_initcall(9s,fn,9s)
  
  Ewww  After module_init()?  Please, don't.  Come on, if it can
  be a module, it _must_ be ready to run late in the game.
 
 Yeah, I think you should just run populate_rootfs() just before 
 module_init (which is the same as device_initcall()).
 
 So perhaps somethign like this? (totally untested)
 
 Btw, if the linker sorts sections some way (does it?) we could probably 
 just make the vmlinux.lds.S file do
 
   *(.initcall*.init)
 
 or something, and then just let special cases like this use
 
   __initcall(myfn, 5.1);
 
 to show that it's between levels 5 and 6. But that would depend on the 
 linker section beign sorted alphabetically. Does anybody know if the 
 linker sorts these things somehow?
 
 This patch is totally untested, but it looks obvious. It just says that 
 we'll populate rootfs _after_ we've done the fs-level initcalls, but 
 before we do any actual device initcalls.
 
 If any really core stuff needs user-land - tough titties, as they say.
 
   Linus
 
 diff --git a/include/asm-generic/vmlinux.lds.h 
 b/include/asm-generic/vmlinux.lds.h
 index 6e9fceb..7437cca 100644
 --- a/include/asm-generic/vmlinux.lds.h
 +++ b/include/asm-generic/vmlinux.lds.h
 @@ -242,6 +242,7 @@
   *(.initcall4s.init) \
   *(.initcall5.init)  \
   *(.initcall5s.init) \
 + *(.initcallrootfs.init) \
   *(.initcall6.init)  \
   *(.initcall6s.init) \
   *(.initcall7.init)  \
 diff --git a/include/linux/init.h b/include/linux/init.h
 index 5eb5d24..5a593a1 100644
 --- a/include/linux/init.h
 +++ b/include/linux/init.h
 @@ -111,6 +111,7 @@ extern void setup_arch(char **);
  #define subsys_initcall_sync(fn) __define_initcall(4s,fn,4s)
  #define fs_initcall(fn)  __define_initcall(5,fn,5)
  #define fs_initcall_sync(fn) __define_initcall(5s,fn,5s)
 +#define rootfs_initcall(fn)  __define_initcall(rootfs,fn,rootfs)
  #define device_initcall(fn)  __define_initcall(6,fn,6)
  #define device_initcall_sync(fn) __define_initcall(6s,fn,6s)
  #define late_initcall(fn)__define_initcall(7,fn,7)

Looks like this might break pcmcia which for some reason does firmware
requesting at fs_initcall level (drivers/pcmcia/ds.c).


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Dominik Brodowski
On Mon, Dec 11, 2006 at 06:08:22PM -0800, Andrew Morton wrote:
  diff --git a/include/linux/init.h b/include/linux/init.h
  index 5eb5d24..5a593a1 100644
  --- a/include/linux/init.h
  +++ b/include/linux/init.h
  @@ -111,6 +111,7 @@ extern void setup_arch(char **);
   #define subsys_initcall_sync(fn)   __define_initcall(4s,fn,4s)
   #define fs_initcall(fn)__define_initcall(5,fn,5)
   #define fs_initcall_sync(fn)   __define_initcall(5s,fn,5s)
  +#define rootfs_initcall(fn)
  __define_initcall(rootfs,fn,rootfs)
   #define device_initcall(fn)__define_initcall(6,fn,6)
   #define device_initcall_sync(fn)   __define_initcall(6s,fn,6s)
   #define late_initcall(fn)  __define_initcall(7,fn,7)
 
 Looks like this might break pcmcia which for some reason does firmware
 requesting at fs_initcall level (drivers/pcmcia/ds.c).

That codepath is not triggered before device_initcall()s occur. So it's a
non-issue for PCMCIA.

Thanks,
Dominik
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch] pipe: Don't oops when pipe filesystem isn't mounted

2006-12-11 Thread Linus Torvalds


On Mon, 11 Dec 2006, Andrew Morton wrote:
 
 Looks like this might break pcmcia which for some reason does firmware
 requesting at fs_initcall level (drivers/pcmcia/ds.c).

Ok, that's just strange. 

I think it's fine to do init_pcmcia_bus early to make sure that the PCMCIA 
bus interface is there by the time the driver init stuff happens, but I 
really don't see the point of that firmware load to be there. And all that 
MATCH_FAKE_CIS stuff is about the _devices_, not the PCMCIA bus, so that 
whole thing looks pretty silly. It should be done by the device 
registration (which is obviously device_initcall), not by some bus layer.

Hopefully Dominik can fix whatever up (if it even needs it)

Linus
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/