[patch 3/4] fastboot: make the raid autodetect code wait for all devices to init - Kernel

This is a discussion on [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init - Kernel ; From: Arjan van de Ven Date: Sun, 20 Jul 2008 13:07:09 -0700 Subject: [PATCH] fastboot: make the raid autodetect code wait for all devices to init The raid autodetect code really needs to have all devices probed before it can ...

+ Reply to Thread
Results 1 to 7 of 7

Thread: [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init

  1. [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init


    From: Arjan van de Ven
    Date: Sun, 20 Jul 2008 13:07:09 -0700
    Subject: [PATCH] fastboot: make the raid autodetect code wait for all devices to init

    The raid autodetect code really needs to have all devices probed before
    it can detect raid arrays; not doing so would give rather messy situations
    where arrays would get detected as degraded while they shouldn't be etc.

    This is in preparation of removing the "wait for everything to init"
    code that makes everyone pay, not just raid users.

    Signed-off-by: Arjan van de Ven
    ---
    init/do_mounts_md.c | 7 +++++++
    1 files changed, 7 insertions(+), 0 deletions(-)

    diff --git a/init/do_mounts_md.c b/init/do_mounts_md.c
    index 693d246..c0412a9 100644
    --- a/init/do_mounts_md.c
    +++ b/init/do_mounts_md.c
    @@ -267,9 +267,16 @@ __setup("md=", md_setup);
    void __init md_run_setup(void)
    {
    create_dev("/dev/md0", MKDEV(MD_MAJOR, 0));
    +
    if (raid_noautodetect)
    printk(KERN_INFO "md: Skipping autodetection of RAID arrays. (raid=noautodetect)\n");
    else {
    + /*
    + * Since we don't want to detect and use half a raid array, we
    + * need to wait for the known devices to complete their probing
    + */
    + while (driver_probe_done() != 0)
    + msleep(100);
    int fd = sys_open("/dev/md0", 0, 0);
    if (fd >= 0) {
    sys_ioctl(fd, RAID_AUTORUN, raid_autopart);
    --
    1.5.5.1

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

  2. Re: [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init


    (wondering why I wasn't Cc:ed on this...)

    On Sunday July 20, arjan@infradead.org wrote:
    >
    > From: Arjan van de Ven
    > Date: Sun, 20 Jul 2008 13:07:09 -0700
    > Subject: [PATCH] fastboot: make the raid autodetect code wait for all devices to init
    >
    > The raid autodetect code really needs to have all devices probed before
    > it can detect raid arrays; not doing so would give rather messy situations
    > where arrays would get detected as degraded while they shouldn't be etc.
    >
    > This is in preparation of removing the "wait for everything to init"
    > code that makes everyone pay, not just raid users.
    >
    > Signed-off-by: Arjan van de Ven
    > ---
    > init/do_mounts_md.c | 7 +++++++
    > 1 files changed, 7 insertions(+), 0 deletions(-)
    >
    > diff --git a/init/do_mounts_md.c b/init/do_mounts_md.c
    > index 693d246..c0412a9 100644
    > --- a/init/do_mounts_md.c
    > +++ b/init/do_mounts_md.c
    > @@ -267,9 +267,16 @@ __setup("md=", md_setup);
    > void __init md_run_setup(void)
    > {
    > create_dev("/dev/md0", MKDEV(MD_MAJOR, 0));
    > +
    > if (raid_noautodetect)
    > printk(KERN_INFO "md: Skipping autodetection of RAID arrays. (raid=noautodetect)\n");
    > else {
    > + /*
    > + * Since we don't want to detect and use half a raid array, we
    > + * need to wait for the known devices to complete their probing
    > + */
    > + while (driver_probe_done() != 0)
    > + msleep(100);
    > int fd = sys_open("/dev/md0", 0, 0);
    > if (fd >= 0) {
    > sys_ioctl(fd, RAID_AUTORUN, raid_autopart);


    I must say that I think this is pretty horrible. But then it is a
    pretty horrible problem and I don't think there is a clean solution.

    If md in a module, this code won't run so there will be no change. If
    md is compiled in, this code will silently slow down boot even if
    there are no raid arrays to assemble. I think the "silently" is a
    problem. I'm not looking forward to "my computer boots slower if I
    compile md into the kernel" reports on linux-raid@vger.

    What would you think of

    if (driver_probe_done() != 0) {
    printk("md: Waiting for all devices to be available before autodetect\n"
    "md: If you don't boot off raid, use raid=noautodetect\n");
    do
    msleep(100);
    while (driver_probe_done() != 0);
    }

    ??

    Also, the "driver_probe_done() != 0" bothers me.

    If driver_probe_done is a boolean, it should be
    while (! drive_probe_done() ) msleep ....

    and if it returns a negative error, then it should be
    while ( drive_probe_done() < 0) msleep ....

    The != 0 confuses me about the expected return type.


    The "real" solution here involves assembling arrays in userspace using
    "mdadm --incremental" from udevd, and using write-intent-bitmaps so
    that writing to an array before all the component devices are
    available can be done without requiring a full resync. There is still
    a bit more code needed to make that work really smoothly.

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

  3. Re: [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init

    On Mon, 21 Jul 2008 09:06:36 +1000
    Neil Brown wrote:

    > > + /*
    > > + * Since we don't want to detect and use half a
    > > raid array, we
    > > + * need to wait for the known devices to complete
    > > their probing
    > > + */
    > > + while (driver_probe_done() != 0)
    > > + msleep(100);
    > > int fd = sys_open("/dev/md0", 0, 0);
    > > if (fd >= 0) {
    > > sys_ioctl(fd, RAID_AUTORUN, raid_autopart);

    >
    > I must say that I think this is pretty horrible. But then it is a
    > pretty horrible problem and I don't think there is a clean solution.


    it's also the existing solution, just moved around to the only place
    that needs it.

    >
    > If md in a module, this code won't run so there will be no change. If
    > md is compiled in, this code will silently slow down boot even if
    > there are no raid arrays to assemble.


    it's not slower than it is is today for *everyone*. All that this does
    is move it to the MD code rather than having it 1 line before calling
    the MD code (the removal of the global wait is in the next patch)
    Waiting twice is the same cost as waiting once; the moment everything
    is done it'll stay done.

    > I think the "silently" is a
    > problem. I'm not looking forward to "my computer boots slower if I
    > compile md into the kernel" reports on linux-raid@vger.


    I *am* looking forward to "my computer boots faster" reports though ;-)

    >
    > What would you think of
    >
    > if (driver_probe_done() != 0) {
    > printk("md: Waiting for all devices to be available before
    > autodetect\n" "md: If you don't boot off raid, use
    > raid=noautodetect\n"); do
    > msleep(100);
    > while (driver_probe_done() != 0);
    > }


    not sure we need the outer if for this (it'll basically always hit, and
    even if by some miracle, everything is done, it's no big deal, you
    tried to wait and were done immediately).
    Adding a printk like this makes total sense to me; I'll add a patch for
    that.

    Maybe it's worth introducing a config option for the raid autodetect
    thing; but I'll leave that up to you.


    > Also, the "driver_probe_done() != 0" bothers me.


    it's the existing form just moved; I wanted to make the change to be as
    simple as possible.

    >
    > The "real" solution here involves assembling arrays in userspace using
    > "mdadm --incremental" from udevd, and using write-intent-bitmaps so
    > that writing to an array before all the component devices are
    > available can be done without requiring a full resync. There is still
    > a bit more code needed to make that work really smoothly.


    if you use an initrd, the code above doesn't run...
    totally different method of booting.

    --
    If you want to reach me at my work email, use arjan@linux.intel.com
    For development, discussion and tips for power savings,
    visit http://www.lesswatts.org
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init

    On Mon, 21 Jul 2008, Neil Brown wrote:

    > I must say that I think this is pretty horrible. But then it is a
    > pretty horrible problem and I don't think there is a clean solution.
    >
    > If md in a module, this code won't run so there will be no change. If
    > md is compiled in, this code will silently slow down boot even if
    > there are no raid arrays to assemble. I think the "silently" is a
    > problem. I'm not looking forward to "my computer boots slower if I
    > compile md into the kernel" reports on linux-raid@vger.
    >
    > What would you think of
    >
    > if (driver_probe_done() != 0) {
    > printk("md: Waiting for all devices to be available before autodetect\n"
    > "md: If you don't boot off raid, use raid=noautodetect\n");
    > do
    > msleep(100);
    > while (driver_probe_done() != 0);
    > }
    >
    > ??


    even if you don't boot off of raid, if the boot process will mount a raid
    that's autodetected this will be a problem. so I think the message needs
    to allow for this.

    David Lang

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

  5. Re: [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init

    On Mon, 21 Jul 2008 09:06:36 +1000
    Neil Brown wrote:

    > if (driver_probe_done() != 0) {
    > printk("md: Waiting for all devices to be available before
    > autodetect\n" "md: If you don't boot off raid, use
    > raid=noautodetect\n"); do



    how about this patch?

    From: Arjan van de Ven
    Date: Sun, 20 Jul 2008 16:30:29 -0700
    Subject: [PATCH] fastboot: make the RAID autostart code print a message just before waiting

    As requested/suggested by Neil Brown: make the raid code print that it's
    about to wait for probing to be done as well as give a suggestion on how
    to disable the probing if the user doesn't use raid.

    Signed-off-by: Arjan van de Ven ---
    init/do_mounts_md.c | 4 +++-
    1 files changed, 3 insertions(+), 1 deletions(-)

    diff --git a/init/do_mounts_md.c b/init/do_mounts_md.c
    index c0412a9..2ab122a 100644
    --- a/init/do_mounts_md.c
    +++ b/init/do_mounts_md.c
    @@ -275,7 +275,9 @@ void __init md_run_setup(void)
    * Since we don't want to detect and use half a raid array, we need to
    * wait for the known devices to complete their probing
    */
    - while (driver_probe_done() != 0)
    + printk(KERN_INFO "md: Waiting for all devices to be available before autodetect\n");
    + printk(KERN_INFO "md: If you don't use raid, use raid=noautodetect\n");
    + while (driver_probe_done())
    msleep(100);
    int fd = sys_open("/dev/md0", 0, 0);
    if (fd >= 0) {
    --
    1.5.5.1

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

  6. Re: [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init

    On Sunday July 20, arjan@infradead.org wrote:
    > On Mon, 21 Jul 2008 09:06:36 +1000
    > Neil Brown wrote:
    >
    > > if (driver_probe_done() != 0) {
    > > printk("md: Waiting for all devices to be available before
    > > autodetect\n" "md: If you don't boot off raid, use
    > > raid=noautodetect\n"); do

    >
    >
    > how about this patch?


    Well,

    > - while (driver_probe_done() != 0)
    > + printk(KERN_INFO "md: Waiting for all devices to be available before autodetect\n");
    > + printk(KERN_INFO "md: If you don't use raid, use raid=noautodetect\n");


    that's better thanks. I'm quite happy with that.


    > + while (driver_probe_done())
    > msleep(100);


    That's worse. Now it really looks like a boolean that is being used
    wrongly. It seems that driver_probe_done either returns 0 or -EBUSY,
    so I'd prefer

    > + while (driver_probe_done() < 0)
    > msleep(100);


    or even

    > + while (driver_probe_done() == -EBUSY)
    > msleep(100);


    though that is less robust.

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

  7. Re: [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init

    On Mon, 21 Jul 2008 14:30:12 +1000
    Neil Brown wrote:

    >
    > > + while (driver_probe_done())
    > > msleep(100);

    >
    > That's worse. Now it really looks like a boolean that is being used
    > wrongly. It seems that driver_probe_done either returns 0 or -EBUSY,
    > so I'd prefer
    >
    > > + while (driver_probe_done() < 0)
    > > msleep(100);

    >
    > or even


    ok I've done this one.

    I'll be the first to say that this driver_probe_done() isn't very
    elegant.. but it's out of my current scope to rewrite that whole
    thing ... I'll leave that to GregKH.


    --
    If you want to reach me at my work email, use arjan@linux.intel.com
    For development, discussion and tips for power savings,
    visit http://www.lesswatts.org
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread