24 Jun, 2010

12 commits

  • There are few situations where it would make any sense to add a spare
    when reducing the number of devices in an array, but it is
    conceivable: A 6 drive RAID6 with two missing devices could be
    reshaped to a 5 drive RAID6, and a spare could become available
    just in time for the reshape, but not early enough to have been
    recovered first. 'freezing' recovery can make this easy to
    do without any races.

    However doing such a thing is a bad idea. md will not record the
    partially-recovered state of the 'spare' and when the reshape
    finished it will think that the spare is still spare.
    Easiest way to avoid this confusion is to simply disallow it.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • As the comment says, the tail of this loop only applies to devices
    that are not fully in sync, so if In_sync was set, we should avoid
    the rest of the loop.

    This bug will hardly ever cause an actual problem. The worst it
    can do is allow an array to be assembled that is dirty and degraded,
    which is not generally a good idea (without warning the sysadmin
    first).

    This will only happen if the array is RAID4 or a RAID5/6 in an
    intermediate state during a reshape and so has one drive that is
    all 'parity' - no data - while some other device has failed.

    This is certainly possible, but not at all common.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • During a recovery of reshape the early part of some devices might be
    in-sync while the later parts are not.
    We we know we are looking at an early part it is good to treat that
    part as in-sync for stripe calculations.

    This is particularly important for a reshape which suffers device
    failure. Treating the data as in-sync can mean the difference between
    data-safety and data-loss.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • When we are reshaping an array, the device failure combinations
    that cause us to decide that the array as failed are more subtle.

    In particular, any 'spare' will be fully in-sync in the section
    of the array that has already been reshaped, thus failures that
    affect only that section are less critical.

    So encode this subtlety in a new function and call it as appropriate.

    The case that showed this problem was a 4 drive RAID5 to 8 drive RAID6
    conversion where the last two devices failed.
    This resulted in:

    good good good good incomplete good good failed failed

    while converting a 5-drive RAID6 to 8 drive RAID5
    The incomplete device causes the whole array to look bad,
    bad as it was actually good for the section that had been
    converted to 8-drives, all the data was actually safe.

    Reported-by: Terry Morris
    Signed-off-by: NeilBrown

    NeilBrown
     
  • When an array is reshaped to have fewer devices, the reshape proceeds
    from the end of the devices to the beginning.

    If a device happens to be non-In_sync (which is possible but rare)
    we would normally update the ->recovery_offset as the reshape
    progresses. However that would be wrong as the recover_offset records
    that the early part of the device is in_sync, while in fact it would
    only be the later part that is in_sync, and in any case the offset
    number would be measured from the wrong end of the device.

    Relatedly, if after a reshape a spare is discovered to not be
    recoverred all the way to the end, not allow spare_active
    to incorporate it in the array.

    This becomes relevant in the following sample scenario:

    A 4 drive RAID5 is converted to a 6 drive RAID6 in a combined
    operation.
    The RAID5->RAID6 conversion will cause a 5 drive to be included as a
    spare, then the 5drive -> 6drive reshape will effectively rebuild that
    spare as it progresses. The 6th drive is treated as in_sync the whole
    time as there is never any case that we might consider reading from
    it, but must not because there is no valid data.

    If we interrupt this reshape part-way through and reverse it to return
    to a 5-drive RAID6 (or event a 4-drive RAID5), we don't want to update
    the recovery_offset - as that would be wrong - and we don't want to
    include that spare as active in the 5-drive RAID6 when the reversed
    reshape completed and it will be mostly out-of-sync still.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • The entries in the stripe_cache maintained by raid5 are enlarged
    when we increased the number of devices in the array, but not
    shrunk when we reduce the number of devices.
    So if entries are added after reducing the number of devices, we
    much ensure to initialise the whole entry, not just the part that
    is currently relevant. Otherwise if we enlarge the array again,
    we will reference uninitialised values.

    As grow_buffers/shrink_buffer now want to use a count that is stored
    explicity in the raid_conf, they should get it from there rather than
    being passed it as a parameter.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • Only level 5 with layout=PARITY_N can be taken over to raid0 now.
    Lets allow level 4 either.

    Signed-off-by: Maciej Trela
    Signed-off-by: NeilBrown

    Maciej Trela
     
  • After takeover from raid5/10 -> raid0 mddev->layout is not cleared.

    Signed-off-by: Maciej Trela
    Signed-off-by: NeilBrown

    Maciej Trela
     
  • Use mddev->new_layout in setup_conf.
    Also use new_chunk, and don't set ->degraded in takeover(). That
    gets set in run()

    Signed-off-by: Maciej Trela
    Signed-off-by: NeilBrown

    Maciej Trela
     
  • Most array level changes leave the list of devices largely unchanged,
    possibly causing one at the end to become redundant.
    However conversions between RAID0 and RAID10 need to renumber
    all devices (except 0).

    This renumbering is currently being done in the ->run method when the
    new personality takes over. However this is too late as the common
    code in md.c might already have invalidated some of the devices if
    they had a ->raid_disk number that appeared to high.

    Moving it into the ->takeover method is too early as the array is
    still active at that time and wrong ->raid_disk numbers could cause
    confusion.

    So add a ->new_raid_disk field to mdk_rdev_s and use it to communicate
    the new raid_disk number.
    Now the common code knows exactly which devices need to be renumbered,
    and which can be invalidated, and can do it all at a convenient time
    when the array is suspend.
    It can also update some symlinks in sysfs which previously were not be
    updated correctly.

    Reported-by: Maciej Trela
    Signed-off-by: NeilBrown

    NeilBrown
     
  • Such NULL pointer dereference can occur when the driver was fixing the
    read errors/bad blocks and the disk was physically removed
    causing a system crash. This patch check if the
    rcu_dereference() returns valid rdev before accessing it in fix_read_error().

    Cc: stable@kernel.org
    Signed-off-by: Prasanna S. Panchamukhi
    Signed-off-by: Rob Becker
    Signed-off-by: NeilBrown

    Prasanna S. Panchamukhi
     
  • Commit b821eaa572fd737faaf6928ba046e571526c36c6 broke partition
    detection for md arrays.

    The logic was almost right. However if revalidate_disk is called
    when the device is not yet open, bdev->bd_disk won't be set, so the
    flush_disk() Call will not set bd_invalidated.

    So when md_open is called we still need to ensure that
    ->bd_invalidated gets set. This is easily done with a call to
    check_disk_size_change in the place where the offending commit removed
    check_disk_change. At the important times, the size will have changed
    from 0 to non-zero, so check_disk_size_change will set bd_invalidated.

    Tested-by: Duncan
    Reported-by: Duncan
    Signed-off-by: NeilBrown

    NeilBrown
     

