[PATCH 000 of 9] md: Assorted patches for the 2.5.26 merge window - Kernel

This is a discussion on [PATCH 000 of 9] md: Assorted patches for the 2.5.26 merge window - Kernel ; Following 9 patches for md are suitable for 2.5.26. Yes, I really should have submitted them to -mm earlier, but I've been on leave, sorry. First patch is a bug fix that should go in 2.6.25.stable. It has been copied ...

+ Reply to Thread
Results 1 to 7 of 7

Thread: [PATCH 000 of 9] md: Assorted patches for the 2.5.26 merge window

  1. [PATCH 000 of 9] md: Assorted patches for the 2.5.26 merge window


    Following 9 patches for md are suitable for 2.5.26.
    Yes, I really should have submitted them to -mm earlier, but I've been
    on leave, sorry.

    First patch is a bug fix that should go in 2.6.25.stable.
    It has been copied to stable@kernel.org.

    Others are mostly related to "external" metadata support. i.e. a userspace
    program does all the management of metadata, and needs to communicate
    with the kernel to do its job properly.

    NeilBrown


    [PATCH 001 of 9] md: Fix use after free when removing rdev via sysfs
    [PATCH 002 of 9] md: Skip all metadata update processing when using external metadata.
    [PATCH 003 of 9] md: Reinitialise more mddev fields in do_md_stop.
    [PATCH 004 of 9] md: Fix 'safemode' handling for external metadata.
    [PATCH 005 of 9] md: Fix up switching md arrays between read-only and read-write
    [PATCH 006 of 9] md: Remove a stray command from a copy and paste error in resync_start_store
    [PATCH 007 of 9] md: prevent duplicates in bind_rdev_to_array
    [PATCH 008 of 9] md: md: raid5 rate limit error printk
    [PATCH 009 of 9] md: md: support blocking writes to an array on device failure
    --
    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. [PATCH 001 of 9] md: Fix use after free when removing rdev via sysfs


    From: Dan Williams

    rdev->mddev is no longer valid upon return from entry->store() when the
    'remove' command is given.

    This should go in 2.6.25.stable.

    Cc: stable@kernel.org
    Signed-off-by: Dan Williams
    Signed-off-by: Neil Brown

    ### Diffstat output
    ./drivers/md/md.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    diff .prev/drivers/md/md.c ./drivers/md/md.c
    --- .prev/drivers/md/md.c 2008-04-29 12:27:50.000000000 +1000
    +++ ./drivers/md/md.c 2008-04-29 12:27:55.000000000 +1000
    @@ -2096,7 +2096,7 @@ rdev_attr_store(struct kobject *kobj, st
    rv = -EBUSY;
    else
    rv = entry->store(rdev, page, length);
    - mddev_unlock(rdev->mddev);
    + mddev_unlock(mddev);
    }
    return rv;
    }
    --
    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. [PATCH 004 of 9] md: Fix 'safemode' handling for external metadata.


    'safemode' relates to marking an array as 'clean' if there has been
    no write traffic for a while (a couple of seconds), to reduce the chance
    of the array being found dirty on reboot.

    ->safemode is set to '1' when there have been no write for a while, and
    it gets set to '0' when the superblock is updates with the 'clean' flag set.

    This requires a few fixes for 'external' metadata:
    - When an array is set to 'clean' via sysfs, 'safemode' must be cleared.
    - when we write to an array that has 'safemode' set (there must have been
    some delay in updating the metadata), we need to clear safemode.
    - Don't try to update external metadata in md_check_recovery for safemode
    transitions - it won't work.

    Also, don't try to support "immediate safe mode" (safemode==2) for external
    metadata, it cannot really work (the safemode timeout can be set very low
    if this is really needed).

    Signed-off-by: Neil Brown

    ### Diffstat output
    ./drivers/md/md.c | 30 +++++++++++++++++++-----------
    1 file changed, 19 insertions(+), 11 deletions(-)

    diff .prev/drivers/md/md.c ./drivers/md/md.c
    --- .prev/drivers/md/md.c 2008-04-29 12:27:56.000000000 +1000
    +++ ./drivers/md/md.c 2008-04-29 12:27:56.000000000 +1000
    @@ -2614,6 +2614,8 @@ array_state_store(mddev_t *mddev, const
    if (atomic_read(&mddev->writes_pending) == 0) {
    if (mddev->in_sync == 0) {
    mddev->in_sync = 1;
    + if (mddev->safemode == 1)
    + mddev->safemode = 0;
    if (mddev->persistent)
    set_bit(MD_CHANGE_CLEAN,
    &mddev->flags);
    @@ -5391,6 +5393,8 @@ void md_write_start(mddev_t *mddev, stru
    md_wakeup_thread(mddev->sync_thread);
    }
    atomic_inc(&mddev->writes_pending);
    + if (mddev->safemode == 1)
    + mddev->safemode = 0;
    if (mddev->in_sync) {
    spin_lock_irq(&mddev->write_lock);
    if (mddev->in_sync) {
    @@ -5815,7 +5819,7 @@ void md_check_recovery(mddev_t *mddev)
    return;

    if (signal_pending(current)) {
    - if (mddev->pers->sync_request) {
    + if (mddev->pers->sync_request && !mddev->external) {
    printk(KERN_INFO "md: %s in immediate safe mode\n",
    mdname(mddev));
    mddev->safemode = 2;
    @@ -5827,7 +5831,7 @@ void md_check_recovery(mddev_t *mddev)
    (mddev->flags && !mddev->external) ||
    test_bit(MD_RECOVERY_NEEDED, &mddev->recovery) ||
    test_bit(MD_RECOVERY_DONE, &mddev->recovery) ||
    - (mddev->safemode == 1) ||
    + (mddev->external == 0 && mddev->safemode == 1) ||
    (mddev->safemode == 2 && ! atomic_read(&mddev->writes_pending)
    && !mddev->in_sync && mddev->recovery_cp == MaxSector)
    ))
    @@ -5836,16 +5840,20 @@ void md_check_recovery(mddev_t *mddev)
    if (mddev_trylock(mddev)) {
    int spares = 0;

    - spin_lock_irq(&mddev->write_lock);
    - if (mddev->safemode && !atomic_read(&mddev->writes_pending) &&
    - !mddev->in_sync && mddev->recovery_cp == MaxSector) {
    - mddev->in_sync = 1;
    - if (mddev->persistent)
    - set_bit(MD_CHANGE_CLEAN, &mddev->flags);
    + if (!mddev->external) {
    + spin_lock_irq(&mddev->write_lock);
    + if (mddev->safemode &&
    + !atomic_read(&mddev->writes_pending) &&
    + !mddev->in_sync &&
    + mddev->recovery_cp == MaxSector) {
    + mddev->in_sync = 1;
    + if (mddev->persistent)
    + set_bit(MD_CHANGE_CLEAN, &mddev->flags);
    + }
    + if (mddev->safemode == 1)
    + mddev->safemode = 0;
    + spin_unlock_irq(&mddev->write_lock);
    }
    - if (mddev->safemode == 1)
    - mddev->safemode = 0;
    - spin_unlock_irq(&mddev->write_lock);

    if (mddev->flags)
    md_update_sb(mddev, 0);
    --
    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 007 of 9] md: prevent duplicates in bind_rdev_to_array

    On Tue, 29 Apr 2008 13:35:27 +1000 NeilBrown wrote:

    >
    > From: Dan Williams
    >
    > Found when trying to reassemble an active externally managed array.
    > Without this check we hit the more noisy "sysfs duplicate" warning in
    > the later call to kobject_add.
    >
    > Signed-off-by: Dan Williams
    > Signed-off-by: Neil Brown
    >
    > ### Diffstat output
    > ./drivers/md/md.c | 5 +++++
    > 1 file changed, 5 insertions(+)
    >
    > diff .prev/drivers/md/md.c ./drivers/md/md.c
    > --- .prev/drivers/md/md.c 2008-04-29 12:27:57.000000000 +1000
    > +++ ./drivers/md/md.c 2008-04-29 12:27:57.000000000 +1000
    > @@ -1369,6 +1369,11 @@ static int bind_rdev_to_array(mdk_rdev_t
    > MD_BUG();
    > return -EINVAL;
    > }
    > +
    > + /* prevent duplicates */
    > + if (find_rdev(mddev, rdev->bdev->bd_dev))
    > + return -EEXIST;
    > +
    > /* make sure rdev->size exceeds mddev->size */
    > if (rdev->size && (mddev->size == 0 || rdev->size < mddev->size)) {
    > if (mddev->pers) {


    Smells racy. Do we have enough locking in place here to make this more
    than a best-effort thing?
    --
    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 008 of 9] md: md: raid5 rate limit error printk

    On Tue, 29 Apr 2008 13:35:34 +1000 NeilBrown wrote:

    > + printk_rl(KERN_WARNING "raid5:%s: read error NOT corrected!! "
    > + "(sector %llu on %s).\n",
    > + mdname(conf->mddev),
    > + (unsigned long long)(sh->sector + rdev->data_offset),
    > + bdn);
    > else if (atomic_read(&rdev->read_errors)
    > > conf->max_nr_stripes)
    > printk(KERN_WARNING
    >
    > diff .prev/include/linux/raid/md_k.h ./include/linux/raid/md_k.h
    > --- .prev/include/linux/raid/md_k.h 2008-04-29 12:25:24.000000000 +1000
    > +++ ./include/linux/raid/md_k.h 2008-04-29 12:27:58.000000000 +1000
    > @@ -368,6 +368,9 @@ static inline void safe_put_page(struct
    > if (p) put_page(p);
    > }
    >
    > +#define printk_rl printk_ratelimit() ?: printk


    (boggle)

    Isn't this backwards? Should be !printk_ratelimit()?

    open-coding the printk_ratelimit() at each callsite would be more
    conventional.
    --
    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 007 of 9] md: prevent duplicates in bind_rdev_to_array

    On Monday April 28, akpm@linux-foundation.org wrote:
    > On Tue, 29 Apr 2008 13:35:27 +1000 NeilBrown wrote:
    >
    > >
    > > From: Dan Williams
    > >
    > > Found when trying to reassemble an active externally managed array.
    > > Without this check we hit the more noisy "sysfs duplicate" warning in
    > > the later call to kobject_add.
    > >
    > > Signed-off-by: Dan Williams
    > > Signed-off-by: Neil Brown
    > >
    > > ### Diffstat output
    > > ./drivers/md/md.c | 5 +++++
    > > 1 file changed, 5 insertions(+)
    > >
    > > diff .prev/drivers/md/md.c ./drivers/md/md.c
    > > --- .prev/drivers/md/md.c 2008-04-29 12:27:57.000000000 +1000
    > > +++ ./drivers/md/md.c 2008-04-29 12:27:57.000000000 +1000
    > > @@ -1369,6 +1369,11 @@ static int bind_rdev_to_array(mdk_rdev_t
    > > MD_BUG();
    > > return -EINVAL;
    > > }
    > > +
    > > + /* prevent duplicates */
    > > + if (find_rdev(mddev, rdev->bdev->bd_dev))
    > > + return -EEXIST;
    > > +
    > > /* make sure rdev->size exceeds mddev->size */
    > > if (rdev->size && (mddev->size == 0 || rdev->size < mddev->size)) {
    > > if (mddev->pers) {

    >
    > Smells racy. Do we have enough locking in place here to make this more
    > than a best-effort thing?



    Yes. We have exclusive access to the mddev at this point, so no race.

    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 008 of 9] md: md: raid5 rate limit error printk

    On Monday April 28, akpm@linux-foundation.org wrote:
    > On Tue, 29 Apr 2008 13:35:34 +1000 NeilBrown wrote:
    >
    > > + printk_rl(KERN_WARNING "raid5:%s: read error NOT corrected!! "
    > > + "(sector %llu on %s).\n",
    > > + mdname(conf->mddev),
    > > + (unsigned long long)(sh->sector + rdev->data_offset),
    > > + bdn);
    > > else if (atomic_read(&rdev->read_errors)
    > > > conf->max_nr_stripes)
    > > printk(KERN_WARNING
    > >
    > > diff .prev/include/linux/raid/md_k.h ./include/linux/raid/md_k.h
    > > --- .prev/include/linux/raid/md_k.h 2008-04-29 12:25:24.000000000 +1000
    > > +++ ./include/linux/raid/md_k.h 2008-04-29 12:27:58.000000000 +1000
    > > @@ -368,6 +368,9 @@ static inline void safe_put_page(struct
    > > if (p) put_page(p);
    > > }
    > >
    > > +#define printk_rl printk_ratelimit() ?: printk

    >
    > (boggle)


    You don't like the "?:" operator?

    Maybe
    printk_ratelimit() && printk
    ?

    >
    > Isn't this backwards? Should be !printk_ratelimit()?


    Arggg.. Did I do that? Bother.

    >
    > open-coding the printk_ratelimit() at each callsite would be more
    > conventional.


    True, but it can get noisy, adding an extra level of indent where it
    isn't really needed (and the printk lines tend to be fairly long
    already).

    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/

+ Reply to Thread