linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: NeilBrown <neilb@suse.de>
Cc: Jonathan Brassow <jbrassow@redhat.com>, linux-raid@vger.kernel.org
Subject: Re: [PATCH] MD: Remember the last sync operation that was performed
Date: Wed, 3 Jul 2013 09:45:53 +1000	[thread overview]
Message-ID: <20130703094553.530f21f9@notabene.brown> (raw)
In-Reply-To: <20130626120024.3ab35f02@notabene.brown>

[-- Attachment #1: Type: text/plain, Size: 11759 bytes --]

On Wed, 26 Jun 2013 12:00:24 +1000 NeilBrown <neilb@suse.de> wrote:

> On Tue, 25 Jun 2013 01:23:59 -0500 Jonathan Brassow <jbrassow@redhat.com>
> wrote:
> 
> > MD:  Remember the last sync operation that was performed
> > 
> > This patch adds a field to the mddev structure to track the last
> > sync operation that was performed.  This is especially useful when
> > it comes to what is recorded in mismatch_cnt in sysfs.  If the
> > last operation was "data-check", then it reports the number of
> > descrepancies found by the user-initiated check.  If it was a
> > "repair" operation, then it is reporting the number of
> > descrepancies repaired.  etc.
> > 
> > Signed-off-by: Jonathan Brassow <jbrassow@redhat.com>
> > 
> > Index: linux-upstream/drivers/md/dm-raid.c
> > ===================================================================
> > --- linux-upstream.orig/drivers/md/dm-raid.c
> > +++ linux-upstream/drivers/md/dm-raid.c
> > @@ -1388,6 +1388,7 @@ static void raid_status(struct dm_target
> >  		 *   performing a "check" of the array.
> >  		 */
> >  		DMEMIT(" %llu",
> > +		       (strcmp(rs->md.last_sync_action, "data-check")) ? 0 :
> >  		       (unsigned long long)
> >  		       atomic64_read(&rs->md.resync_mismatches));
> >  		break;
> > @@ -1651,7 +1652,7 @@ static void raid_resume(struct dm_target
> >  
> >  static struct target_type raid_target = {
> >  	.name = "raid",
> > -	.version = {1, 5, 1},
> > +	.version = {1, 5, 2},
> >  	.module = THIS_MODULE,
> >  	.ctr = raid_ctr,
> >  	.dtr = raid_dtr,
> > Index: linux-upstream/drivers/md/md.c
> > ===================================================================
> > --- linux-upstream.orig/drivers/md/md.c
> > +++ linux-upstream/drivers/md/md.c
> > @@ -521,6 +521,7 @@ void mddev_init(struct mddev *mddev)
> >  	init_waitqueue_head(&mddev->recovery_wait);
> >  	mddev->reshape_position = MaxSector;
> >  	mddev->reshape_backwards = 0;
> > +	mddev->last_sync_action = "none";
> >  	mddev->resync_min = 0;
> >  	mddev->resync_max = MaxSector;
> >  	mddev->level = LEVEL_NONE;
> > @@ -4272,6 +4273,17 @@ action_store(struct mddev *mddev, const
> >  	return len;
> >  }
> >  
> > +static struct md_sysfs_entry md_scan_mode =
> > +__ATTR(sync_action, S_IRUGO|S_IWUSR, action_show, action_store);
> > +
> > +static ssize_t
> > +last_sync_action_show(struct mddev *mddev, char *page)
> > +{
> > +	return sprintf(page, "%s\n", mddev->last_sync_action);
> > +}
> > +
> > +static struct md_sysfs_entry md_last_scan_mode = __ATTR_RO(last_sync_action);
> > +
> >  static ssize_t
> >  mismatch_cnt_show(struct mddev *mddev, char *page)
> >  {
> > @@ -4280,10 +4292,6 @@ mismatch_cnt_show(struct mddev *mddev, c
> >  		       atomic64_read(&mddev->resync_mismatches));
> >  }
> >  
> > -static struct md_sysfs_entry md_scan_mode =
> > -__ATTR(sync_action, S_IRUGO|S_IWUSR, action_show, action_store);
> > -
> > -
> >  static struct md_sysfs_entry md_mismatches = __ATTR_RO(mismatch_cnt);
> >  
> >  static ssize_t
> > @@ -4686,6 +4694,7 @@ static struct attribute *md_default_attr
> >  
> >  static struct attribute *md_redundancy_attrs[] = {
> >  	&md_scan_mode.attr,
> > +	&md_last_scan_mode.attr,
> >  	&md_mismatches.attr,
> >  	&md_sync_min.attr,
> >  	&md_sync_max.attr,
> > @@ -7336,7 +7345,7 @@ void md_do_sync(struct md_thread *thread
> >  		if (test_bit(MD_RECOVERY_CHECK, &mddev->recovery))
> >  			desc = "data-check";
> >  		else if (test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery))
> > -			desc = "requested-resync";
> > +			desc = "requested-resync"; /* AKA "repair" */
> >  		else
> >  			desc = "resync";
> >  	} else if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery))
> > @@ -7344,6 +7353,8 @@ void md_do_sync(struct md_thread *thread
> >  	else
> >  		desc = "recovery";
> >  
> > +	mddev->last_sync_action = desc;
> > +
> >  	/* we overload curr_resync somewhat here.
> >  	 * 0 == not engaged in resync at all
> >  	 * 2 == checking that there is no conflict with another sync
> > Index: linux-upstream/drivers/md/md.h
> > ===================================================================
> > --- linux-upstream.orig/drivers/md/md.h
> > +++ linux-upstream/drivers/md/md.h
> > @@ -268,6 +268,14 @@ struct mddev {
> >  
> >  	struct md_thread		*thread;	/* management thread */
> >  	struct md_thread		*sync_thread;	/* doing resync or reconstruct */
> > +
> > +	/* 'last_sync_action' is initialized to "none".  It is set when a
> > +	 * sync operation (i.e "data-check", "requested-resync", "resync",
> > +	 * "recovery", or "reshape") is started.  It holds this value even
> > +	 * when the sync thread is "frozen" (interrupted) or "idle" (stopped
> > +	 * or finished).  It is overwritten when a new sync operation is begun.
> > +	 */
> > +	char				*last_sync_action;
> >  	sector_t			curr_resync;	/* last block scheduled */
> >  	/* As resync requests can complete out of order, we cannot easily track
> >  	 * how much resync has been completed.  So we occasionally pause until
> > Index: linux-upstream/Documentation/device-mapper/dm-raid.txt
> > ===================================================================
> > --- linux-upstream.orig/Documentation/device-mapper/dm-raid.txt
> > +++ linux-upstream/Documentation/device-mapper/dm-raid.txt
> > @@ -223,3 +223,4 @@ Version History
> >  1.5.0   Add message interface to allow manipulation of the sync_action.
> >  	New status (STATUSTYPE_INFO) fields: sync_action and mismatch_cnt.
> >  1.5.1   Add ability to restore transiently failed devices on resume.
> > +1.5.2   'mismatch_cnt' is zero unless [last_]sync_action is "check".
> > 
> > 
> 
> Looks sane.  Applied, thanks.

Actually, I thought better of one little part of this.
The words "data-check" and "requested-resync" only appear in 'dmesg', not in
/sys or /proc and I want it to stay that way.
Also it is important that the new "last_sync_action" has the same set of
words as "sync_action".
So I made that change.  The new patch is below and in my for-next, and will
go to Linus tomorrow.

NeilBrown


From c4a39551451666229b4ea5e8aae8ca0131d00665 Mon Sep 17 00:00:00 2001
From: Jonathan Brassow <jbrassow@redhat.com>
Date: Tue, 25 Jun 2013 01:23:59 -0500
Subject: [PATCH] MD: Remember the last sync operation that was performed

MD:  Remember the last sync operation that was performed

This patch adds a field to the mddev structure to track the last
sync operation that was performed.  This is especially useful when
it comes to what is recorded in mismatch_cnt in sysfs.  If the
last operation was "data-check", then it reports the number of
descrepancies found by the user-initiated check.  If it was a
"repair" operation, then it is reporting the number of
descrepancies repaired.  etc.

Signed-off-by: Jonathan Brassow <jbrassow@redhat.com>
Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/Documentation/device-mapper/dm-raid.txt b/Documentation/device-mapper/dm-raid.txt
index 2bb3a68..ef8ba9f 100644
--- a/Documentation/device-mapper/dm-raid.txt
+++ b/Documentation/device-mapper/dm-raid.txt
@@ -223,3 +223,4 @@ Version History
 1.5.0   Add message interface to allow manipulation of the sync_action.
 	New status (STATUSTYPE_INFO) fields: sync_action and mismatch_cnt.
 1.5.1   Add ability to restore transiently failed devices on resume.
+1.5.2   'mismatch_cnt' is zero unless [last_]sync_action is "check".
diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index 21e8e46..4880b69 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -1388,6 +1388,7 @@ static void raid_status(struct dm_target *ti, status_type_t type,
 		 *   performing a "check" of the array.
 		 */
 		DMEMIT(" %llu",
+		       (strcmp(rs->md.last_sync_action, "check")) ? 0 :
 		       (unsigned long long)
 		       atomic64_read(&rs->md.resync_mismatches));
 		break;
@@ -1651,7 +1652,7 @@ static void raid_resume(struct dm_target *ti)
 
 static struct target_type raid_target = {
 	.name = "raid",
-	.version = {1, 5, 1},
+	.version = {1, 5, 2},
 	.module = THIS_MODULE,
 	.ctr = raid_ctr,
 	.dtr = raid_dtr,
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 26f9452..dddc87b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -521,6 +521,7 @@ void mddev_init(struct mddev *mddev)
 	init_waitqueue_head(&mddev->recovery_wait);
 	mddev->reshape_position = MaxSector;
 	mddev->reshape_backwards = 0;
+	mddev->last_sync_action = "none";
 	mddev->resync_min = 0;
 	mddev->resync_max = MaxSector;
 	mddev->level = LEVEL_NONE;
@@ -4272,6 +4273,17 @@ action_store(struct mddev *mddev, const char *page, size_t len)
 	return len;
 }
 
+static struct md_sysfs_entry md_scan_mode =
+__ATTR(sync_action, S_IRUGO|S_IWUSR, action_show, action_store);
+
+static ssize_t
+last_sync_action_show(struct mddev *mddev, char *page)
+{
+	return sprintf(page, "%s\n", mddev->last_sync_action);
+}
+
+static struct md_sysfs_entry md_last_scan_mode = __ATTR_RO(last_sync_action);
+
 static ssize_t
 mismatch_cnt_show(struct mddev *mddev, char *page)
 {
@@ -4280,10 +4292,6 @@ mismatch_cnt_show(struct mddev *mddev, char *page)
 		       atomic64_read(&mddev->resync_mismatches));
 }
 
-static struct md_sysfs_entry md_scan_mode =
-__ATTR(sync_action, S_IRUGO|S_IWUSR, action_show, action_store);
-
-
 static struct md_sysfs_entry md_mismatches = __ATTR_RO(mismatch_cnt);
 
 static ssize_t
@@ -4686,6 +4694,7 @@ static struct attribute *md_default_attrs[] = {
 
 static struct attribute *md_redundancy_attrs[] = {
 	&md_scan_mode.attr,
+	&md_last_scan_mode.attr,
 	&md_mismatches.attr,
 	&md_sync_min.attr,
 	&md_sync_max.attr,
@@ -7329,7 +7338,7 @@ void md_do_sync(struct md_thread *thread)
 	sector_t last_check;
 	int skipped = 0;
 	struct md_rdev *rdev;
-	char *desc;
+	char *desc, *action = NULL;
 	struct blk_plug plug;
 
 	/* just incase thread restarts... */
@@ -7339,17 +7348,21 @@ void md_do_sync(struct md_thread *thread)
 		return;
 
 	if (test_bit(MD_RECOVERY_SYNC, &mddev->recovery)) {
-		if (test_bit(MD_RECOVERY_CHECK, &mddev->recovery))
+		if (test_bit(MD_RECOVERY_CHECK, &mddev->recovery)) {
 			desc = "data-check";
-		else if (test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery))
+			action = "check";
+		} else if (test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery)) {
 			desc = "requested-resync";
-		else
+			action = "repair";
+		} else
 			desc = "resync";
 	} else if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery))
 		desc = "reshape";
 	else
 		desc = "recovery";
 
