[PATCH md 000 of 10] Introduction

[PATCH md 000 of 10] Introduction

[NeilBrown]

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.

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

[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

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 <neilb@suse.de>

### Diffstat output
 ./drivers/md/md.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
--- ./drivers/md/md.c~current~	2005-11-02 17:32:28.000000000 +1100
+++ ./drivers/md/md.c	2005-11-02 17:32:41.000000000 +1100
@@ -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;
 }
 

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

[Greg KH]

On Wed, Nov 02, 2005 at 09:14:52PM +1100, NeilBrown wrote:
> 
> 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 <neilb@suse.de>

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

[PATCH md 002 of 10] Make manual repair work for raid1.

[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 <neilb@suse.de>

### Diffstat output
 ./drivers/md/raid1.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff ./drivers/md/raid1.c~current~ ./drivers/md/raid1.c
--- ./drivers/md/raid1.c~current~	2005-11-02 17:32:28.000000000 +1100
+++ ./drivers/md/raid1.c	2005-11-02 17:32:48.000000000 +1100
@@ -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();

[PATCH md 003 of 10] Make sure a user-request sync of raid5 ignores intent bitmap

[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 <neilb@suse.de>

### Diffstat output
 ./drivers/md/raid5.c |    1 +
 1 file changed, 1 insertion(+)

diff ./drivers/md/raid5.c~current~ ./drivers/md/raid5.c
--- ./drivers/md/raid5.c~current~	2005-11-02 17:32:28.000000000 +1100
+++ ./drivers/md/raid5.c	2005-11-02 17:32:57.000000000 +1100
@@ -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;

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

[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 <neilb@suse.de>

### Diffstat output
 ./drivers/md/md.c    |   62 +++++++++++++++++++++++----------------------------
 ./drivers/md/raid5.c |   30 ++++++++++++++----------
 2 files changed, 46 insertions(+), 46 deletions(-)

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
--- ./drivers/md/md.c~current~	2005-11-02 17:32:41.000000000 +1100
+++ ./drivers/md/md.c	2005-11-02 17:33:16.000000000 +1100
@@ -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)

diff ./drivers/md/raid5.c~current~ ./drivers/md/raid5.c
--- ./drivers/md/raid5.c~current~	2005-11-02 17:32:57.000000000 +1100
+++ ./drivers/md/raid5.c	2005-11-02 17:33:17.000000000 +1100
@@ -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;
 }

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

[Greg KH]

On Wed, Nov 02, 2005 at 09:15:10PM +1100, NeilBrown wrote:
> 
> 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 <neilb@suse.de>

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

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

[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 <neilb@suse.de>

### Diffstat output
 ./drivers/md/md.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
--- ./drivers/md/md.c~current~	2005-11-02 17:33:16.000000000 +1100
+++ ./drivers/md/md.c	2005-11-02 17:33:22.000000000 +1100
@@ -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)

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

[Greg KH]

On Wed, Nov 02, 2005 at 09:15:16PM +1100, NeilBrown wrote:
> 
> 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 <neilb@suse.de>

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

[PATCH md 006 of 10] Only try to print recovery/resync status for personalities that support recovery

[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 <neilb@suse.de>

### Diffstat output
 ./drivers/md/md.c |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
--- ./drivers/md/md.c~current~	2005-11-02 17:33:22.000000000 +1100
+++ ./drivers/md/md.c	2005-11-02 17:33:26.000000000 +1100
@@ -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       ");
 

[PATCH md 007 of 10] Ignore auto-readonly flag for arrays where it isn't meaningful.

[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 <neilb@suse.de>

### Diffstat output
 ./drivers/md/md.c |    3 +++
 1 file changed, 3 insertions(+)

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
--- ./drivers/md/md.c~current~	2005-11-02 17:33:26.000000000 +1100
+++ ./drivers/md/md.c	2005-11-02 17:33:29.000000000 +1100
@@ -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;

[PATCH md 008 of 10] Complete conversion of md to use kthreads.

[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 <neilb@suse.de>

### Diffstat output
 ./drivers/md/md.c           |   33 ++++++++++-----------------------
 ./include/linux/raid/md_k.h |    2 --
 2 files changed, 10 insertions(+), 25 deletions(-)

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
--- ./drivers/md/md.c~current~	2005-11-02 17:33:29.000000000 +1100
+++ ./drivers/md/md.c	2005-11-02 17:33:34.000000000 +1100
@@ -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;
 			}
 		}

diff ./include/linux/raid/md_k.h~current~ ./include/linux/raid/md_k.h
--- ./include/linux/raid/md_k.h~current~	2005-11-02 17:32:28.000000000 +1100
+++ ./include/linux/raid/md_k.h	2005-11-02 17:33:34.000000000 +1100
@@ -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

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

[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 <neilb@suse.de>

### 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~current~	2005-11-02 17:33:34.000000000 +1100
+++ ./drivers/md/md.c	2005-11-02 17:33:35.000000000 +1100
@@ -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

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

[Greg KH]

On Wed, Nov 02, 2005 at 09:15:55PM +1100, NeilBrown wrote:
> 
> 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 <neilb@suse.de>
> 
> ### 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~current~	2005-11-02 17:33:34.000000000 +1100
> +++ ./drivers/md/md.c	2005-11-02 17:33:35.000000000 +1100
> @@ -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);

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

thanks,

greg k-h

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

[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 <neilb@suse.de>

### Diffstat output
 ./Documentation/md.txt |  119 +++++++++++++++++++++++++++++++++++++++++++++++++
 ./drivers/md/md.c      |    4 +
 2 files changed, 122 insertions(+), 1 deletion(-)

diff ./Documentation/md.txt~current~ ./Documentation/md.txt
--- ./Documentation/md.txt~current~	2005-11-02 17:32:28.000000000 +1100
+++ ./Documentation/md.txt	2005-11-02 17:33:38.000000000 +1100
@@ -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

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
--- ./drivers/md/md.c~current~	2005-11-02 17:33:35.000000000 +1100
+++ ./drivers/md/md.c	2005-11-02 17:33:38.000000000 +1100
@@ -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);
 }
 

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

[Greg KH]

On Wed, Nov 02, 2005 at 09:16:03PM +1100, NeilBrown wrote:
> 
> 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 <neilb@suse.de>

Nice, gotta love documentation :)

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>