28 May, 2010

1 commit


22 May, 2010

4 commits

  • * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6: (69 commits)
    fix handling of offsets in cris eeprom.c, get rid of fake on-stack files
    get rid of home-grown mutex in cris eeprom.c
    switch ecryptfs_write() to struct inode *, kill on-stack fake files
    switch ecryptfs_get_locked_page() to struct inode *
    simplify access to ecryptfs inodes in ->readpage() and friends
    AFS: Don't put struct file on the stack
    Ban ecryptfs over ecryptfs
    logfs: replace inode uid,gid,mode initialization with helper function
    ufs: replace inode uid,gid,mode initialization with helper function
    udf: replace inode uid,gid,mode init with helper
    ubifs: replace inode uid,gid,mode initialization with helper function
    sysv: replace inode uid,gid,mode initialization with helper function
    reiserfs: replace inode uid,gid,mode initialization with helper function
    ramfs: replace inode uid,gid,mode initialization with helper function
    omfs: replace inode uid,gid,mode initialization with helper function
    bfs: replace inode uid,gid,mode initialization with helper function
    ocfs2: replace inode uid,gid,mode initialization with helper function
    nilfs2: replace inode uid,gid,mode initialization with helper function
    minix: replace inode uid,gid,mode init with helper
    ext4: replace inode uid,gid,mode init with helper
    ...

    Trivial conflict in fs/fs-writeback.c (mark bitfields unsigned)

    Linus Torvalds
     
  • Conflicts:
    drivers/md/md.c

    - Resolved conflict in md_update_sb
    - Added extra 'NULL' arg to new instance of sysfs_get_dirent.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • Now that the last user passing a NULL file pointer is gone we can remove
    the redundant dentry argument and associated hacks inside vfs_fsynmc_range.

    The next step will be removig the dentry argument from ->fsync, but given
    the luck with the last round of method prototype changes I'd rather
    defer this until after the main merge window.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Al Viro

    Christoph Hellwig
     
  • The problem. When implementing a network namespace I need to be able
    to have multiple network devices with the same name. Currently this
    is a problem for /sys/class/net/*, /sys/devices/virtual/net/*, and
    potentially a few other directories of the form /sys/ ... /net/*.

    What this patch does is to add an additional tag field to the
    sysfs dirent structure. For directories that should show different
    contents depending on the context such as /sys/class/net/, and
    /sys/devices/virtual/net/ this tag field is used to specify the
    context in which those directories should be visible. Effectively
    this is the same as creating multiple distinct directories with
    the same name but internally to sysfs the result is nicer.

    I am calling the concept of a single directory that looks like multiple
    directories all at the same path in the filesystem tagged directories.

    For the networking namespace the set of directories whose contents I need
    to filter with tags can depend on the presence or absence of hotplug
    hardware or which modules are currently loaded. Which means I need
    a simple race free way to setup those directories as tagged.

    To achieve a reace free design all tagged directories are created
    and managed by sysfs itself.

    Users of this interface:
    - define a type in the sysfs_tag_type enumeration.
    - call sysfs_register_ns_types with the type and it's operations
    - sysfs_exit_ns when an individual tag is no longer valid

    - Implement mount_ns() which returns the ns of the calling process
    so we can attach it to a sysfs superblock.
    - Implement ktype.namespace() which returns the ns of a syfs kobject.

    Everything else is left up to sysfs and the driver layer.

    For the network namespace mount_ns and namespace() are essentially
    one line functions, and look to remain that.

    Tags are currently represented a const void * pointers as that is
    both generic, prevides enough information for equality comparisons,
    and is trivial to create for current users, as it is just the
    existing namespace pointer.

    The work needed in sysfs is more extensive. At each directory
    or symlink creating I need to check if the directory it is being
    created in is a tagged directory and if so generate the appropriate
    tag to place on the sysfs_dirent. Likewise at each symlink or
    directory removal I need to check if the sysfs directory it is
    being removed from is a tagged directory and if so figure out
    which tag goes along with the name I am deleting.

    Currently only directories which hold kobjects, and
    symlinks are supported. There is not enough information
    in the current file attribute interfaces to give us anything
    to discriminate on which makes it useless, and there are
    no potential users which makes it an uninteresting problem
    to solve.

    Signed-off-by: Eric W. Biederman
    Signed-off-by: Benjamin Thery
    Signed-off-by: Greg Kroah-Hartman

    Eric W. Biederman
     

18 May, 2010

23 commits

  • Devices which know that they are spares do not really need to have
    an event count that matches the rest of the array, so there are no
    data-in-sync issues. It is enough that the uuid matches.
    So remove the requirement that the event count is up-to-date.

    We currently still write out and event count on spares, but this
    allows us in a year or 3 to stop doing that completely.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • When updating the event count for a simple clean dirty transition,
    we try to avoid updating the spares so they can safely spin-down.
    As the event_counts across an array must be +/- 1, this means
    decrementing the event_count on a dirty->clean transition.
    This is not always safe and we have to avoid the unsafe time.
    We current do this with a misguided idea about it being safe or
    not depending on whether the event_count is odd or even. This
    approach only works reliably in a few common instances, but easily
    falls down.

    So instead, simply keep internal state concerning whether it is safe
    or not, and always assume it is not safe when an array is first
    assembled.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • Fix: Raid-6 was not trying to correct a read-error when in
    singly-degraded state and was instead dropping one more device, going to
    doubly-degraded state. This patch fixes this behaviour.

    Tested-by: Janos Haar
    Signed-off-by: Gabriele A. Trombetti
    Reported-by: Janos Haar
    Signed-off-by: NeilBrown
    Cc: stable@kernel.org

    Gabriele A. Trombetti
     
  • Some time ago we stopped the clean/active metadata updates
    from being written to a 'spare' device in most cases so that
    it could spin down and say spun down. Device failure/removal
    etc are still recorded on spares.

    However commit 51d5668cb2e3fd1827a55 broke this 50% of the time,
    depending on whether the event count is even or odd.
    The change log entry said:

    This means that the alignment between 'odd/even' and
    'clean/dirty' might take a little longer to attain,

    how ever the code makes no attempt to create that alignment, so it
    could take arbitrarily long.

    So when we find that clean/dirty is not aligned with odd/even,
    force a second metadata-update immediately. There are already cases
    where a second metadata-update is needed immediately (e.g. when a
    device fails during the metadata update). We just piggy-back on that.

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

    NeilBrown
     
  • read_balance uses a "unsigned long" for a sector number which
    will get truncated beyond 2TB.
    This will cause read-balancing to be non-optimal, and can cause
    data to be read from the 'wrong' branch during a resync. This has a
    very small chance of returning wrong data.

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

    NeilBrown
     
  • md/linear:mdname:

    Signed-off-by: NeilBrown

    NeilBrown
     
  • All messages now start
    md/raid0:md-device-name:

    Signed-off-by: NeilBrown

    NeilBrown
     
  • All raid10 printk messages now start
    md/raid10:md-device-name:

    Signed-off-by: NeilBrown

    NeilBrown
     
  • Make sure the array name is included in a uniform way in all printk
    messages.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • Many 'printk' messages from the raid456 module mention 'raid5' even
    though it may be a 'raid6' or even 'raid4' array. This can cause
    confusion.
    Also the actual array name is not always reported and when it is
    it is not reported consistently.

    So change all the messages to start:
    md/raid:%s:
    where '%s' becomes e.g. md3 to identify the particular array.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • RAID10 has been available for quite a while now and is quite well
    tested, so we can remove the EXPERIMENTAL designation.

    Reported-by: Eric MSP Veith
    Signed-off-by: NeilBrown

    NeilBrown
     
  • e.g. allow md to interpret 'echo 4 > md/level' as a request for raid4.

    Signed-off-by: Dan Williams

    Dan Williams
     
  • Level modifications change the output of mdstat. The mdmon manager
    thread is interested in these events for external metadata management.

    Signed-off-by: Dan Williams

    Dan Williams
     
  • For consistency allow raid4 to takeover raid0 in addition to raid5 (with a
    raid4 layout).

    Signed-off-by: Dan Williams

    Dan Williams
     
  • When a raid1 array is configured to support write-behind
    on some devices, it normally only reads from other devices.
    If all devices are write-behind (because the rest have failed)
    it is possible for a read request to be serviced before a
    behind-write request, which would appear as data corruption.

    So when forced to read from a WriteMostly device, wait for any
    write-behind to complete, and don't start any more behind-writes.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • This message seems to suggest the named device is the one on which a
    read failed, however it is actually the device that the read will be
    redirected to.
    So make the message a little clearer.

    Reported-by: Tim Burgess
    Signed-off-by: NeilBrown

    NeilBrown
     
  • This is
    - unnecessary because mddev_suspend is always followed by a call to
    ->stop, and each ->stop unregisters the thread, and
    - a problem as it makes it awkwards to suspend and then resume a
    device as we will want later.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • This is a simple factorisation that makes mddev_find easier to read.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • We used to pass the personality make_request function direct
    to the block layer so the first argument had to be a queue.
    But now we have the intermediary md_make_request so it makes
    at lot more sense to pass a struct mddev_s.
    It makes it possible to have an mddev without its own queue too.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • This moves the call to the other side of set_readonly, but that should
    not be an issue.
    This encapsulates in 'md_stop' all of the functionality for internally
    stopping the array, leaving all the interactions with externalities
    (sysfs, request_queue, gendisk) in do_md_stop.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • Using do_md_stop to set an array to read-only is a little confusing.
    Now most of the common code has been factored out, split
    md_set_readonly off in to a separate function.

    Signed-off-by: NeilBrown

    NeilBrown
     
  • Further refactoring of do_md_stop.
    This one requires some explanation as it takes code from different
    places in do_md_stop, so some re-ordering happens.

    We only get into this part of do_md_stop if there are no active opens
    of the device, so no writes can be happening and the device must have
    been flushed. In md_stop_writes we want to stop any internal sources
    of writes - i.e. resync - and flush out the metadata.

    The only code that was previously before some of this code is
    code to clean up the queue, the mddev, the gendisk, or sysfs, all
    of which is probably better after code that makes active changes (i.e.
    triggers writes).

    Signed-off-by: NeilBrown

    NeilBrown
     
  • do_md_stop is large and clunky, so hard to understand.

    This is a first step of refactoring, pulling two simple
    sub-functions out.

    Signed-off-by: NeilBrown

    NeilBrown