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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> On  Mon, 4 Jun 2012 22:49:02 -0700, neil wrote:
> >On Tue, 5 Jun 2012 13:37:15 +0800 majianpeng <majianpeng@xxxxxxxxx> 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@xxxxxxxxx>
> >> ---
> >>  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.
> >NeilBrown

I've applied this patch to fix the problem.

Thanks,
NeilBrown

From 5fc056ff25485cecb8464a6fe09d3d486fd99d6a Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@xxxxxxx>
Date: Wed, 27 Jun 2012 13:53:52 +1000
Subject: [PATCH] md: make 'name' arg to md_register_thread non-optional.

Having the 'name' arg optional and defaulting to the current
personality name is no necessary and leads to errors, as when
changing the level of an array we can end up using the
name of the old level instead of the new one.

So make it non-optional and always explicitly pass the name
of the level that the array will be.

Reported-by: majianpeng <majianpeng@xxxxxxxxx>
Signed-off-by: NeilBrown <neilb@xxxxxxx>

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 973aa84..c601c4b 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 bcf6ea8..0ee7602 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3427,7 +3427,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 62b6b3a..a5135e5 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -4840,6 +4840,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
@@ -4963,7 +4964,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: signature.asc
Description: PGP signature


[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux