Skip to contentSkip to conversation

Introduction

PATCH New NeilBrown 2 participants 16 messages Nov 2, 2005

Subsystems (2)
Documentation MD RAID
lore
Share & tools
lore

NeilBrown

author
#0 lore raw

PATCH 1/1 Introduction

Following are 10 patches for md against what is currently in -mm. '8' probbly applied to 2.6.14, but the rest do not as they are fixes and improvements to stuff that is only in -mm.

The bulk of these patches are refining the new sysfs-based interface to md devices (/sys/block/mdX/md/...) and fixing up some of the functionality provided by that interface. The interface is a long way from complete, but at least with these patches it should be correct.

Notable patches include '4' which fixes some issues with module ref-counting that I had missed, and protect against the possibility of a sysfs file being openned with an array is active, and read after it has been stopped. Also '10' is notable as it contains some documentation for the interface.

At Andrew's request I'm cc:ing some of these to Greg KH - those that specifically involve sysfs stuff (and are marked with an '*').

With these in place, I think the md stuff in -mm is ready to go to Linus.

Thanks,
NeilBrown

*[PATCH md 001 of 10] Make sure /block link in /sys/.../md/ goes to correct devices.
[PATCH md 002 of 10] Make manual repair work for raid1.
[PATCH md 003 of 10] Make sure a user-request sync of raid5 ignores intent bitmap
*[PATCH md 004 of 10] Fix some locking and module refcounting issues with md's use of sysfs. *[PATCH md 005 of 10] Split off some md attributes in sysfs to a separate group.
[PATCH md 006 of 10] Only try to print recovery/resync status for personalities that support recovery [PATCH md 007 of 10] Ignore auto-readonly flag for arrays where it isn't meaningful. [PATCH md 008 of 10] Complete conversion of md to use kthreads.
*[PATCH md 009 of 10] Improve 'scan_mode' and rename it to 'sync_action'
*[PATCH md 010 of 10] Document sysfs usage of md, and make a couple of small refinements.

author
#1 lore raw

PATCH 1/1 Make sure /block link in /sys/.../md/ goes to correct devices.

If a block_device is a partition, then it's kobject is
bdev->bd_part->kobj
otherwise (if it is a full device), the kobject is
bdev->bd_disk->kobj