+	mddev->last_sync_action = action ?: desc;
+
 	/* we overload curr_resync somewhat here.
 	 * 0 == not engaged in resync at all
 	 * 2 == checking that there is no conflict with another sync
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 653f992b6..20f02c0 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -268,6 +268,14 @@ struct mddev {
 
 	struct md_thread		*thread;	/* management thread */
 	struct md_thread		*sync_thread;	/* doing resync or reconstruct */
+
+	/* 'last_sync_action' is initialized to "none".  It is set when a
+	 * sync operation (i.e "data-check", "requested-resync", "resync",
+	 * "recovery", or "reshape") is started.  It holds this value even
+	 * when the sync thread is "frozen" (interrupted) or "idle" (stopped
+	 * or finished).  It is overwritten when a new sync operation is begun.
+	 */
+	char				*last_sync_action;
 	sector_t			curr_resync;	/* last block scheduled */
 	/* As resync requests can complete out of order, we cannot easily track
 	 * how much resync has been completed.  So we occasionally pause until


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

      reply	other threads:[~2013-07-02 23:45 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-25  6:23 [PATCH] MD: Remember the last sync operation that was performed Jonathan Brassow
2013-06-26  2:00 ` NeilBrown
2013-07-02 23:45   ` NeilBrown [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130703094553.530f21f9@notabene.brown \
    --to=neilb@suse.de \
    --cc=jbrassow@redhat.com \
    --cc=linux-raid@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).