10 Feb, 2010

1 commit

  • ======
    This fix is related to
    http://bugzilla.kernel.org/show_bug.cgi?id=15142
    but does not address that exact issue.
    ======

    sysfs does like attributes being removed while they are being accessed
    (i.e. read or written) and waits for the access to complete.

    As accessing some md attributes takes the same lock that is held while
    removing those attributes a deadlock can occur.

    This patch addresses 3 issues in md that could lead to this deadlock.

    Two relate to calling flush_scheduled_work while the lock is held.
    This is probably a bad idea in general and as we use schedule_work to
    delete various sysfs objects it is particularly bad.

    In one case flush_scheduled_work is called from md_alloc (called by
    md_probe) called from do_md_run which holds the lock. This call is
    only present to ensure that ->gendisk is set. However we can be sure
    that gendisk is always set (though possibly we couldn't when that code
    was originally written. This is because do_md_run is called in three
    different contexts:
    1/ from md_ioctl. This requires that md_open has succeeded, and it
    fails if ->gendisk is not set.
    2/ from writing a sysfs attribute. This can only happen if the
    mddev has been registered in sysfs which happens in md_alloc
    after ->gendisk has been set.
    3/ from autorun_array which is only called by autorun_devices, which
    checks for ->gendisk to be set before calling autorun_array.
    So the call to md_probe in do_md_run can be removed, and the check on
    ->gendisk can also go.

    In the other case flush_scheduled_work is being called in do_md_stop,
    purportedly to wait for all md_delayed_delete calls (which delete the
    component rdevs) to complete. However there really isn't any need to
    wait for them - they have already been disconnected in all important
    ways.

    The third issue is that raid5->stop() removes some attribute names
    while the lock is held. There is already some infrastructure in place
    to delay attribute removal until after the lock is released (using
    schedule_work). So extend that infrastructure to remove the
    raid5_attrs_group.

    This does not address all lockdep issues related to the sysfs
    "s_active" lock. The rest can be address by splitting that lockdep
    context between symlinks and non-symlinks which hopefully will happen.

    Signed-off-by: NeilBrown

    NeilBrown
     

30 Dec, 2009

5 commits

  • If two arrays share a device, then they will not both resync at the
    same time. One will wait for the other to complete.
    While waiting, the MD_RECOVERY_INTR flag is not checked so a device
    failure, which would make the resync pointless, does not cause the
    resync to abort, so the failed device cannot be removed (as it cannot
    be remove while a resync is happening).

    So add a test for MD_RECOVERY_INTR.

    Reported-by: Brett Russ
    Signed-off-by: NeilBrown

    NeilBrown
     
  • Since commit dfc7064500061677720fa26352963c772d3ebe6b,
    ->hot_remove_disks has not removed non-failed devices from
    an array until recovery is no longer possible.
    So the code in do_md_run to get around the fact that
    md_check_recovery (which calls ->hot_remove_disks) would
    remove partially-in-sync devices is no longer needed.

    So remove it.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • By default md_do_sync() will perform recovery if no other actions are
    specified. However, action_show() relies on MD_RECOVERY_RECOVER to be
    set otherwise it returns 'idle'. So, add a missing set
    MD_RECOVERY_RECOVER when starting recovery.

    Signed-off-by: Dan Williams
    Signed-off-by: NeilBrown

    Dan Williams
     
  • The start_ro modules parameter can be used to force arrays to be
    started in 'auto-readonly' in which they are read-only until the first
    write. This ensures that no resync/recovery happens until something
    else writes to the device. This is important for resume-from-disk
    off an md array.

    However if an array is started 'readonly' (by writing 'readonly' to
    the 'array_state' sysfs attribute) we want it to be really 'readonly',
    not 'auto-readonly'.

    So strengthen the condition to only set auto-readonly if the
    array is not already read-only.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • evms configures md arrays by:
    open device
    send ioctl
    close device

    for each different ioctl needed.
    Since 2.6.29, the device can disappear after the 'close'
    unless a significant configuration has happened to the device.
    The change made by "SET_ARRAY_INFO" can too minor to stop the device
    from disappearing, but important enough that losing the change is bad.

    So: make sure SET_ARRAY_INFO sets mddev->ctime, and keep the device
    active as long as ctime is non-zero (it gets zeroed with lots of other
    things when the array is stopped).

    This is suitable for -stable kernels since 2.6.29.

    Signed-off-by: NeilBrown
    Cc: stable@kernel.org

    NeilBrown
     

16 Dec, 2009

2 commits

  • Signed-off-by: Joe Perches
    Cc: Neil Brown
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Joe Perches
     
  • Makes use of skip_spaces() defined in lib/string.c for removing leading
    spaces from strings all over the tree.

    It decreases lib.a code size by 47 bytes and reuses the function tree-wide:
    text data bss dec hex filename
    64688 584 592 65864 10148 (TOTALS-BEFORE)
    64641 584 592 65817 10119 (TOTALS-AFTER)

    Also, while at it, if we see (*str && isspace(*str)), we can be sure to
    remove the first condition (*str) as the second one (isspace(*str)) also
    evaluates to 0 whenever *str == 0, making it redundant. In other words,
    "a char equals zero is never a space".

    Julia Lawall tried the semantic patch (http://coccinelle.lip6.fr) below,
    and found occurrences of this pattern on 3 more files:
    drivers/leds/led-class.c
    drivers/leds/ledtrig-timer.c
    drivers/video/output.c

    @@
    expression str;
    @@

    ( // ignore skip_spaces cases
    while (*str && isspace(*str)) { \(str++;\|++str;\) }
    |
    - *str &&
    isspace(*str)
    )

    Signed-off-by: André Goddard Rosa
    Cc: Julia Lawall
    Cc: Martin Schwidefsky
    Cc: Jeff Dike
    Cc: Ingo Molnar
    Cc: Thomas Gleixner
    Cc: "H. Peter Anvin"
    Cc: Richard Purdie
    Cc: Neil Brown
    Cc: Kyle McMartin
    Cc: Henrique de Moraes Holschuh
    Cc: David Howells
    Cc:
    Cc: Samuel Ortiz
    Cc: Patrick McHardy
    Cc: Takashi Iwai
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    André Goddard Rosa
     

14 Dec, 2009

14 commits

  • Enable external metadata arrays to manage rebuild checkpointing via a
    md/dev-XXX/recovery_start attribute which reflects rdev->recovery_offset

    Also update resync_start_store to allow 'none' to be written, for
    consistency.

    Signed-off-by: Dan Williams
    Signed-off-by: NeilBrown

    Dan Williams
     
  • Other walks of this list are either under rcu_read_lock() or the list
    mutation lock (mddev_lock()). This protects against the improbable case of a
    disk being removed from the array at the start of md_do_sync().

    Signed-off-by: Dan Williams

    Dan Williams
     
  • As v1.x metadata can record that a member of the array is
    not completely recovered, it make sense to record that a
    spare has become a regular member of the array at the earliest
    opportunity.
    So remove the tests on "recovery_offset > 0" in super_1_sync
    as they really aren't needed, and schedule a metadata update
    immediately after adding spares to a degraded array.

    This means that if a crash happens immediately after a recovery
    starts, the new device will be included in the array and recovery will
    continue from wherever it was up to. Previously this didn't happen
    unless recovery was at least 1/16 of the way through.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • The RAID ioctls are only implemented in md.c, so the
    handling for them should also be moved there from
    fs/compat_ioctl.c.

    Signed-off-by: Arnd Bergmann
    Cc: Neil Brown
    Cc: Andre Noll
    Cc: linux-raid@vger.kernel.org
    Signed-off-by: NeilBrown

    Arnd Bergmann
     
  • Suggested by Oren Held

    Signed-off-by: NeilBrown

    NeilBrown
     
  • We've noticed severe lasting performance degradation of our raid
    arrays when we have drives that yield large amounts of media errors.
    The raid10 module will queue each failed read for retry, and also
    will attempt call fix_read_error() to perform the read recovery.
    Read recovery is performed while the array is frozen, so repeated
    recovery attempts can degrade the performance of the array for
    extended periods of time.

    With this patch I propose adding a per md device max number of
    corrected read attempts. Each rdev will maintain a count of
    read correction attempts in the rdev->read_errors field (not
    used currently for raid10). When we enter fix_read_error()
    we'll check to see when the last read error occurred, and
    divide the read error count by 2 for every hour since the
    last read error. If at that point our read error count
    exceeds the read error threshold, we'll fail the raid device.

    In addition in this patch I add sysfs nodes (get/set) for
    the per md max_read_errors attribute, the rdev->read_errors
    attribute, and added some printk's to indicate when
    fix_read_error fails to repair an rdev.

    For testing I used debugfs->fail_make_request to inject
    IO errors to the rdev while doing IO to the raid array.

    Signed-off-by: Robert Becker
    Signed-off-by: NeilBrown

    Robert Becker
     
  • A new attribute directory 'bitmap' in 'md' is created which
    contains files for configuring the bitmap.
    'location' identifies where the bitmap is, either 'none',
    or 'file' or 'sector offset from metadata'.
    Writing 'location' can create or remove a bitmap.
    Adding a 'file' bitmap this way is not yet supported.
    'chunksize' and 'time_base' must be set before 'location'
    can be set.

    'chunksize' can be set before creating a bitmap, but is
    currently always over-ridden by the bitmap superblock.

    'time_base' and 'backlog' can be updated at any time.

    Signed-off-by: NeilBrown
    Reviewed-by: Andre Noll

    NeilBrown
     
  • safe_delay_store can parse fixed point numbers (for fractions
    of a second). We will want to do that for another sysfs
    file soon, so factor out the code.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • ... and into bitmap_info. These are all configuration parameters
    that need to be set before the bitmap is created.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • In preparation for making bitmap fields configurable via sysfs,
    start tidying up by making a single structure to contain the
    configuration fields.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • Previously barriers were only supported on RAID1. This is because
    other levels requires synchronisation across all devices and so needed
    a different approach.
    Here is that approach.

    When a barrier arrives, we send a zero-length barrier to every active
    device. When that completes - and if the original request was not
    empty - we submit the barrier request itself (with the barrier flag
    cleared) and then submit a fresh load of zero length barriers.

    The barrier request itself is asynchronous, but any subsequent
    request will block until the barrier completes.

    The reason for clearing the barrier flag is that a barrier request is
    allowed to fail. If we pass a non-empty barrier through a striping
    raid level it is conceivable that part of it could succeed and part
    could fail. That would be way too hard to deal with.
    So if the first run of zero length barriers succeed, we assume all is
    sufficiently well that we send the request and ignore errors in the
    second run of barriers.

    RAID5 needs extra care as write requests may not have been submitted
    to the underlying devices yet. So we flush the stripe cache before
    proceeding with the barrier.

    Note that the second set of zero-length barriers are submitted
    immediately after the original request is submitted. Thus when
    a personality finds mddev->barrier to be set during make_request,
    it should not return from make_request until the corresponding
    per-device request(s) have been queued.

    That will be done in later patches.

    Signed-off-by: NeilBrown
    Reviewed-by: Andre Noll

    NeilBrown
     
  • If a resync/recovery/check/repair is interrupted for some reason, it
    can be useful to know exactly where it got up to.
    So in that case, do not clear curr_resync_completed.
    Initialise it when starting a resync/recovery/... instead.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • When a 'check' or 'repair' finished we should clear resync_min
    so that a future check/repair will cover the whole array (by default).
    However if it is interrupted, we should update resync_min to
    where we got up to, so that when the check/repair continues it
    just does the remainder of the array.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • A write intent bitmap can be removed from an array while the
    array is active.
    When this happens, all IO is suspended and flushed before the
    bitmap is removed.
    However it is possible that bitmap_daemon_work is still running to
    clear old bits from the bitmap. If it is, it can dereference the
    bitmap after it has been freed.

    So introduce a new mutex to protect bitmap_daemon_work and get it
    before destroying a bitmap.

    This is suitable for any current -stable kernel.

    Signed-off-by: NeilBrown
    Cc: stable@kernel.org

    NeilBrown
     

08 Dec, 2009

1 commit

  • * git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/sysctl-2.6: (43 commits)
    security/tomoyo: Remove now unnecessary handling of security_sysctl.
    security/tomoyo: Add a special case to handle accesses through the internal proc mount.
    sysctl: Drop & in front of every proc_handler.
    sysctl: Remove CTL_NONE and CTL_UNNUMBERED
    sysctl: kill dead ctl_handler definitions.
    sysctl: Remove the last of the generic binary sysctl support
    sysctl net: Remove unused binary sysctl code
    sysctl security/tomoyo: Don't look at ctl_name
    sysctl arm: Remove binary sysctl support
    sysctl x86: Remove dead binary sysctl support
    sysctl sh: Remove dead binary sysctl support
    sysctl powerpc: Remove dead binary sysctl support
    sysctl ia64: Remove dead binary sysctl support
    sysctl s390: Remove dead sysctl binary support
    sysctl frv: Remove dead binary sysctl support
    sysctl mips/lasat: Remove dead binary sysctl support
    sysctl drivers: Remove dead binary sysctl support
    sysctl crypto: Remove dead binary sysctl support
    sysctl security/keys: Remove dead binary sysctl support
    sysctl kernel: Remove binary sysctl logic
    ...

    Linus Torvalds
     

19 Nov, 2009

1 commit


17 Nov, 2009

1 commit


13 Nov, 2009

1 commit

  • This is a combination that didn't really make sense before.
    However when a reshape is converting e.g. raid5 -> raid6, the extra
    device is not fully in-sync, but is certainly active and contains
    important data.
    So allow that start to be meaningful and in particular get
    the 'recovery_offset' value (which is needed for any non-in-sync
    active device) from the reshape_position.

    Signed-off-by: NeilBrown

    NeilBrown
     

12 Nov, 2009

2 commits

  • Now that sys_sysctl is a wrapper around /proc/sys all of
    the binary sysctl support elsewhere in the tree is
    dead code.

    Cc: Jens Axboe
    Cc: Corey Minyard
    Cc: Greg Kroah-Hartman
    Cc: Matt Mackall
    Cc: Herbert Xu
    Cc: Neil Brown
    Cc: "James E.J. Bottomley"
    Acked-by: Clemens Ladisch for drivers/char/hpet.c
    Signed-off-by: Eric W. Biederman

    Eric W. Biederman
     
  • Each device has its own 'recovery_offset' showing how far
    recovery has progressed on the device.
    As the only real significance of this is that fact that it can
    be stored in the metadata and recovered at restart, and as
    only 1.x metadata can do this, we were only updating
    'recovery_offset' to 'curr_resync_completed' when updating
    v1.x metadata.
    But this is wrong, and we will shortly make limited use of this
    field in v0.90 metadata.

    So move the update into common code.

    Signed-off-by: NeilBrown

    NeilBrown
     

06 Nov, 2009

1 commit

  • If a 'sync_max' has been set (via sysfs), it is wrong to clear it
    until a resync (or reshape or recovery ...) actually reached that
    point.
    So if a resync is interrupted (e.g. by device failure),
    leave 'resync_max' unchanged.

    This is particularly important for 'reshape' operations that do not
    change the size of the array. For such operations mdadm needs to
    monitor the reshape taking rolling backups of the section being
    reshaped. If resync_max gets cleared, the reshape can get ahead of
    mdadm and then the backups that mdadm creates are useless.

    This is suitable for 2.6.31.y stable kernels.
    Cc: stable@kernel.org
    Signed-off-by: NeilBrown

    NeilBrown
     

16 Oct, 2009

1 commit

  • When a raid5 (or raid6) array is being reshaped to have fewer devices,
    conf->raid_disks is the latter and hence smaller number of devices.
    However sometimes we want to use a number which is the total number of
    currently required devices - the larger of the 'old' and 'new' sizes.
    Before we implemented reducing the number of devices, this was always
    'new' i.e. ->raid_disks.
    Now we need max(raid_disks, previous_raid_disks) in those places.

    This particularly affects assembling an array that was shutdown while
    in the middle of a reshape to fewer devices.

    md.c needs a similar fix when interpreting the md metadata.

    Signed-off-by: NeilBrown

    NeilBrown
     

23 Sep, 2009

3 commits


22 Sep, 2009

1 commit


18 Aug, 2009

1 commit

  • Recent commit c8c00a6915a2e3d10416e8bdd3138429beb96210
    changed the exit paths in do_md_stop and was not quite
    careful enough. There is one path were 'err' now needs
    to be cleared but it isn't.
    So setting an array to readonly (with mdadm --readonly) will
    work, but will incorrectly report and error: ENXIO.

    Signed-off-by: NeilBrown

    NeilBrown
     

13 Aug, 2009

2 commits

  • Normally we only allow the upper limit for a reshape to be decreased
    when the array not performing a sync/recovery/reshape, otherwise there
    could be races. But if an array is part-way through a reshape when it
    is assembled the reshape is started immediately leaving no window
    to set an upper bound.

    If the array is started read-only, the reshape will be suspended until
    the array becomes writable, so that provides a window during which it
    is perfectly safe to reduce the upper limit of a reshape.

    So: allow the upper limit (sync_max) to be reduced even if the reshape
    thread is running, as long as the array is still read-only.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • When assembling arrays, md allows two devices to have different event
    counts as long as the difference is only '1'. This is to cope with
    a system failure between updating the metadata on two difference
    devices.

    However there are currently times when we update the event count by
    2. This was done to keep the event count even when the array is clean
    and odd when it is dirty, which allows us to avoid writing common
    update to spare devices and so allow those spares to go to sleep.

    This is bad for the above reason. So change it to never increase by
    two. This means that the alignment between 'odd/even' and
    'clean/dirty' might take a little longer to attain, but that is only a
    small cost. The spares will get a few more updates but that will
    still be spared (;-) most updates and can still go to sleep.

    Prior to this patch there was a small chance that after a crash an
    array would fail to assemble due to the overly large event count
    mismatch.

    Signed-off-by: NeilBrown

    NeilBrown
     

10 Aug, 2009

1 commit

  • A recent commit:
    commit 449aad3e25358812c43afc60918c5ad3819488e7

    introduced the possibility of an A-B/B-A deadlock between
    bd_mutex and reconfig_mutex.

    __blkdev_get holds bd_mutex while calling md_open which takes
    reconfig_mutex,
    do_md_run is always called with reconfig_mutex held, and it now
    takes bd_mutex in the call the revalidate_disk.

    This potential deadlock was not caught by lockdep due to the
    use of mutex_lock_interruptible_nexted which was introduced
    by
    commit d63a5a74dee87883fda6b7d170244acaac5b05e8
    do avoid a warning of an impossible deadlock.

    It is quite possible to split reconfig_mutex in to two locks.
    One protects the array data structures while it is being
    reconfigured, the other ensures that an array is never even partially
    open while it is being deactivated.
    In particular, the second lock prevents an open from completing
    between the time when do_md_stop checks if there are any active opens,
    and the time when the array is either set read-only, or when ->pers is
    set to NULL. So we can be certain that no IO is in flight as the
    array is being destroyed.

    So create a new lock, open_mutex, just to ensure exclusion between
    'open' and 'stop'.

    This avoids the deadlock and also avoids the lockdep warning mentioned
    in commit d63a5a74d

    Reported-by: "Mike Snitzer"
    Reported-by: "H. Peter Anvin"
    Signed-off-by: NeilBrown

    NeilBrown
     

03 Aug, 2009

2 commits

  • As revalidate_disk calls check_disk_size_change, it will cause
    any capacity change of a gendisk to be propagated to the blockdev
    inode. So use that instead of mucking about with locks and
    i_size_write.

    Also add a call to revalidate_disk in do_md_run and a few other places
    where the gendisk capacity is changed.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • The v1.x metadata does not have a fixed size and can grow
    when devices are added.
    If it grows enough to require an extra sector of storage,
    we need to update the 'sb_size' to match.

    Without this, md can write out an incomplete superblock with a
    bad checksum, which will be rejected when trying to re-assemble
    the array.

    Cc: stable@kernel.org
    Signed-off-by: NeilBrown

    NeilBrown