As md wants back-links to the correct object (whether partition or not), we need to respect this difference...
(Thus current code shows a link to the whole device, whether we are using a partition or not, which is wrong).

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/md.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/md/md.c renamed from drivers/md/md.c~current~ +6 -1
diff ./drivers/md/md.c~current~ ./drivers/md/md.c
@@ -1182,6 +1182,7 @@ static int bind_rdev_to_array(mdk_rdev_t
 {
 	mdk_rdev_t *same_pdev;
 	char b[BDEVNAME_SIZE], b2[BDEVNAME_SIZE];
+	struct kobject *ko;
 
 	if (rdev->mddev) {
 		MD_BUG();

@@ -1221,7 +1222,11 @@ static int bind_rdev_to_array(mdk_rdev_t
 	rdev->kobj.parent = &mddev->kobj;
 	kobject_add(&rdev->kobj);
 
-	sysfs_create_link(&rdev->kobj, &rdev->bdev->bd_disk->kobj, "block");
+	if (rdev->bdev->bd_part)
+		ko = &rdev->bdev->bd_part->kobj;
+	else
+		ko = &rdev->bdev->bd_disk->kobj;
+	sysfs_create_link(&rdev->kobj, ko, "block");
 	return 0;
 }
Acked-by Greg Kroah-Hartman on v1 1/1
#2 lore raw

Re: [PATCH md 001 of 10] Make sure /block link in /sys/.../md/ goes to correct devices.

Quoted 6 lines · NeilBrown

If a block_device is a partition, then it's kobject is
bdev->bd_part->kobj
otherwise (if it is a full device), the kobject is
bdev->bd_disk->kobj

Quoted 6 lines

As md wants back-links to the correct object (whether partition or not), we need to respect this difference...
(Thus current code shows a link to the whole device, whether we are using a partition or not, which is wrong).

Signed-off-by Neil Brown <[email protected]>
Acked-by Greg Kroah-Hartman <[email protected]>
author
#3 lore raw

PATCH 1/1 Make manual repair work for raid1.

In reply to NeilBrown

Raid1 currently optimises resync using the intent bitmap etc. This optimisation is not wanted when we explicitly request a repair through sysfs, so add appropriate checks.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/raid1.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/md/raid1.c renamed from drivers/md/raid1.c~current~ +6 -4
diff ./drivers/md/raid1.c~current~ ./drivers/md/raid1.c
@@ -1314,7 +1314,7 @@ static sector_t sync_request(mddev_t *md
 	 * This call the bitmap_start_sync doesn't actually record anything
 	 */
 	if (!bitmap_start_sync(mddev->bitmap, sector_nr, &sync_blocks, 1) &&
-	    !conf->fullsync) {
+	    !conf->fullsync && !test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery)) {
 		/* We can skip this block, and probably several more */
 		*skipped = 1;
 		return sync_blocks;

@@ -1391,7 +1391,8 @@ static sector_t sync_request(mddev_t *md
 			still_degraded = 1;
 			continue;
 		} else if (!test_bit(In_sync, &conf->mirrors[i].rdev->flags) ||
-			   sector_nr + RESYNC_SECTORS > mddev->recovery_cp) {
+			   sector_nr + RESYNC_SECTORS > mddev->recovery_cp   ||
+			   test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery)) {
 			bio->bi_rw = WRITE;
 			bio->bi_end_io = end_sync_write;
 			write_targets ++;

@@ -1425,8 +1426,9 @@ static sector_t sync_request(mddev_t *md
 			break;
 		if (sync_blocks == 0) {
 			if (!bitmap_start_sync(mddev->bitmap, sector_nr,
-					&sync_blocks, still_degraded) &&
-					!conf->fullsync)
+					       &sync_blocks, still_degraded) &&
+			    !conf->fullsync &&
+			    !test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery))
 				break;
 			if (sync_blocks < (PAGE_SIZE>>9))
 				BUG();
author
#4 lore raw

PATCH 1/1 Make sure a user-request sync of raid5 ignores intent bitmap

In reply to NeilBrown

A sync of raid5 usually ignore blocks which the bitmap says are in-sync. But a user-request check or repair should not ignore these.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/raid5.c |    1 +
 1 file changed, 1 insertion(+)
drivers/md/raid5.c renamed from drivers/md/raid5.c~current~ +1 -0
diff ./drivers/md/raid5.c~current~ ./drivers/md/raid5.c
@@ -1649,6 +1649,7 @@ static sector_t sync_request(mddev_t *md
 		return rv;
 	}
 	if (!bitmap_start_sync(mddev->bitmap, sector_nr, &sync_blocks, 1) &&
+	    !test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery) &&
 	    !conf->fullsync && sync_blocks >= STRIPE_SECTORS) {
 		/* we can skip this block, and probably more */
 		sync_blocks /= STRIPE_SECTORS;
author
#5 lore raw

PATCH 1/1 Fix some locking and module refcounting issues with md's use of sysfs.

In reply to NeilBrown

1/ I really should be using the __ATTR macros for defining attributes, so
that the .owner field get set properly, otherwise modules can be removed while sysfs files are open.
This also involves some name changes of _show routines.

2/ Always lock the mddev (against reconfiguration) for all sysfs
attribute access. This easily avoid certain races and is completely consistant with other interfaces (ioctl and /proc/mdstat both always lock against reconfiguration).

3/ raid5 attributes must check that the 'conf' structure actually
exists (the array could have been stopped while an attribute file was open).

4/ A missing 'kfree' from when the raid5_conf_t was converted to have
a kobject embedded, and then converted back again.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/md.c    |   62 +++++++++++++++++++++++----------------------------
 ./drivers/md/raid5.c |   30 ++++++++++++++----------
 2 files changed, 46 insertions(+), 46 deletions(-)
drivers/md/md.c renamed from drivers/md/md.c~current~ +28 -34
diff ./drivers/md/md.c~current~ ./drivers/md/md.c
@@ -1504,7 +1504,7 @@ struct rdev_sysfs_entry {
 };
 
 static ssize_t
-rdev_show_state(mdk_rdev_t *rdev, char *page)
+state_show(mdk_rdev_t *rdev, char *page)
 {
 	char *sep = "";
 	int len=0;

@@ -1525,13 +1525,11 @@ rdev_show_state(mdk_rdev_t *rdev, char *
 	return len+sprintf(page+len, "\n");
 }
 
-static struct rdev_sysfs_entry rdev_state = {
-	.attr = {.name = "state", .mode = S_IRUGO },
-	.show = rdev_show_state,
-};
+static struct rdev_sysfs_entry
+rdev_state = __ATTR_RO(state);
 
 static ssize_t
-rdev_show_super(mdk_rdev_t *rdev, char *page)
+super_show(mdk_rdev_t *rdev, char *page)
 {
 	if (rdev->sb_loaded && rdev->sb_size) {
 		memcpy(page, page_address(rdev->sb_page), rdev->sb_size);

@@ -1539,10 +1537,8 @@ rdev_show_super(mdk_rdev_t *rdev, char *
 	} else
 		return 0;
 }
-static struct rdev_sysfs_entry rdev_super = {
-	.attr = {.name = "super", .mode = S_IRUGO },
-	.show = rdev_show_super,
-};
+static struct rdev_sysfs_entry rdev_super = __ATTR_RO(super);
+
 static struct attribute *rdev_default_attrs[] = {
 	&rdev_state.attr,
 	&rdev_super.attr,

@@ -1728,7 +1724,7 @@ static void analyze_sbs(mddev_t * mddev)
 }
 
 static ssize_t
-md_show_level(mddev_t *mddev, char *page)
+level_show(mddev_t *mddev, char *page)
 {
 	mdk_personality_t *p = mddev->pers;
 	if (p == NULL)

@@ -1739,21 +1735,15 @@ md_show_level(mddev_t *mddev, char *page
 		return sprintf(page, "%s\n", p->name);
 }
 
-static struct md_sysfs_entry md_level = {
-	.attr = {.name = "level", .mode = S_IRUGO },
-	.show = md_show_level,
-};
+static struct md_sysfs_entry md_level = __ATTR_RO(level);
 
 static ssize_t
-md_show_rdisks(mddev_t *mddev, char *page)
+raid_disks_show(mddev_t *mddev, char *page)
 {
 	return sprintf(page, "%d\n", mddev->raid_disks);
 }
 
-static struct md_sysfs_entry md_raid_disks = {
-	.attr = {.name = "raid_disks", .mode = S_IRUGO },
-	.show = md_show_rdisks,
-};
+static struct md_sysfs_entry md_raid_disks = __ATTR_RO(raid_disks);
 
 static ssize_t
 md_show_scan(mddev_t *mddev, char *page)

@@ -1782,10 +1772,10 @@ md_store_scan(mddev_t *mddev, const char
 	if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) ||
 	    test_bit(MD_RECOVERY_NEEDED, &mddev->recovery))
 		return -EBUSY;
-	down(&mddev->reconfig_sem);
+
 	if (mddev->pers && mddev->pers->sync_request)
 		canscan=1;
-	up(&mddev->reconfig_sem);
+
 	if (!canscan)
 		return -EINVAL;
 

@@ -1801,22 +1791,18 @@ md_store_scan(mddev_t *mddev, const char
 }
 
 static ssize_t
-md_show_mismatch(mddev_t *mddev, char *page)
+mismatch_cnt_show(mddev_t *mddev, char *page)
 {
 	return sprintf(page, "%llu\n",
 		       (unsigned long long) mddev->resync_mismatches);
 }
 
-static struct md_sysfs_entry md_scan_mode = {
-	.attr = {.name = "scan_mode", .mode = S_IRUGO|S_IWUSR },
-	.show = md_show_scan,
-	.store = md_store_scan,
-};
+static struct md_sysfs_entry
+md_scan_mode = __ATTR(scan_mode, S_IRUGO|S_IWUSR, md_show_scan, md_store_scan);
 
-static struct md_sysfs_entry md_mismatches = {
-	.attr = {.name = "mismatch_cnt", .mode = S_IRUGO },
-	.show = md_show_mismatch,
-};
+
+static struct md_sysfs_entry
+md_mismatches = __ATTR_RO(mismatch_cnt);
 
 static struct attribute *md_default_attrs[] = {
 	&md_level.attr,

@@ -1831,10 +1817,14 @@ md_attr_show(struct kobject *kobj, struc
 {
 	struct md_sysfs_entry *entry = container_of(attr, struct md_sysfs_entry, attr);
 	mddev_t *mddev = container_of(kobj, struct mddev_s, kobj);
+	ssize_t rv;
 
 	if (!entry->show)
 		return -EIO;
-	return entry->show(mddev, page);
+	mddev_lock(mddev);
+	rv = entry->show(mddev, page);
+	mddev_unlock(mddev);
+	return rv;
 }
 
 static ssize_t

@@ -1843,10 +1833,14 @@ md_attr_store(struct kobject *kobj, stru
 {
 	struct md_sysfs_entry *entry = container_of(attr, struct md_sysfs_entry, attr);
 	mddev_t *mddev = container_of(kobj, struct mddev_s, kobj);
+	ssize_t rv;
 
 	if (!entry->store)
 		return -EIO;
-	return entry->store(mddev, page, length);
+	mddev_lock(mddev);
+	rv = entry->store(mddev, page, length);
+	mddev_unlock(mddev);
+	return rv;
 }
 
 static void md_free(struct kobject *ko)
drivers/md/raid5.c renamed from drivers/md/raid5.c~current~ +18 -12
diff ./drivers/md/raid5.c~current~ ./drivers/md/raid5.c
@@ -1750,7 +1750,10 @@ static ssize_t
 raid5_show_stripe_cache_size(mddev_t *mddev, char *page)
 {
 	raid5_conf_t *conf = mddev_to_conf(mddev);
-	return sprintf(page, "%d\n", conf->max_nr_stripes);
+	if (conf)
+		return sprintf(page, "%d\n", conf->max_nr_stripes);
+	else
+		return 0;
 }
 
 static ssize_t

@@ -1761,6 +1764,8 @@ raid5_store_stripe_cache_size(mddev_t *m
 	int new;
 	if (len >= PAGE_SIZE)
 		return -EINVAL;
+	if (!conf)
+		return -ENODEV;
 
 	new = simple_strtoul(page, &end, 10);
 	if (!*page || (*end && *end != '\n') )

@@ -1781,23 +1786,23 @@ raid5_store_stripe_cache_size(mddev_t *m
 	return len;
 }
 
-static struct md_sysfs_entry raid5_stripecache_size = {
-	.attr = {.name = "stripe_cache_size", .mode = S_IRUGO | S_IWUSR },
-	.show = raid5_show_stripe_cache_size,
-	.store = raid5_store_stripe_cache_size,
-};
+static struct md_sysfs_entry
+raid5_stripecache_size = __ATTR(stripe_cache_size, S_IRUGO | S_IWUSR,
+				raid5_show_stripe_cache_size,
+				raid5_store_stripe_cache_size);
 
 static ssize_t
-raid5_show_stripe_cache_active(mddev_t *mddev, char *page)
+stripe_cache_active_show(mddev_t *mddev, char *page)
 {
 	raid5_conf_t *conf = mddev_to_conf(mddev);
-	return sprintf(page, "%d\n", atomic_read(&conf->active_stripes));
+	if (conf)
+		return sprintf(page, "%d\n", atomic_read(&conf->active_stripes));
+	else
+		return 0;
 }
 
-static struct md_sysfs_entry raid5_stripecache_active = {
-	.attr = {.name = "stripe_cache_active", .mode = S_IRUGO},
-	.show = raid5_show_stripe_cache_active,
-};
+static struct md_sysfs_entry
+raid5_stripecache_active = __ATTR_RO(stripe_cache_active);
 
 static struct attribute *raid5_attrs[] =  {
 	&raid5_stripecache_size.attr,

@@ -1985,6 +1990,7 @@ static int stop(mddev_t *mddev)
 	free_pages((unsigned long) conf->stripe_hashtbl, HASH_PAGES_ORDER);
 	blk_sync_queue(mddev->queue); /* the unplug fn references 'conf'*/
 	sysfs_remove_group(&mddev->kobj, &raid5_attrs_group);
+	kfree(conf);
 	mddev->private = NULL;
 	return 0;
 }
Acked-by Greg Kroah-Hartman on v1 1/1
#6 lore raw

Re: [PATCH md 004 of 10] Fix some locking and module refcounting issues with md's use of sysfs.

Quoted 15 lines · NeilBrown

1/ I really should be using the __ATTR macros for defining attributes, so
that the .owner field get set properly, otherwise modules can be removed while sysfs files are open.
This also involves some name changes of _show routines.

2/ Always lock the mddev (against reconfiguration) for all sysfs
attribute access. This easily avoid certain races and is completely consistant with other interfaces (ioctl and /proc/mdstat both always lock against reconfiguration).

3/ raid5 attributes must check that the 'conf' structure actually
exists (the array could have been stopped while an attribute file was open).

Quoted 6 lines

4/ A missing 'kfree' from when the raid5_conf_t was converted to have
a kobject embedded, and then converted back again.

Signed-off-by Neil Brown <[email protected]>
Acked-by Greg Kroah-Hartman <[email protected]>
author
#7 lore raw

PATCH 1/1 Split off some md attributes in sysfs to a separate group.

In reply to NeilBrown

Some, but not all, md array support data redundancy and hence support checking and restoring that redundancy (resync, rebuild).

Some attributes apply specifically to functions involving this redundancy, and so should only appear for md arrays for which they are meaningful. i.e. they should not appear for raid0, linear, multpath, faulty.

This patch separates these into a distinct group and creates the group only if the personality supports sync_request.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/md.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)
drivers/md/md.c renamed from drivers/md/md.c~current~ +14 -0
diff ./drivers/md/md.c~current~ ./drivers/md/md.c
@@ -1807,10 +1807,19 @@ md_mismatches = __ATTR_RO(mismatch_cnt);
 static struct attribute *md_default_attrs[] = {
 	&md_level.attr,
 	&md_raid_disks.attr,
+	NULL,
+};
+
+static struct attribute *md_redundancy_attrs[] = {
 	&md_scan_mode.attr,
 	&md_mismatches.attr,
 	NULL,
 };
+static struct attribute_group md_redundancy_group = {
+	.name = NULL,
+	.attrs = md_redundancy_attrs,
+};
+
 
 static ssize_t
 md_attr_show(struct kobject *kobj, struct attribute *attr, char *page)

@@ -2047,6 +2056,8 @@ static int do_md_run(mddev_t * mddev)
 		bitmap_destroy(mddev);
 		return err;
 	}
+	if (mddev->pers->sync_request)
+		sysfs_create_group(&mddev->kobj, &md_redundancy_group);
  	atomic_set(&mddev->writes_pending,0);
 	mddev->safemode = 0;
 	mddev->safemode_timer.function = md_safemode_timeout;

@@ -2155,6 +2166,9 @@ static int do_md_stop(mddev_t * mddev, i
 				set_disk_ro(disk, 0);
 			blk_queue_make_request(mddev->queue, md_fail_request);
 			mddev->pers->stop(mddev);
+			if (mddev->pers->sync_request)
+				sysfs_remove_group(&mddev->kobj, &md_redundancy_group);
+
 			module_put(mddev->pers->owner);
 			mddev->pers = NULL;
 			if (mddev->ro)
Acked-by Greg Kroah-Hartman on v1 1/1
#8 lore raw

Re: [PATCH md 005 of 10] Split off some md attributes in sysfs to a separate group.

Quoted 9 lines · NeilBrown

Some, but not all, md array support data redundancy and hence support checking and restoring that redundancy (resync, rebuild).

Some attributes apply specifically to functions involving this redundancy, and so should only appear for md arrays for which they are meaningful. i.e. they should not appear for raid0, linear, multpath, faulty.

Quoted 4 lines

This patch separates these into a distinct group and creates the group only if the personality supports sync_request.

Signed-off-by Neil Brown <[email protected]>
Acked-by Greg Kroah-Hartman <[email protected]>
author
#9 lore raw

PATCH 1/1 Only try to print recovery/resync status for personalities that support recovery

In reply to NeilBrown

The introduction of 'resync=PENDING' (for read-only devices) caused that message to appear for non-syncable arrays like raid0 and linear. Simplest thing is to not try to print any resync info unless the personality clearly supports it.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/md.c |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)
drivers/md/md.c renamed from drivers/md/md.c~current~ +9 -7
diff ./drivers/md/md.c~current~ ./drivers/md/md.c
@@ -3722,13 +3722,15 @@ static int md_seq_show(struct seq_file *
 		if (mddev->pers) {
 			mddev->pers->status (seq, mddev);
 	 		seq_printf(seq, "\n      ");
-			if (mddev->curr_resync > 2) {
-				status_resync (seq, mddev);
-				seq_printf(seq, "\n      ");
-			} else if (mddev->curr_resync == 1 || mddev->curr_resync == 2)
-				seq_printf(seq, "\tresync=DELAYED\n      ");
-			else if (mddev->recovery_cp < MaxSector)
-				seq_printf(seq, "\tresync=PENDING\n      ");
+			if (mddev->pers->sync_request) {
+				if (mddev->curr_resync > 2) {
+					status_resync (seq, mddev);
+					seq_printf(seq, "\n      ");
+				} else if (mddev->curr_resync == 1 || mddev->curr_resync == 2)
+					seq_printf(seq, "\tresync=DELAYED\n      ");
+				else if (mddev->recovery_cp < MaxSector)
+					seq_printf(seq, "\tresync=PENDING\n      ");
+			}
 		} else
 			seq_printf(seq, "\n       ");
author
#10 lore raw

PATCH 1/1 Ignore auto-readonly flag for arrays where it isn't meaningful.

In reply to NeilBrown

The 'auto-readonly' flag (which suppresses resync and superblock updates until the first write) is not meaningful for personalities that don't support resync or superblock writes (raid0, linear, etc).

So clear the setting early to avoid it confusing anything - e.g. appearing in /proc/mdstat

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/md.c |    3 +++
 1 file changed, 3 insertions(+)
drivers/md/md.c renamed from drivers/md/md.c~current~ +3 -0
diff ./drivers/md/md.c~current~ ./drivers/md/md.c
@@ -2058,6 +2058,9 @@ static int do_md_run(mddev_t * mddev)
 	}
 	if (mddev->pers->sync_request)
 		sysfs_create_group(&mddev->kobj, &md_redundancy_group);
+	else if (mddev->ro == 2) /* auto-readonly not meaningful */
+		mddev->ro = 0;
+
  	atomic_set(&mddev->writes_pending,0);
 	mddev->safemode = 0;
 	mddev->safemode_timer.function = md_safemode_timeout;
author
#11 lore raw

PATCH 1/1 Complete conversion of md to use kthreads.

In reply to NeilBrown

There are a few loose ends following the conversion of md to use kthreads:
- Some fields in mdk_thread_t that aren't needed (kthreads does it's own completion and manages it's own name).
- thread->run is now never NULL, so no need to check
- Some tests for signal_pending that aren't needed (As we don't use signals to stop threads any more)
- Some flush_signals are not needed
- Some waits are interruptible and don't need to be.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/md.c           |   33 ++++++++++-----------------------
 ./include/linux/raid/md_k.h |    2 --
 2 files changed, 10 insertions(+), 25 deletions(-)
drivers/md/md.c renamed from drivers/md/md.c~current~ +10 -23
diff ./drivers/md/md.c~current~ ./drivers/md/md.c
@@ -3424,21 +3424,17 @@ static int md_thread(void * arg)
 	 */
 
 	allow_signal(SIGKILL);
-	complete(thread->event);
 	while (!kthread_should_stop()) {
-		void (*run)(mddev_t *);
 
-		wait_event_interruptible_timeout(thread->wqueue,
-						 test_bit(THREAD_WAKEUP, &thread->flags)
-						 || kthread_should_stop(),
-						 thread->timeout);
+		wait_event_timeout(thread->wqueue,
+				   test_bit(THREAD_WAKEUP, &thread->flags)
+				   || kthread_should_stop(),
+				   thread->timeout);
 		try_to_freeze();
 
 		clear_bit(THREAD_WAKEUP, &thread->flags);
 
-		run = thread->run;
-		if (run)
-			run(thread->mddev);
+		thread->run(thread->mddev);
 	}
 
 	return 0;

@@ -3457,7 +3453,6 @@ mdk_thread_t *md_register_thread(void (*
 				 const char *name)
 {
 	mdk_thread_t *thread;
-	struct completion event;
 
 	thread = kmalloc(sizeof(mdk_thread_t), GFP_KERNEL);
 	if (!thread)

@@ -3466,18 +3461,14 @@ mdk_thread_t *md_register_thread(void (*
 	memset(thread, 0, sizeof(mdk_thread_t));
 	init_waitqueue_head(&thread->wqueue);
 
-	init_completion(&event);
-	thread->event = &event;
 	thread->run = run;
 	thread->mddev = mddev;
-	thread->name = name;
 	thread->timeout = MAX_SCHEDULE_TIMEOUT;
 	thread->tsk = kthread_run(md_thread, thread, name, mdname(thread->mddev));
 	if (IS_ERR(thread->tsk)) {
 		kfree(thread);
 		return NULL;
 	}
-	wait_for_completion(&event);
 	return thread;
 }
 

@@ -3941,9 +3932,7 @@ static void md_do_sync(mddev_t *mddev)
 		mddev->curr_resync = 2;
 
 	try_again:
-		if (signal_pending(current) ||
-		    kthread_should_stop()) {
-			flush_signals(current);
+		if (kthread_should_stop()) {
 			set_bit(MD_RECOVERY_INTR, &mddev->recovery);
 			goto skip;
 		}

@@ -3963,9 +3952,8 @@ static void md_do_sync(mddev_t *mddev)
 					 * time 'round when curr_resync == 2
 					 */
 					continue;
-				prepare_to_wait(&resync_wait, &wq, TASK_INTERRUPTIBLE);
-				if (!signal_pending(current) &&
-				    !kthread_should_stop() &&
+				prepare_to_wait(&resync_wait, &wq, TASK_UNINTERRUPTIBLE);
+				if (!kthread_should_stop() &&
 				    mddev2->curr_resync >= mddev->curr_resync) {
 					printk(KERN_INFO "md: delaying resync of %s"
 					       " until %s has finished resync (they"

@@ -4074,13 +4062,12 @@ static void md_do_sync(mddev_t *mddev)
 		}
 
 
-		if (signal_pending(current) || kthread_should_stop()) {
+		if (kthread_should_stop()) {
 			/*
 			 * got a signal, exit.
 			 */
 			printk(KERN_INFO 
 				"md: md_do_sync() got signal ... exiting\n");
-			flush_signals(current);
 			set_bit(MD_RECOVERY_INTR, &mddev->recovery);
 			goto out;
 		}

@@ -4102,7 +4089,7 @@ static void md_do_sync(mddev_t *mddev)
 		if (currspeed > sysctl_speed_limit_min) {
 			if ((currspeed > sysctl_speed_limit_max) ||
 					!is_mddev_idle(mddev)) {
-				msleep_interruptible(250);
+				msleep(250);
 				goto repeat;
 			}
 		}
include/linux/raid/md_k.h renamed from include/linux/raid/md_k.h~current~ +0 -2
diff ./include/linux/raid/md_k.h~current~ ./include/linux/raid/md_k.h
@@ -334,10 +334,8 @@ typedef struct mdk_thread_s {
 	mddev_t			*mddev;
 	wait_queue_head_t	wqueue;
 	unsigned long           flags;
-	struct completion	*event;
 	struct task_struct	*tsk;
 	unsigned long		timeout;
-	const char		*name;
 } mdk_thread_t;
 
 #define THREAD_WAKEUP  0
author
#12 lore raw

PATCH 1/1 Improve 'scan_mode' and rename it to 'sync_action'

In reply to NeilBrown

The current sync_action for an array can be one of

idle - nothing happening
resync - reduncancy being recalcualted recover - missing device being recoverred to spare
check - user initiated check of redundancy
repair - like resync but user-initiated and ignores
bitmap optimisation.

Each of these strings can also be written to the 'sync_action' file to cause that action to happen (if appropriate).

While 'sync' is not technically correct, as a recovery is *not* a 'sync', I think it is the most servicable word here. Also 'action' is a strong word than 'mode'.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/md.c |   48 +++++++++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 19 deletions(-)
drivers/md/md.c renamed from drivers/md/md.c~current~ +29 -19
diff ./drivers/md/md.c~current~ ./drivers/md/md.c
@@ -1746,9 +1746,9 @@ raid_disks_show(mddev_t *mddev, char *pa
 static struct md_sysfs_entry md_raid_disks = __ATTR_RO(raid_disks);
 
 static ssize_t
-md_show_scan(mddev_t *mddev, char *page)
+action_show(mddev_t *mddev, char *page)
 {
-	char *type = "none";
+	char *type = "idle";
 	if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) ||
 	    test_bit(MD_RECOVERY_NEEDED, &mddev->recovery)) {
 		if (test_bit(MD_RECOVERY_SYNC, &mddev->recovery)) {

@@ -1765,27 +1765,37 @@ md_show_scan(mddev_t *mddev, char *page)
 }
 
 static ssize_t
-md_store_scan(mddev_t *mddev, const char *page, size_t len)
+action_store(mddev_t *mddev, const char *page, size_t len)
 {
-	int canscan=0;
+	if (!mddev->pers || !mddev->pers->sync_request)
+		return -EINVAL;
+
+	if (strcmp(page, "idle")==0 || strcmp(page, "idle\n")==0) {
+		if (mddev->sync_thread) {
+			set_bit(MD_RECOVERY_INTR, &mddev->recovery);
+			md_unregister_thread(mddev->sync_thread);
+			mddev->sync_thread = NULL;
+			mddev->recovery = 0;
+			printk("stop at %llu\n", (unsigned long long)mddev->recovery_cp);
+		}
+		return len;
+	}
 
 	if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) ||
 	    test_bit(MD_RECOVERY_NEEDED, &mddev->recovery))
 		return -EBUSY;
-
-	if (mddev->pers && mddev->pers->sync_request)
-		canscan=1;
-
-	if (!canscan)
-		return -EINVAL;
-
-	if (strcmp(page, "check")==0 || strcmp(page, "check\n")==0)
-		set_bit(MD_RECOVERY_CHECK, &mddev->recovery);
-	else if (strcmp(page, "repair")!=0 && strcmp(page, "repair\n")!=0)
-		return -EINVAL;
-	set_bit(MD_RECOVERY_REQUESTED, &mddev->recovery);
-	set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
-	set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
+	if (strcmp(page, "resync")==0 || strcmp(page, "resync\n")==0 ||
+	    strcmp(page, "recover")==0 || strcmp(page, "recover\n")==0)
+		set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
+	else {
+		if (strcmp(page, "check")==0 || strcmp(page, "check\n")==0)
+			set_bit(MD_RECOVERY_CHECK, &mddev->recovery);
+		else if (strcmp(page, "repair")!=0 && strcmp(page, "repair\n")!=0)
+			return -EINVAL;
+		set_bit(MD_RECOVERY_REQUESTED, &mddev->recovery);
+		set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
+		set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
+	}
 	md_wakeup_thread(mddev->thread);
 	return len;
 }

@@ -1798,7 +1808,7 @@ mismatch_cnt_show(mddev_t *mddev, char *
 }
 
 static struct md_sysfs_entry
-md_scan_mode = __ATTR(scan_mode, S_IRUGO|S_IWUSR, md_show_scan, md_store_scan);
+md_scan_mode = __ATTR(sync_action, S_IRUGO|S_IWUSR, action_show, action_store);
 
 
 static struct md_sysfs_entry
#13 lore raw

Re: [PATCH md 009 of 10] Improve 'scan_mode' and rename it to 'sync_action'

Quoted 47 lines · NeilBrown

The current sync_action for an array can be one of

idle - nothing happening
resync - reduncancy being recalcualted recover - missing device being recoverred to spare
check - user initiated check of redundancy
repair - like resync but user-initiated and ignores
bitmap optimisation.

Each of these strings can also be written to the 'sync_action' file to cause that action to happen (if appropriate).

While 'sync' is not technically correct, as a recovery is *not* a 'sync', I think it is the most servicable word here. Also 'action' is a strong word than 'mode'.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./drivers/md/md.c |   48 +++++++++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 19 deletions(-)

diff ./drivers/md/md.c~current~ ./drivers/md/md.c

drivers/md/md.c renamed from drivers/md/md.c~current~ +3 -4
@@ -1746,9 +1746,9 @@ raid_disks_show(mddev_t *mddev, char *pa
 static struct md_sysfs_entry md_raid_disks = __ATTR_RO(raid_disks);
 
 static ssize_t
-md_show_scan(mddev_t *mddev, char *page)
+action_show(mddev_t *mddev, char *page)
 {
-	char *type = "none";
+	char *type = "idle";
 	if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) ||
 	    test_bit(MD_RECOVERY_NEEDED, &mddev->recovery)) {
 		if (test_bit(MD_RECOVERY_SYNC, &mddev->recovery)) {

@@ -1765,27 +1765,37 @@ md_show_scan(mddev_t *mddev, char *page)
 }
 
 static ssize_t
-md_store_scan(mddev_t *mddev, const char *page, size_t len)
+action_store(mddev_t *mddev, const char *page, size_t len)
 {
-	int canscan=0;

drivers/md/md.c md_show_scan(mddev_t *mddev, char *page) · line 1779

Quoted 10 lines
drivers/md/md.c renamed from drivers/md/md.c~current~ +10 -0
+	if (!mddev->pers || !mddev->pers->sync_request)
+		return -EINVAL;
+
+	if (strcmp(page, "idle")==0 || strcmp(page, "idle\n")==0) {
+		if (mddev->sync_thread) {
+			set_bit(MD_RECOVERY_INTR, &mddev->recovery);
+			md_unregister_thread(mddev->sync_thread);
+			mddev->sync_thread = NULL;
+			mddev->recovery = 0;
+			printk("stop at %llu\n", (unsigned long long)mddev->recovery_cp);

Your printk() needs a KERN_ level. Or is this debugging code left over?

thanks,

greg k-h

author
#14 lore raw

PATCH 1/1 Document sysfs usage of md, and make a couple of small refinements.

In reply to NeilBrown

Document in Documentation/md.txt the files that now appear in sysfs, and make a couple of small refinements to exactly when 'level' and 'raid_disks' are empty, to make it match the documentation.

Signed-off-by Neil Brown <[email protected]>

### Diffstat output

 ./Documentation/md.txt |  119 +++++++++++++++++++++++++++++++++++++++++++++++++
 ./drivers/md/md.c      |    4 +
 2 files changed, 122 insertions(+), 1 deletion(-)
Documentation/md.txt renamed from Documentation/md.txt~current~ +119 -0
diff ./Documentation/md.txt~current~ ./Documentation/md.txt
@@ -116,3 +116,122 @@ and it's role in the array.
 
 Once started with RUN_ARRAY, uninitialized spares can be added with
 HOT_ADD_DISK.
+
+
+
+MD devices in sysfs
+-------------------
+md devices appear in sysfs (/sys) as regular block devices,
+e.g.
+   /sys/block/md0
+
+Each 'md' device will contain a subdirectory called 'md' which
+contains further md-specific information about the device.
+
+All md devices contain:
+  level
+     a text file indicating the 'raid level'.  This may be a standard
+     numerical level prefixed by "RAID-" - e.g. "RAID-5", or some
+     other name such as "linear" or "multipath".
+     If no raid level has been set yet (array is still being
+     assembled), this file will be empty.
+
+  raid_disks
+     a text file with a simple number indicating the number of devices
+     in a fully functional array.  If this is not yet known, the file
+     will be empty.  If an array is being resized (not currently
+     possible) this will contain the larger of the old and new sizes.
+
+As component devices are added to an md array, they appear in the 'md'
+directory as new directories named
+      dev-XXX
+where XXX is a name that the kernel knows for the device, e.g. hdb1.
+Each directory contains:
+
+      block
+        a symlink to the block device in /sys/block, e.g.
+	     /sys/block/md0/md/dev-hdb1/block -> ../../../../block/hdb/hdb1
+
+      super
+        A file containing an image of the superblock read from, or
+        written to, that device.
+
+      state
+        A file recording the current state of the device in the array
+	which can be a comma separated list of
+	      faulty   - device has been kicked from active use due to
+                         a detected fault
+	      in_sync  - device is a fully in-sync member of the array
+	      spare    - device is working, but not a full member.
+			 This includes spares that are in the process
+			 of being recoverred to
+	This list make grow in future.
+
+
+An active md device will also contain and entry for each active device
+in the array.  These are named
+
+    rdNN
+
+where 'NN' is the possition in the array, starting from 0.
+So for a 3 drive array there will be rd0, rd1, rd2.
+These are symbolic links to the appropriate 'dev-XXX' entry.
+Thus, for example,
+       cat /sys/block/md*/md/rd*/state
+will show 'in_sync' on every line.
+
+
+
+Active md devices for levels that support data redundancy (1,4,5,6)
+also have
+
+   sync_action
+     a text file that can be used to monitor and control the rebuild
+     process.  It contains one word which can be one of:
+       resync        - redundancy is being recalculated after unclean
+                       shutdown or creation
+       recover       - a hot spare is being built to replace a
+                       failed/missing device
+       idle          - nothing is happening
+       check         - A full check of redundancy was requested and is
+                       happening.  This reads all block and checks
+                       them. A repair may also happen for some raid
+                       levels.
+       repair        - A full check and repair is happening.  This is
+                       similar to 'resync', but was requested by the
+                       user, and the write-intent bitmap is NOT used to
+		       optimise the process.
+
+      This file is writable, and each of the strings that could be
+      read are meaningful for writing.
+
+       'idle' will stop an active resync/recovery etc.  There is no
+           guarantee that another resync/recovery may not be automatically
+	   started again, though some event will be needed to trigger
+           this.
+	'resync' or 'recovery' can be used to restart the
+           corresponding operation if it was stopped with 'idle'.
+	'check' and 'repair' will start the appropriate process
+           providing the current state is 'idle'.
+
+   mismatch_count
+      When performing 'check' and 'repair', and possibly when
+      performing 'resync', md will count the number of errors that are
+      found.  The count in 'mismatch_cnt' is the number of sectors
+      that were re-written, or (for 'check') would have been
+      re-written.  As most raid levels work in units of pages rather
+      than sectors, this my be larger than the number of actual errors
+      by a factor of the number of sectors in a page.
+
+Each active md device may also have attributes specific to the
+personality module that manages it.
+These are specific to the implementation of the module and could
+change substantially if the implementation changes.
+
+These currently include
+
+  stripe_cache_size  (currently raid5 only)
+      number of entries in the stripe cache.  This is writable, but
+      there are upper and lower limits (32768, 16).  Default is 128.
+  strip_cache_active (currently raid5 only)
+      number of active entries in the stripe cache
drivers/md/md.c renamed from drivers/md/md.c~current~ +3 -1
diff ./drivers/md/md.c~current~ ./drivers/md/md.c
@@ -1727,7 +1727,7 @@ static ssize_t
 level_show(mddev_t *mddev, char *page)
 {
 	mdk_personality_t *p = mddev->pers;
-	if (p == NULL)
+	if (p == NULL && mddev->raid_disks == 0)
 		return 0;
 	if (mddev->level >= 0)
 		return sprintf(page, "RAID-%d\n", mddev->level);

@@ -1740,6 +1740,8 @@ static struct md_sysfs_entry md_level = 
 static ssize_t
 raid_disks_show(mddev_t *mddev, char *page)
 {
+	if (mddev->raid_disks == 0)
+		return 0;
 	return sprintf(page, "%d\n", mddev->raid_disks);
 }
Acked-by Greg Kroah-Hartman on v1 1/1
#15 lore raw

Re: [PATCH md 010 of 10] Document sysfs usage of md, and make a couple of small refinements.

Quoted 6 lines · NeilBrown

Document in Documentation/md.txt the files that now appear in sysfs, and make a couple of small refinements to exactly when 'level' and 'raid_disks' are empty, to make it match the documentation.

Signed-off-by Neil Brown <[email protected]>

Nice, gotta love documentation :)

Acked-by Greg Kroah-Hartman <[email protected]>