* [PATCH 1/5] remove_and_add_spares() to activate specific rdev
2015-10-05 15:09 [PATCH 0/5 v2] md-cluster: A better way for METADATA_UPDATED processing rgoldwyn
@ 2015-10-05 15:09 ` rgoldwyn
2015-10-05 15:09 ` [PATCH 2/5] md-cluster: Improve md_reload_sb to be less error prone rgoldwyn
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: rgoldwyn @ 2015-10-05 15:09 UTC (permalink / raw)
To: linux-raid, neilb; +Cc: Goldwyn Rodrigues
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
remove_and_add_spares() checks for all devices to activate spare.
Change it to activate a specific device if a non-null rdev
argument is passed.
remove_and_add_spares() can be used to activate spares in
slot_store() as well.
For hot_remove_disk(), check if rdev->raid_disk == -1 before
calling remove_and_add_spares()
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
drivers/md/md.c | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 9798a99..fcdf47b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2691,15 +2691,9 @@ slot_store(struct md_rdev *rdev, const char *buf, size_t len)
rdev->saved_raid_disk = -1;
clear_bit(In_sync, &rdev->flags);
clear_bit(Bitmap_sync, &rdev->flags);
- err = rdev->mddev->pers->
- hot_add_disk(rdev->mddev, rdev);
- if (err) {
- rdev->raid_disk = -1;
- return err;
- } else
- sysfs_notify_dirent_safe(rdev->sysfs_state);
- if (sysfs_link_rdev(rdev->mddev, rdev))
- /* failure here is OK */;
+ remove_and_add_spares(rdev->mddev, rdev);
+ if (rdev->raid_disk == -1)
+ return -EBUSY;
/* don't wakeup anyone, leave that to userspace. */
} else {
if (slot >= rdev->mddev->raid_disks &&
@@ -6001,15 +5995,15 @@ static int hot_remove_disk(struct mddev *mddev, dev_t dev)
if (!rdev)
return -ENXIO;
+ if (rdev->raid_disk >= 0)
+ goto busy;
+
if (mddev_is_clustered(mddev))
md_cluster_ops->metadata_update_start(mddev);
clear_bit(Blocked, &rdev->flags);
remove_and_add_spares(mddev, rdev);
- if (rdev->raid_disk >= 0)
- goto busy;
-
if (mddev_is_clustered(mddev))
md_cluster_ops->remove_disk(mddev, rdev);
@@ -6022,8 +6016,6 @@ static int hot_remove_disk(struct mddev *mddev, dev_t dev)
return 0;
busy:
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_cancel(mddev);
printk(KERN_WARNING "md: cannot remove active disk %s from %s ...\n",
bdevname(rdev->bdev,b), mdname(mddev));
return -EBUSY;
@@ -8018,10 +8010,12 @@ static int remove_and_add_spares(struct mddev *mddev,
if (removed && mddev->kobj.sd)
sysfs_notify(&mddev->kobj, NULL, "degraded");
- if (this)
+ if (this && removed)
goto no_add;
rdev_for_each(rdev, mddev) {
+ if (this && this != rdev)
+ continue;
if (rdev->raid_disk >= 0 &&
!test_bit(In_sync, &rdev->flags) &&
!test_bit(Faulty, &rdev->flags))
--
1.8.5.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/5] md-cluster: Improve md_reload_sb to be less error prone
2015-10-05 15:09 [PATCH 0/5 v2] md-cluster: A better way for METADATA_UPDATED processing rgoldwyn
2015-10-05 15:09 ` [PATCH 1/5] remove_and_add_spares() to activate specific rdev rgoldwyn
@ 2015-10-05 15:09 ` rgoldwyn
2015-10-05 15:09 ` [PATCH 3/5] md-cluster: Perform a lazy update rgoldwyn
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: rgoldwyn @ 2015-10-05 15:09 UTC (permalink / raw)
To: linux-raid, neilb; +Cc: Goldwyn Rodrigues
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
md_reload_sb is too simplistic and it explicitly needs to determine
the changes made by the writing node. However, there are multiple areas
where a simple reload could fail.
Instead, read the superblock of one of the "good" rdevs and update
the necessary information:
- read the superblock into a newly allocated page, by temporarily
swapping out rdev->sb_page and calling ->load_super.
- if that fails return
- if it succeeds, call check_sb_changes
1. iterates over list of active devices and checks the matching
dev_roles[] value.
If that is 'faulty', the device must be marked as faulty
- call md_error to mark the device as faulty. Make sure
not to set CHANGE_DEVS and wakeup mddev->thread or else
it would initiate a resync process, which is the responsibility
of the "primary" node.
- clear the Blocked bit
- Call remove_and_add_spares() to hot remove the device.
If the device is 'spare':
- call remove_and_add_spares() to get the number of spares
added in this operation.
- Reduce mddev->degraded to mark the array as not degraded.
2. reset recovery_cp
- read the rest of the rdevs to update recovery_offset
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
drivers/md/md-cluster.c | 27 ++++++-----
drivers/md/md.c | 120 ++++++++++++++++++++++++++++++++++++++++++------
drivers/md/md.h | 2 +-
drivers/md/raid1.c | 2 +
4 files changed, 124 insertions(+), 27 deletions(-)
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index b94a2e6..4645a93 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -424,8 +424,7 @@ static void process_add_new_disk(struct mddev *mddev, struct cluster_msg *cmsg)
static void process_metadata_update(struct mddev *mddev, struct cluster_msg *msg)
{
struct md_cluster_info *cinfo = mddev->cluster_info;
-
- md_reload_sb(mddev);
+ md_reload_sb(mddev, le32_to_cpu(msg->raid_slot));
dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_CR);
}
@@ -811,11 +810,23 @@ static int metadata_update_finish(struct mddev *mddev)
{
struct md_cluster_info *cinfo = mddev->cluster_info;
struct cluster_msg cmsg;
- int ret;
+ struct md_rdev *rdev;
+ int ret = 0;
memset(&cmsg, 0, sizeof(cmsg));
cmsg.type = cpu_to_le32(METADATA_UPDATED);
- ret = __sendmsg(cinfo, &cmsg);
+ cmsg.raid_slot = -1;
+ /* Pick up a good active device number to send.
+ */
+ rdev_for_each(rdev, mddev)
+ if (rdev->raid_disk > -1 && !test_bit(Faulty, &rdev->flags)) {
+ cmsg.raid_slot = cpu_to_le32(rdev->desc_nr);
+ break;
+ }
+ if (cmsg.raid_slot >= 0)
+ ret = __sendmsg(cinfo, &cmsg);
+ else
+ pr_warn("md-cluster: No good device id found to send\n");
unlock_comm(cinfo);
return ret;
}
@@ -899,15 +910,9 @@ static int add_new_disk_start(struct mddev *mddev, struct md_rdev *rdev)
static int add_new_disk_finish(struct mddev *mddev)
{
- struct cluster_msg cmsg;
- struct md_cluster_info *cinfo = mddev->cluster_info;
- int ret;
/* Write sb and inform others */
md_update_sb(mddev, 1);
- cmsg.type = METADATA_UPDATED;
- ret = __sendmsg(cinfo, &cmsg);
- unlock_comm(cinfo);
- return ret;
+ return metadata_update_finish(mddev);
}
static int new_disk_ack(struct mddev *mddev, bool ack)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index fcdf47b..36e751e 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -7987,7 +7987,7 @@ void md_do_sync(struct md_thread *thread)
EXPORT_SYMBOL_GPL(md_do_sync);
static int remove_and_add_spares(struct mddev *mddev,
- struct md_rdev *this)
+ struct md_rdev *this)
{
struct md_rdev *rdev;
int spares = 0;
@@ -8917,25 +8917,115 @@ err_wq:
return ret;
}
-void md_reload_sb(struct mddev *mddev)
+static void check_sb_changes(struct mddev *mddev, struct md_rdev *rdev)
{
- struct md_rdev *rdev, *tmp;
+ struct mdp_superblock_1 *sb = page_address(rdev->sb_page);
+ struct md_rdev *rdev2;
+ int role, ret;
+ char b[BDEVNAME_SIZE];
- rdev_for_each_safe(rdev, tmp, mddev) {
- rdev->sb_loaded = 0;
- ClearPageUptodate(rdev->sb_page);
+ /* Check for change of roles in the active devices */
+ rdev_for_each(rdev2, mddev) {
+ if (test_bit(Faulty, &rdev2->flags))
+ continue;
+
+ /* Check if the roles changed */
+ role = le16_to_cpu(sb->dev_roles[rdev2->desc_nr]);
+ if (role != rdev2->raid_disk) {
+ /* got activated */
+ if (rdev2->raid_disk == -1 && role != 0xffff) {
+ rdev2->saved_raid_disk = role;
+ ret = remove_and_add_spares(mddev, rdev2);
+ pr_info("Activated spare: %s\n",
+ bdevname(rdev2->bdev,b));
+ continue;
+ }
+ /* device faulty
+ * We just want to do the minimum to mark the disk
+ * as faulty. The recovery is performed by the
+ * one who initiated the error.
+ */
+ if ((role == 0xfffe) || (role == 0xfffd)) {
+ md_error(mddev, rdev2);
+ clear_bit(Blocked, &rdev2->flags);
+ remove_and_add_spares(mddev, rdev2);
+ }
+ }
}
- mddev->raid_disks = 0;
- analyze_sbs(mddev);
- rdev_for_each_safe(rdev, tmp, mddev) {
- struct mdp_superblock_1 *sb = page_address(rdev->sb_page);
- /* since we don't write to faulty devices, we figure out if the
- * disk is faulty by comparing events
- */
- if (mddev->events > sb->events)
- set_bit(Faulty, &rdev->flags);
+
+ /* recovery_cp changed */
+ if (le64_to_cpu(sb->resync_offset) != mddev->recovery_cp) {
+ pr_info("%s:%d recovery_cp changed from %lu to %lu\n", __func__,
+ __LINE__, mddev->recovery_cp,
+ (unsigned long) le64_to_cpu(sb->resync_offset));
+ mddev->recovery_cp = le64_to_cpu(sb->resync_offset);
}
+ /* Finally set the event to be up to date */
+ mddev->events = le64_to_cpu(sb->events);
+}
+
+static int read_rdev(struct mddev *mddev, struct md_rdev *rdev)
+{
+ int err;
+ struct page *swapout = rdev->sb_page;
+ struct mdp_superblock_1 *sb;
+
+ /* Store the sb page of the rdev in the swapout temporary
+ * variable in case we err in the future
+ */
+ rdev->sb_page = NULL;
+ alloc_disk_sb(rdev);
+ ClearPageUptodate(rdev->sb_page);
+ rdev->sb_loaded = 0;
+ err = super_types[mddev->major_version].load_super(rdev, NULL, mddev->minor_version);
+
+ if (err < 0) {
+ pr_warn("%s: %d Could not reload rdev(%d) err: %d. Restoring old values\n",
+ __func__, __LINE__, rdev->desc_nr, err);
+ put_page(rdev->sb_page);
+ rdev->sb_page = swapout;
+ rdev->sb_loaded = 1;
+ return err;
+ }
+
+ sb = page_address(rdev->sb_page);
+ rdev->recovery_offset = le64_to_cpu(sb->recovery_offset);
+
+ put_page(swapout);
+ return 0;
+}
+
+void md_reload_sb(struct mddev *mddev, int nr)
+{
+ struct md_rdev *rdev;
+ int err;
+
+ /* Find the rdev */
+ rdev_for_each_rcu(rdev, mddev) {
+ if (rdev->desc_nr == nr)
+ break;
+ }
+
+ if (!rdev || rdev->desc_nr != nr) {
+ pr_warn("%s: %d Could not find rdev with nr %d\n", __func__, __LINE__, nr);
+ return;
+ }
+
+ err = read_rdev(mddev, rdev);
+ if (err < 0)
+ return;
+
+ check_sb_changes(mddev, rdev);
+
+ /* Read the rest of the rdev's to update recovery_offset */
+ rdev_for_each_rcu(rdev, mddev) {
+ /* We have already read this one */
+ if (rdev->desc_nr == nr)
+ continue;
+
+ read_rdev(mddev, rdev);
+ }
}
EXPORT_SYMBOL(md_reload_sb);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index ab33957..2ea0035 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -658,7 +658,7 @@ extern struct bio *bio_alloc_mddev(gfp_t gfp_mask, int nr_iovecs,
struct mddev *mddev);
extern void md_unplug(struct blk_plug_cb *cb, bool from_schedule);
-extern void md_reload_sb(struct mddev *mddev);
+extern void md_reload_sb(struct mddev *mddev, int raid_disk);
extern void md_update_sb(struct mddev *mddev, int force);
extern void md_kick_rdev_from_array(struct md_rdev * rdev);
struct md_rdev *md_find_rdev_nr_rcu(struct mddev *mddev, int nr);
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 1dd13bb..03752bd 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1608,6 +1608,8 @@ static int raid1_add_disk(struct mddev *mddev, struct md_rdev *rdev)
*/
if (rdev->saved_raid_disk < 0)
conf->fullsync = 1;
+ else if (mddev_is_clustered(mddev))
+ rdev->raid_disk = rdev->saved_raid_disk;
rcu_assign_pointer(p->rdev, rdev);
break;
}
--
1.8.5.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/5] md-cluster: Perform a lazy update
2015-10-05 15:09 [PATCH 0/5 v2] md-cluster: A better way for METADATA_UPDATED processing rgoldwyn
2015-10-05 15:09 ` [PATCH 1/5] remove_and_add_spares() to activate specific rdev rgoldwyn
2015-10-05 15:09 ` [PATCH 2/5] md-cluster: Improve md_reload_sb to be less error prone rgoldwyn
@ 2015-10-05 15:09 ` rgoldwyn
2015-10-05 15:09 ` [PATCH 4/5] Perform resync/recovery under a DLM lock rgoldwyn
2015-10-05 15:09 ` [PATCH 5/5] Fix adding of new disk with new reload code rgoldwyn
4 siblings, 0 replies; 6+ messages in thread
From: rgoldwyn @ 2015-10-05 15:09 UTC (permalink / raw)
To: linux-raid, neilb; +Cc: Goldwyn Rodrigues
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
In a clustered environment, a change such as marking a device faulty,
can be recorded by any of the nodes. This is communicated to all the
nodes and re-recording such a change is unnecessary, and quite often
pretty disruptive.
With this patch, just before the update, we detect for the changes
and if the changes are already in superblock, we abort the update
after clearing all the flags
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
drivers/md/md.c | 101 ++++++++++++++++++++++++++++++++------------------------
1 file changed, 57 insertions(+), 44 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 36e751e..9abd070 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2199,6 +2199,46 @@ static void sync_sbs(struct mddev *mddev, int nospares)
}
}
+static bool does_sb_need_changing(struct mddev *mddev)
+{
+ struct md_rdev *rdev;
+ struct mdp_superblock_1 *sb;
+ int role;
+
+ /* Find a good rdev */
+ rdev_for_each_rcu(rdev, mddev)
+ if ((rdev->raid_disk >= 0) && !test_bit(Faulty, &rdev->flags))
+ break;
+
+ /* No good device found. */
+ if (!rdev)
+ return false;
+
+ sb = page_address(rdev->sb_page);
+ /* Check if a device has become faulty or a spare become active */
+ rdev_for_each(rdev, mddev) {
+ role = le16_to_cpu(sb->dev_roles[rdev->desc_nr]);
+ /* Device activated? */
+ if (role == 0xffff && rdev->raid_disk >=0 && !test_bit(Faulty, &rdev->flags))
+ return true;
+ /* Device turned faulty? */
+ if (test_bit(Faulty, &rdev->flags) && (role < 0xfffd))
+ return true;
+ }
+
+ /* Check if any mddev parameters have changed */
+ if ((mddev->dev_sectors != le64_to_cpu(sb->size)) ||
+ (mddev->reshape_position != le64_to_cpu(sb->reshape_position)) ||
+ (mddev->recovery_cp != le64_to_cpu(sb->resync_offset)) ||
+ (mddev->layout != le64_to_cpu(sb->layout)) ||
+ (mddev->raid_disks != le32_to_cpu(sb->raid_disks)) ||
+ (mddev->chunk_sectors != le32_to_cpu(sb->chunksize))
+ )
+ return true;
+
+ return false;
+}
+
void md_update_sb(struct mddev *mddev, int force_change)
{
struct md_rdev *rdev;
@@ -2211,6 +2251,18 @@ void md_update_sb(struct mddev *mddev, int force_change)
set_bit(MD_CHANGE_DEVS, &mddev->flags);
return;
}
+
+ if (mddev_is_clustered(mddev)) {
+ if (test_and_clear_bit(MD_CHANGE_DEVS, &mddev->flags))
+ force_change = 1;
+ md_cluster_ops->metadata_update_start(mddev);
+ /* Has someone else has updated the sb */
+ if (!does_sb_need_changing(mddev)) {
+ md_cluster_ops->metadata_update_cancel(mddev);
+ clear_bit(MD_CHANGE_PENDING, &mddev->flags);
+ return;
+ }
+ }
repeat:
/* First make sure individual recovery_offsets are correct */
rdev_for_each(rdev, mddev) {
@@ -2359,6 +2411,9 @@ repeat:
clear_bit(BlockedBadBlocks, &rdev->flags);
wake_up(&rdev->blocked_wait);
}
+
+ if (mddev_is_clustered(mddev))
+ md_cluster_ops->metadata_update_finish(mddev);
}
EXPORT_SYMBOL(md_update_sb);
@@ -2496,13 +2551,9 @@ state_store(struct md_rdev *rdev, const char *buf, size_t len)
if (mddev_is_clustered(mddev))
md_cluster_ops->remove_disk(mddev, rdev);
md_kick_rdev_from_array(rdev);
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_start(mddev);
if (mddev->pers)
md_update_sb(mddev, 1);
md_new_event(mddev);
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_finish(mddev);
err = 0;
}
} else if (cmd_match(buf, "writemostly")) {
@@ -4063,12 +4114,8 @@ size_store(struct mddev *mddev, const char *buf, size_t len)
if (err)
return err;
if (mddev->pers) {
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_start(mddev);
err = update_size(mddev, sectors);
md_update_sb(mddev, 1);
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_finish(mddev);
} else {
if (mddev->dev_sectors == 0 ||
mddev->dev_sectors > sectors)
@@ -5306,8 +5353,6 @@ static void md_clean(struct mddev *mddev)
static void __md_stop_writes(struct mddev *mddev)
{
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_start(mddev);
set_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
flush_workqueue(md_misc_wq);
if (mddev->sync_thread) {
@@ -5326,8 +5371,6 @@ static void __md_stop_writes(struct mddev *mddev)
mddev->in_sync = 1;
md_update_sb(mddev, 1);
}
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_finish(mddev);
}
void md_stop_writes(struct mddev *mddev)
@@ -6011,9 +6054,6 @@ static int hot_remove_disk(struct mddev *mddev, dev_t dev)
md_update_sb(mddev, 1);
md_new_event(mddev);
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_finish(mddev);
-
return 0;
busy:
printk(KERN_WARNING "md: cannot remove active disk %s from %s ...\n",
@@ -6066,14 +6106,12 @@ static int hot_add_disk(struct mddev *mddev, dev_t dev)
goto abort_export;
}
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_start(mddev);
clear_bit(In_sync, &rdev->flags);
rdev->desc_nr = -1;
rdev->saved_raid_disk = -1;
err = bind_rdev_to_array(rdev, mddev);
if (err)
- goto abort_clustered;
+ goto abort_export;
/*
* The rest should better be atomic, we can have disk failures
@@ -6083,9 +6121,6 @@ static int hot_add_disk(struct mddev *mddev, dev_t dev)
rdev->raid_disk = -1;
md_update_sb(mddev, 1);
-
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_finish(mddev);
/*
* Kick recovery, maybe this spare has to be added to the
* array immediately.
@@ -6095,9 +6130,6 @@ static int hot_add_disk(struct mddev *mddev, dev_t dev)
md_new_event(mddev);
return 0;
-abort_clustered:
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_cancel(mddev);
abort_export:
export_rdev(rdev);
return err;
@@ -6415,8 +6447,6 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
return rv;
}
}
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_start(mddev);
if (info->size >= 0 && mddev->dev_sectors / 2 != info->size)
rv = update_size(mddev, (sector_t)info->size * 2);
@@ -6474,12 +6504,8 @@ static int update_array_info(struct mddev *mddev, mdu_array_info_t *info)
}
}
md_update_sb(mddev, 1);
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_finish(mddev);
return rv;
err:
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_cancel(mddev);
return rv;
}
@@ -7592,11 +7618,7 @@ int md_allow_write(struct mddev *mddev)
mddev->safemode == 0)
mddev->safemode = 1;
spin_unlock(&mddev->lock);
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_start(mddev);
md_update_sb(mddev, 0);
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_finish(mddev);
sysfs_notify_dirent_safe(mddev->sysfs_state);
} else
spin_unlock(&mddev->lock);
@@ -8175,13 +8197,8 @@ void md_check_recovery(struct mddev *mddev)
sysfs_notify_dirent_safe(mddev->sysfs_state);
}
- if (mddev->flags & MD_UPDATE_SB_FLAGS) {
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_start(mddev);
+ if (mddev->flags & MD_UPDATE_SB_FLAGS)
md_update_sb(mddev, 0);
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_finish(mddev);
- }
if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) &&
!test_bit(MD_RECOVERY_DONE, &mddev->recovery)) {
@@ -8279,8 +8296,6 @@ void md_reap_sync_thread(struct mddev *mddev)
set_bit(MD_CHANGE_DEVS, &mddev->flags);
}
}
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_start(mddev);
if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
mddev->pers->finish_reshape)
mddev->pers->finish_reshape(mddev);
@@ -8293,8 +8308,6 @@ void md_reap_sync_thread(struct mddev *mddev)
rdev->saved_raid_disk = -1;
md_update_sb(mddev, 1);
- if (mddev_is_clustered(mddev))
- md_cluster_ops->metadata_update_finish(mddev);
clear_bit(MD_RECOVERY_RUNNING, &mddev->recovery);
clear_bit(MD_RECOVERY_DONE, &mddev->recovery);
clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
--
1.8.5.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/5] Perform resync/recovery under a DLM lock
2015-10-05 15:09 [PATCH 0/5 v2] md-cluster: A better way for METADATA_UPDATED processing rgoldwyn
` (2 preceding siblings ...)
2015-10-05 15:09 ` [PATCH 3/5] md-cluster: Perform a lazy update rgoldwyn
@ 2015-10-05 15:09 ` rgoldwyn
2015-10-05 15:09 ` [PATCH 5/5] Fix adding of new disk with new reload code rgoldwyn
4 siblings, 0 replies; 6+ messages in thread
From: rgoldwyn @ 2015-10-05 15:09 UTC (permalink / raw)
To: linux-raid, neilb; +Cc: Goldwyn Rodrigues
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
Resync or recovery must be performed by only one node at a time.
A DLM lock resource, resync_lockres provides the mutual exclusion
so that only one node performs the recovery/resync at a time.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
drivers/md/md-cluster.c | 29 +++++++++++++++++++++++++++++
drivers/md/md-cluster.h | 1 +
drivers/md/md.c | 17 ++++++++++++++---
3 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
index 4645a93..17f24bf 100644
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -46,6 +46,7 @@ struct resync_info {
#define MD_CLUSTER_WAITING_FOR_NEWDISK 1
#define MD_CLUSTER_SUSPEND_READ_BALANCING 2
#define MD_CLUSTER_BEGIN_JOIN_CLUSTER 3
+#define MD_CLUSTER_RECOVERY_NEEDED 4
struct md_cluster_info {
@@ -55,6 +56,7 @@ struct md_cluster_info {
struct completion completion;
struct mutex sb_mutex;
struct dlm_lock_resource *bitmap_lockres;
+ struct dlm_lock_resource *resync_lockres;
struct list_head suspend_list;
spinlock_t suspend_lock;
struct md_thread *recovery_thread;
@@ -382,6 +384,8 @@ static void process_suspend_info(struct mddev *mddev,
if (!hi) {
remove_suspend_info(cinfo, slot);
+ if (test_and_clear_bit(MD_CLUSTER_RECOVERY_NEEDED, &cinfo->state))
+ set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
return;
}
s = kzalloc(sizeof(struct suspend_info), GFP_KERNEL);
@@ -755,6 +759,10 @@ static int join(struct mddev *mddev, int nodes)
goto err;
}
+ cinfo->resync_lockres = lockres_init(mddev, "resync", NULL, 0);
+ if (!cinfo->resync_lockres)
+ goto err;
+
ret = gather_all_resync_info(mddev, nodes);
if (ret)
goto err;
@@ -765,6 +773,7 @@ err:
lockres_free(cinfo->token_lockres);
lockres_free(cinfo->ack_lockres);
lockres_free(cinfo->no_new_dev_lockres);
+ lockres_free(cinfo->resync_lockres);
lockres_free(cinfo->bitmap_lockres);
if (cinfo->lockspace)
dlm_release_lockspace(cinfo->lockspace, 2);
@@ -838,6 +847,21 @@ static int metadata_update_cancel(struct mddev *mddev)
return dlm_unlock_sync(cinfo->token_lockres);
}
+static int resync_start(struct mddev *mddev)
+{
+ struct md_cluster_info *cinfo = mddev->cluster_info;
+ int ret = 0;
+ cinfo->resync_lockres->flags |= DLM_LKF_NOQUEUE;
+ ret = dlm_lock_sync(cinfo->resync_lockres, DLM_LOCK_EX);
+ if (ret) {
+ if (!test_bit(MD_CLUSTER_RECOVERY_NEEDED, &cinfo->state))
+ pr_info("md-cluster: Postponing recovery until other nodes finish recovery\n");
+ set_bit(MD_CLUSTER_RECOVERY_NEEDED, &cinfo->state);
+ } else
+ clear_bit(MD_CLUSTER_RECOVERY_NEEDED, &cinfo->state);
+ return ret;
+}
+
static int resync_info_update(struct mddev *mddev, sector_t lo, sector_t hi)
{
struct md_cluster_info *cinfo = mddev->cluster_info;
@@ -854,6 +878,10 @@ static int resync_info_update(struct mddev *mddev, sector_t lo, sector_t hi)
cmsg.slot = cpu_to_le32(slot);
cmsg.low = cpu_to_le64(lo);
cmsg.high = cpu_to_le64(hi);
+
+ /* Resync is finished */
+ if (hi == 0)
+ dlm_unlock_sync(cinfo->resync_lockres);
return sendmsg(cinfo, &cmsg);
}
@@ -972,6 +1000,7 @@ static struct md_cluster_operations cluster_ops = {
.join = join,
.leave = leave,
.slot_number = slot_number,
+ .resync_start = resync_start,
.resync_info_update = resync_info_update,
.metadata_update_start = metadata_update_start,
.metadata_update_finish = metadata_update_finish,
diff --git a/drivers/md/md-cluster.h b/drivers/md/md-cluster.h
index f5bdc0c..7061020 100644
--- a/drivers/md/md-cluster.h
+++ b/drivers/md/md-cluster.h
@@ -16,6 +16,7 @@ struct md_cluster_operations {
int (*metadata_update_start)(struct mddev *mddev);
int (*metadata_update_finish)(struct mddev *mddev);
int (*metadata_update_cancel)(struct mddev *mddev);
+ int (*resync_start)(struct mddev *mddev);
int (*area_resyncing)(struct mddev *mddev, int direction, sector_t lo, sector_t hi);
int (*add_new_disk_start)(struct mddev *mddev, struct md_rdev *rdev);
int (*add_new_disk_finish)(struct mddev *mddev);
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 9abd070..75d14e3 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8071,14 +8071,25 @@ no_add:
static void md_start_sync(struct work_struct *ws)
{
struct mddev *mddev = container_of(ws, struct mddev, del_work);
+ int ret = 0;
+
+ if (mddev_is_clustered(mddev)) {
+ ret = md_cluster_ops->resync_start(mddev);
+ if (ret) {
+ mddev->sync_thread = NULL;
+ goto out;
+ }
+ }
mddev->sync_thread = md_register_thread(md_do_sync,
mddev,
"resync");
+out:
if (!mddev->sync_thread) {
- printk(KERN_ERR "%s: could not start resync"
- " thread...\n",
- mdname(mddev));
+ if (!mddev_is_clustered(mddev))
+ printk(KERN_ERR "%s: could not start resync"
+ " thread...\n",
+ mdname(mddev));
/* leave the spares where they are, it shouldn't hurt */
clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
clear_bit(MD_RECOVERY_RESHAPE, &mddev->recovery);
--
1.8.5.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 5/5] Fix adding of new disk with new reload code
2015-10-05 15:09 [PATCH 0/5 v2] md-cluster: A better way for METADATA_UPDATED processing rgoldwyn
` (3 preceding siblings ...)
2015-10-05 15:09 ` [PATCH 4/5] Perform resync/recovery under a DLM lock rgoldwyn
@ 2015-10-05 15:09 ` rgoldwyn
4 siblings, 0 replies; 6+ messages in thread
From: rgoldwyn @ 2015-10-05 15:09 UTC (permalink / raw)
To: linux-raid, neilb; +Cc: Goldwyn Rodrigues
From: Goldwyn Rodrigues <rgoldwyn@suse.com>
Adding the disk worked incorrectly with the new reload code. Fix it:
- No operation should be performed on rdev marked as Candidate
- After a metadata update operation, kick disk if role is 0xfffe
else clear Candidate bit and continue with the regular change check.
- On the initiating node, cancel the metadata_update (unlock token),
in case of an error while adding
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
drivers/md/md.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 75d14e3..e324544 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -3246,14 +3246,6 @@ static void analyze_sbs(struct mddev *mddev)
md_kick_rdev_from_array(rdev);
continue;
}
- /* No device should have a Candidate flag
- * when reading devices
- */
- if (test_bit(Candidate, &rdev->flags)) {
- pr_info("md: kicking Cluster Candidate %s from array!\n",
- bdevname(rdev->bdev, b));
- md_kick_rdev_from_array(rdev);
- }
}
if (mddev->level == LEVEL_MULTIPATH) {
rdev->desc_nr = i++;
@@ -5962,7 +5954,7 @@ static int add_new_disk(struct mddev *mddev, mdu_disk_info_t *info)
/* --add initiated by this node */
err = md_cluster_ops->add_new_disk_start(mddev, rdev);
if (err) {
- md_cluster_ops->add_new_disk_finish(mddev);
+ md_cluster_ops->metadata_update_cancel(mddev);
export_rdev(rdev);
return err;
}
@@ -5973,11 +5965,11 @@ static int add_new_disk(struct mddev *mddev, mdu_disk_info_t *info)
err = bind_rdev_to_array(rdev, mddev);
if (err)
export_rdev(rdev);
- else
+ else if (!(info->state & (1 << MD_DISK_CANDIDATE)))
err = add_bound_rdev(rdev);
- if (mddev_is_clustered(mddev) &&
+ if (err && mddev_is_clustered(mddev) &&
(info->state & (1 << MD_DISK_CLUSTER_ADD)))
- md_cluster_ops->add_new_disk_finish(mddev);
+ md_cluster_ops->metadata_update_cancel(mddev);
return err;
}
@@ -8038,6 +8030,8 @@ static int remove_and_add_spares(struct mddev *mddev,
rdev_for_each(rdev, mddev) {
if (this && this != rdev)
continue;
+ if (test_bit(Candidate, &rdev->flags))
+ continue;
if (rdev->raid_disk >= 0 &&
!test_bit(In_sync, &rdev->flags) &&
!test_bit(Faulty, &rdev->flags))
@@ -8955,6 +8949,19 @@ static void check_sb_changes(struct mddev *mddev, struct md_rdev *rdev)
/* Check if the roles changed */
role = le16_to_cpu(sb->dev_roles[rdev2->desc_nr]);
+
+ if (test_bit(Candidate, &rdev2->flags)) {
+ if (role == 0xfffe) {
+ pr_info("md: Removing Candidate device %s because add failed\n", bdevname(rdev2->bdev,b));
+ md_kick_rdev_from_array(rdev2);
+ continue;
+ }
+ else {
+ clear_bit(Candidate, &rdev2->flags);
+ rdev2->raid_disk = -1;
+ }
+ }
+
if (role != rdev2->raid_disk) {
/* got activated */
if (rdev2->raid_disk == -1 && role != 0xffff) {
--
1.8.5.6
^ permalink raw reply related [flat|nested] 6+ messages in thread