1. 01 Jul, 2008 1 commit
    • Dan Williams's avatar
      md: resolve external metadata handling deadlock in md_allow_write · b5470dc5
      Dan Williams authored
      md_allow_write() marks the metadata dirty while holding mddev->lock and then
      waits for the write to complete.  For externally managed metadata this causes a
      deadlock as userspace needs to take the lock to communicate that the metadata
      update has completed.
      
      Change md_allow_write() in the 'external' case to start the 'mark active'
      operation and then return -EAGAIN.  The expected side effects while waiting for
      userspace to write 'active' to 'array_state' are holding off reshape (code
      currently handles -ENOMEM), cause some 'stripe_cache_size' change requests to
      fail, cause some GET_BITMAP_FILE ioctl requests to fall back to GFP_NOIO, and
      cause updates to 'raid_disks' to fail.  Except for 'stripe_cache_size' changes
      these failures can be mitigated by coordinating with mdmon.
      
      md_write_start() still prevents writes from occurring until the metadata
      handler has had a chance to take action as it unconditionally waits for
      MD_CHANGE_CLEAN to be cleared.
      
      [neilb@suse.de: return -EAGAIN, try GFP_NOIO]
      Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
      b5470dc5
  2. 27 Jun, 2008 29 commits
    • Dan Williams's avatar
      md: rationalize raid5 function names · 1fe797e6
      Dan Williams authored
      From: Dan Williams <dan.j.williams@intel.com>
      
      Commit a4456856 refactored some of the deep code paths in raid5.c into separate
      functions.  The names chosen at the time do not consistently indicate what is
      going to happen to the stripe.  So, update the names, and since a stripe is a
      cache element use cache semantics like fill, dirty, and clean.
      
      (also, fix up the indentation in fetch_block5)
      Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      1fe797e6
    • Dan Williams's avatar
      md: handle operation chaining in raid5_run_ops · 7b3a871e
      Dan Williams authored
      From: Dan Williams <dan.j.williams@intel.com>
      
      Neil said:
      > At the end of ops_run_compute5 you have:
      >         /* ack now if postxor is not set to be run */
      >         if (tx && !test_bit(STRIPE_OP_POSTXOR, &s->ops_run))
      >                 async_tx_ack(tx);
      >
      > It looks odd having that test there.  Would it fit in raid5_run_ops
      > better?
      
      The intended global interpretation is that raid5_run_ops can build a chain
      of xor and memcpy operations.  When MD registers the compute-xor it tells
      async_tx to keep the operation handle around so that another item in the
      dependency chain can be submitted. If we are just computing a block to
      satisfy a read then we can terminate the chain immediately.  raid5_run_ops
      gives a better context for this test since it cares about the entire chain.
      Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      7b3a871e
    • Dan Williams's avatar
      md: replace R5_WantPrexor with R5_WantDrain, add 'prexor' reconstruct_states · d8ee0728
      Dan Williams authored
      From: Dan Williams <dan.j.williams@intel.com>
      
      Currently ops_run_biodrain and other locations have extra logic to determine
      which blocks are processed in the prexor and non-prexor cases.  This can be
      eliminated if handle_write_operations5 flags the blocks to be processed in all
      cases via R5_Wantdrain.  The presence of the prexor operation is tracked in
      sh->reconstruct_state.
      Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      d8ee0728
    • Dan Williams's avatar
      md: replace STRIPE_OP_{BIODRAIN,PREXOR,POSTXOR} with 'reconstruct_states' · 600aa109
      Dan Williams authored
      From: Dan Williams <dan.j.williams@intel.com>
      
      Track the state of reconstruct operations (recalculating the parity block
      usually due to incoming writes, or as part of array expansion)  Reduces the
      scope of the STRIPE_OP_{BIODRAIN,PREXOR,POSTXOR} flags to only tracking whether
      a reconstruct operation has been requested via the ops_request field of struct
      stripe_head_state.
      
      This is the final step in the removal of ops.{pending,ack,complete,count}, i.e.
      the STRIPE_OP_{BIODRAIN,PREXOR,POSTXOR} flags only request an operation and do
      not track the state of the operation.
      Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      600aa109
    • Dan Williams's avatar
      md: replace STRIPE_OP_COMPUTE_BLK with STRIPE_COMPUTE_RUN · 976ea8d4
      Dan Williams authored
      From: Dan Williams <dan.j.williams@intel.com>
      
      Track the state of compute operations (recalculating a block from all the other
      blocks in a stripe) with a state flag.  Reduces the scope of the
      STRIPE_OP_COMPUTE_BLK flag to only tracking whether a compute operation has
      been requested via the ops_request field of struct stripe_head_state.
      
      Note, the compute operation that is performed in the course of doing a 'repair'
      operation (check the parity block, recalculate it and write it back if the
      check result is not zero) is tracked separately with the 'check_state'
      variable.  Compute operations are held off while a 'check' is in progress, and
      moving this check out to handle_issuing_new_read_requests5 the helper routine
      __handle_issuing_new_read_requests5 can be simplified.
      
      This is another step towards the removal of ops.{pending,ack,complete,count},
      i.e. STRIPE_OP_COMPUTE_BLK only requests an operation and does not track the
      state of the operation.
      Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      976ea8d4
    • Dan Williams's avatar
      md: replace STRIPE_OP_BIOFILL with STRIPE_BIOFILL_RUN · 83de75cc
      Dan Williams authored
      From: Dan Williams <dan.j.williams@intel.com>
      
      Track the state of read operations (copying data from the stripe cache to bio
      buffers outside the lock) with a state flag.  Reduce the scope of the
      STRIPE_OP_BIOFILL flag to only tracking whether a biofill operation has been
      requested via the ops_request field of struct stripe_head_state.
      
      This is another step towards the removal of ops.{pending,ack,complete,count},
      i.e. STRIPE_OP_BIOFILL only requests an operation and does not track the state
      of the operation.
      Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      83de75cc
    • Dan Williams's avatar
      md: replace STRIPE_OP_CHECK with 'check_states' · ecc65c9b
      Dan Williams authored
      From: Dan Williams <dan.j.williams@intel.com>
      
      The STRIPE_OP_* flags record the state of stripe operations which are
      performed outside the stripe lock.  Their use in indicating which
      operations need to be run is straightforward; however, interpolating what
      the next state of the stripe should be based on a given combination of
      these flags is not straightforward, and has led to bugs.  An easier to read
      implementation with minimal degrees of freedom is needed.
      
      Towards this goal, this patch introduces explicit states to replace what was
      previously interpolated from the STRIPE_OP_* flags.  For now this only converts
      the handle_parity_checks5 path, removing a user of the
      ops.{pending,ack,complete,count} fields of struct stripe_operations.
      
      This conversion also found a remaining issue with the current code.  There is
      a small window for a drive to fail between when we schedule a repair and when
      the parity calculation for that repair completes.  When this happens we will
      writeback to 'failed_num' when we really want to write back to 'pd_idx'.
      Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      ecc65c9b
    • Dan Williams's avatar
      md: unify raid5/6 i/o submission · f0e43bcd
      Dan Williams authored
      From: Dan Williams <dan.j.williams@intel.com>
      
      Let the raid6 path call ops_run_io to get pending i/o submitted.
      Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      f0e43bcd
    • Dan Williams's avatar
      md: use stripe_head_state in ops_run_io() · c4e5ac0a
      Dan Williams authored
      From: Dan Williams <dan.j.williams@intel.com>
      
      In handle_stripe after taking sh->lock we sample some bits into 's' (struct
      stripe_head_state):
      
      	s.syncing = test_bit(STRIPE_SYNCING, &sh->state);
      	s.expanding = test_bit(STRIPE_EXPAND_SOURCE, &sh->state);
      	s.expanded = test_bit(STRIPE_EXPAND_READY, &sh->state);
      
      Use these values from 's' in ops_run_io() rather than re-sampling the bits.
      This ensures a consistent snapshot (as seen under sh->lock) is used.
      Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      c4e5ac0a
    • Dan Williams's avatar
      md: kill STRIPE_OP_IO flag · 2b7497f0
      Dan Williams authored
      From: Dan Williams <dan.j.williams@intel.com>
      
      The R5_Want{Read,Write} flags already gate i/o.  So, this flag is
      superfluous and we can unconditionally call ops_run_io().
      Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      2b7497f0
    • Dan Williams's avatar
      md: kill STRIPE_OP_MOD_DMA in raid5 offload · b203886e
      Dan Williams authored
      From: Dan Williams <dan.j.williams@intel.com>
      
      This micro-optimization allowed the raid code to skip a re-read of the
      parity block after checking parity.  It took advantage of the fact that
      xor-offload-engines have their own internal result buffer and can check
      parity without writing to memory.  Remove it for the following reasons:
      
      1/ It is a layering violation for MD to need to manage the DMA and
         non-DMA paths within async_xor_zero_sum
      2/ Bad precedent to toggle the 'ops' flags outside the lock
      3/ Hard to realize a performance gain as reads will not need an updated
         parity block and writes will dirty it anyways.
      Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      b203886e
    • Chris Webb's avatar
      Support changing rdev size on running arrays. · 0cd17fec
      Chris Webb authored
      From: Chris Webb <chris@arachsys.com>
      
      Allow /sys/block/mdX/md/rdY/size to change on running arrays, moving the
      superblock if necessary for this metadata version. We prevent the available
      space from shrinking to less than the used size, and allow it to be set to zero
      to fill all the available space on the underlying device.
      Signed-off-by: default avatarChris Webb <chris@arachsys.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      0cd17fec
    • Neil Brown's avatar
      Make sure all changes to md/dev-XX/state are notified · 52664732
      Neil Brown authored
      The important state change happens during an interrupt
      in md_error.  So just set a flag there and call sysfs_notify
      later in process context.
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      52664732
    • Neil Brown's avatar
      Make sure all changes to md/degraded are notified. · a99ac971
      Neil Brown authored
      When a device fails, when a spare is activated, when
      an array is reshaped, or when an array is started,
      the extent to which the array is degraded can change.
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      a99ac971
    • Neil Brown's avatar
      Make sure all changes to md/sync_action are notified. · 72a23c21
      Neil Brown authored
      When the 'resync' thread starts or stops, when we explicitly
      set sync_action, or when we determine that there is definitely nothing
      to do, we notify sync_action.
      
      To stop "sync_action" from occasionally showing the wrong value,
      we introduce a new flags - MD_RECOVERY_RECOVER - to say that a
      recovery is probably needed or happening, and we make sure
      that we set MD_RECOVERY_RUNNING before clearing MD_RECOVERY_NEEDED.
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      72a23c21
    • Neil Brown's avatar
      Make sure all changes to md/array_state are notified. · 0fd62b86
      Neil Brown authored
      Changes in md/array_state could be of interest to a monitoring
      program.  So make sure all changes trigger a notification.
      
      Exceptions:
         changing active_idle to active is not reported because it
            is frequent and not interesting.
         changing active to active_idle is only reported on arrays
            with externally managed metadata, as it is not interesting
            otherwise.
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      0fd62b86
    • Neil Brown's avatar
      Don't reject HOT_REMOVE_DISK request for an array that is not yet started. · c7d0c941
      Neil Brown authored
      There is really no need for this test here, and there are valid
      cases for selectively removing devices from an array that
      it not actually active.
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      c7d0c941
    • Neil Brown's avatar
      rationalise return value for ->hot_add_disk method. · 199050ea
      Neil Brown authored
      For all array types but linear, ->hot_add_disk returns 1 on
      success, 0 on failure.
      For linear, it returns 0 on success and -errno on failure.
      
      This doesn't cause a functional problem because the ->hot_add_disk
      function of linear is used quite differently to the others.
      However it is confusing.
      
      So convert all to return 0 for success or -errno on failure
      and fix call sites to match.
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      199050ea
    • Neil Brown's avatar
      Support adding a spare to a live md array with external metadata. · 6c2fce2e
      Neil Brown authored
      i.e. extend the 'md/dev-XXX/slot' attribute so that you can
      tell a device to fill an vacant slot in an and md array.
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      6c2fce2e
    • Neil Brown's avatar
      Enable setting of 'offset' and 'size' of a hot-added spare. · 8ed0a521
      Neil Brown authored
      offset_store and rdev_size_store allow control of the region of a
      device which is to be using in an md/raid array.
      They only allow these values to be set when an array is being assembled,
      as changing them on an active array could be dangerous.
      However when adding a spare device to an array, we might need to
      set the offset and size before starting recovery.  So allow
      these values to be set also if "->raid_disk < 0" which indicates that
      the device is still a spare.
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      8ed0a521
    • Neil Brown's avatar
      Don't try to make md arrays dirty if that is not meaningful. · 1a0fd497
      Neil Brown authored
      Arrays personalities such as 'raid0' and 'linear' have no redundancy,
      and so marking them as 'clean' or 'dirty' is not meaningful.
      So always allow write requests without requiring a superblock update.
      
      Such arrays types are detected by ->sync_request being NULL.  If it is
      not possible to send a sync request we don't need a 'dirty' flag because
      all a dirty flag does is trigger some sync_requests.
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      1a0fd497
    • Neil Brown's avatar
      Close race in md_probe · f48ed538
      Neil Brown authored
      There is a possible race in md_probe.  If two threads call md_probe
      for the same device, then one could exit (having checked that
      ->gendisk exists) before the other has called kobject_init_and_add,
      thus returning an incomplete kobj which will cause problems when
      we try to add children to it.
      
      So extend the range of protection of disks_mutex slightly to
      avoid this possibility.
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      f48ed538
    • Neil Brown's avatar
      Allow setting start point for requested check/repair · 5e96ee65
      Neil Brown authored
      This makes it possible to just resync a small part of an array.
      e.g. if a drive reports that it has questionable sectors,
      a 'repair' of just the region covering those sectors will
      cause them to be read and, if there is an error, re-written
      with correct data.
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      5e96ee65
    • Neil Brown's avatar
      Improve setting of "events_cleared" for write-intent bitmaps. · a0da84f3
      Neil Brown authored
      When an array is degraded, bits in the write-intent bitmap are not
      cleared, so that if the missing device is re-added, it can be synced
      by only updated those parts of the device that have changed since
      it was removed.
      
      The enable this a 'events_cleared' value is stored. It is the event
      counter for the array the last time that any bits were cleared.
      
      Sometimes - if a device disappears from an array while it is 'clean' -
      the events_cleared value gets updated incorrectly (there are subtle
      ordering issues between updateing events in the main metadata and the
      bitmap metadata) resulting in the missing device appearing to require
      a full resync when it is re-added.
      
      With this patch, we update events_cleared precisely when we are about
      to clear a bit in the bitmap.  We record events_cleared when we clear
      the bit internally, and copy that to the superblock which is written
      out before the bit on storage.  This makes it more "obviously correct".
      
      We also need to update events_cleared when the event_count is going
      backwards (as happens on a dirty->clean transition of a non-degraded
      array).
      
      Thanks to Mike Snitzer for identifying this problem and testing early
      "fixes".
      
      Cc:  "Mike Snitzer" <snitzer@gmail.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      a0da84f3
    • Neil Brown's avatar
      use bio_endio instead of a call to bi_end_io · 0e13fe23
      Neil Brown authored
      Turn calls to bi->bi_end_io() into bio_endio(). Apparently bio_endio does
      exactly the same error processing as is hardcoded at these places.
      
      bio_endio() avoids recursion (or will soon), so it should be used.
      Signed-off-by: default avatarMikulas Patocka <mpatocka@redhat.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      0e13fe23
    • Nikanth Karthikesan's avatar
      linear: correct disk numbering error check · 13864515
      Nikanth Karthikesan authored
      From: "Nikanth Karthikesan" <knikanth@novell.com>
      
      Correct disk numbering problem check.
      Signed-off-by: default avatarNikanth Karthikesan <knikanth@suse.de>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      13864515
    • Neil Brown's avatar
      Fix error paths if md_probe fails. · 9bbbca3a
      Neil Brown authored
      md_probe can fail (e.g. alloc_disk could fail) without
      returning an error (as it alway returns NULL).
      So when we call mddev_find immediately afterwards, we need
      to check that md_probe actually succeeded.  This means checking
      that mdev->gendisk is non-NULL.
      
      cc: <stable@kernel.org>
      Cc: Dave Jones <davej@redhat.com>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      9bbbca3a
    • Neil Brown's avatar
      Don't acknowlege that stripe-expand is complete until it really is. · efe31143
      Neil Brown authored
      We shouldn't acknowledge that a stripe has been expanded (When
      reshaping a raid5 by adding a device) until the moved data has
      actually been written out.  However we are currently
      acknowledging (by calling md_done_sync) when the POST_XOR
      is complete and before the write.
      
      So track in s.locked whether there are pending writes, and don't
      call md_done_sync yet if there are.
      
      Note: we all set R5_LOCKED on devices which are are about to
      read from.  This probably isn't technically necessary, but is
      usually done when writing a block, and justifies the use of
      s.locked here.
      
      This bug can lead to a crash if an array is stopped while an reshape
      is in progress.
      
      Cc: <stable@kernel.org>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      efe31143
    • Neil Brown's avatar
      Ensure interrupted recovery completed properly (v1 metadata plus bitmap) · 8c2e870a
      Neil Brown authored
      If, while assembling an array, we find a device which is not fully
      in-sync with the array, it is important to set the "fullsync" flags.
      This is an exact analog to the setting of this flag in hot_add_disk
      methods.
      
      Currently, only v1.x metadata supports having devices in an array
      which are not fully in-sync (it keep track of how in sync they are).
      The 'fullsync' flag only makes a difference when a write-intent bitmap
      is being used.  In this case it tells recovery to ignore the bitmap
      and recovery all blocks.
      
      This fix is already in place for raid1, but not raid5/6 or raid10.
      
      So without this fix, a raid1 ir raid4/5/6 array with version 1.x
      metadata and a write intent bitmaps, that is stopped in the middle
      of a recovery, will appear to complete the recovery instantly
      after it is reassembled, but the recovery will not be correct.
      
      If you might have an array like that, issueing
         echo repair > /sys/block/mdXX/md/sync_action
      
      will make sure recovery completes properly.
      
      Cc: <stable@kernel.org>
      Signed-off-by: default avatarNeil Brown <neilb@suse.de>
      8c2e870a
  3. 25 Jun, 2008 4 commits
  4. 24 Jun, 2008 6 commits