Re: [PATCH V1] md:Fix name of raid thread when raid takeovered.

[NeilBrown <neilb@suse.de>]

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

On Tue, 5 Jun 2012 14:06:06 +0800 majianpeng <majianpeng@gmail.com> wrote:

> On  Mon, 4 Jun 2012 22:49:02 -0700, neil wrote:
> >On Tue, 5 Jun 2012 13:37:15 +0800 majianpeng <majianpeng@gmail.com> wrote:
> >
> >> When raid takeovered, it used old personality to set task->comm[].
> >> So we add judgement and set corrected task->comm[].
> >> 
> >> Signed-off-by: majianpeng <majianpeng@gmail.com>
> >> ---
> >>  drivers/md/md.c |   14 ++++++++++++++
> >>  1 files changed, 14 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/drivers/md/md.c b/drivers/md/md.c
> >> index 1c2f904..6b3ce6e 100644
> >> --- a/drivers/md/md.c
> >> +++ b/drivers/md/md.c
> >> @@ -6738,11 +6738,25 @@ struct md_thread *md_register_thread(void (*run) (struct mddev *), struct mddev
> >>  				 const char *name)
> >>  {
> >>  	struct md_thread *thread;
> >> +	struct md_personality *pers = NULL;
> >>  
> >>  	thread = kzalloc(sizeof(struct md_thread), GFP_KERNEL);
> >>  	if (!thread)
> >>  		return NULL;
> >>  
> >> +	/*
> >> +	 *When raid takeovered,the mddev->pers was old,so the
> >> +	 *thread_name is wrong.
> >> +	 */
> >> +	if (name == NULL && mddev->level != mddev->new_level) {
> >> +		spin_lock(&pers_lock);
> >> +		list_for_each_entry(pers, &pers_list, list)
> >> +			if (pers->level == mddev->new_level) {
> >> +				name = pers->name;
> >> +				break;
> >> +			}
> >> +		spin_unlock(&pers_lock);
> >> +	}
> >>  	init_waitqueue_head(&thread->wqueue);
> >>  
> >>  	thread->run = run;
> >
> >No, I really don't like that.  It is to focussed on fixing the particular
> >symptom rather that making the code generally cleaner.
> >
> 
> >Was there a reason you didn't just change the last arg of md_register_thread
> >to be the level name instead of NULL??
> >
> No.I first wanted to corrected in which call md_register_thread.But I found it will take some codes.
> So i think corrected this function may better.

I don't think it is that much code.  Just this:

NeilBrown

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 1c2f904..2e70768 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6751,7 +6751,7 @@ struct md_thread *md_register_thread(void (*run) (struct mddev *), struct mddev
 	thread->tsk = kthread_run(md_thread, thread,
 				  "%s_%s",
 				  mdname(thread->mddev),
-				  name ?: mddev->pers->name);
+				  name);
 	if (IS_ERR(thread->tsk)) {
 		kfree(thread);
 		return NULL;
diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
index 9339e67..61a1833 100644
--- a/drivers/md/multipath.c
+++ b/drivers/md/multipath.c
@@ -474,7 +474,8 @@ static int multipath_run (struct mddev *mddev)
 	}
 
 	{
-		mddev->thread = md_register_thread(multipathd, mddev, NULL);
+		mddev->thread = md_register_thread(multipathd, mddev,
+						   "multipath");
 		if (!mddev->thread) {
 			printk(KERN_ERR "multipath: couldn't allocate thread"
 				" for %s\n", mdname(mddev));
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index a9c7981..39b2a8a 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2621,7 +2621,7 @@ static struct r1conf *setup_conf(struct mddev *mddev)
 		goto abort;
 	}
 	err = -ENOMEM;
-	conf->thread = md_register_thread(raid1d, mddev, NULL);
+	conf->thread = md_register_thread(raid1d, mddev, "raid1");
 	if (!conf->thread) {
 		printk(KERN_ERR
 		       "md/raid1:%s: couldn't allocate thread\n",
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 99ae606..994ac9a 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3421,7 +3421,7 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	spin_lock_init(&conf->resync_lock);
 	init_waitqueue_head(&conf->wait_barrier);
 
-	conf->thread = md_register_thread(raid10d, mddev, NULL);
+	conf->thread = md_register_thread(raid10d, mddev, "raid10");
 	if (!conf->thread)
 		goto out;
 
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index d267672..92c164f 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -4823,6 +4823,7 @@ static struct r5conf *setup_conf(struct mddev *mddev)
 	int raid_disk, memory, max_disks;
 	struct md_rdev *rdev;
 	struct disk_info *disk;
+	char pers_name[6];
 
 	if (mddev->new_level != 5
 	    && mddev->new_level != 4
@@ -4946,7 +4947,8 @@ static struct r5conf *setup_conf(struct mddev *mddev)
 		printk(KERN_INFO "md/raid:%s: allocated %dkB\n",
 		       mdname(mddev), memory);
 
-	conf->thread = md_register_thread(raid5d, mddev, NULL);
+	sprintf(pers_name, "raid%d", mddev->new_level);
+	conf->thread = md_register_thread(raid5d, mddev, pers_name);
 	if (!conf->thread) {
 		printk(KERN_ERR
 		       "md/raid:%s: couldn't allocate thread.\n",

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