diff options
author | NeilBrown <neilb@suse.de> | 2010-08-08 07:18:03 -0400 |
---|---|---|
committer | NeilBrown <neilb@suse.de> | 2010-08-08 07:21:27 -0400 |
commit | bb4f1e9d0e2ef93de8e36ca0f5f26625fcd70b7d (patch) | |
tree | 7c7edc0d5fa2b5702358f11396d52d07183708c0 | |
parent | 147e0b6a639ac581ca3bf627bedc3f4a6d3eca66 (diff) |
md: fix another deadlock with removing sysfs attributes.
Move the deletion of sysfs attributes from reconfig_mutex to
open_mutex didn't really help as a process can try to take
open_mutex while holding reconfig_mutex, so the same deadlock can
happen, just requiring one more process to be involved in the chain.
I looks like I cannot easily use locking to wait for the sysfs
deletion to complete, so don't.
The only things that we cannot do while the deletions are still
pending is other things which can change the sysfs namespace: run,
takeover, stop. Each of these can fail with -EBUSY.
So set a flag while doing a sysfs deletion, and fail run, takeover,
stop if that flag is set.
This is suitable for 2.6.35.x
Cc: stable@kernel.org
Signed-off-by: NeilBrown <neilb@suse.de>
-rw-r--r-- | drivers/md/md.c | 31 | ||||
-rw-r--r-- | drivers/md/md.h | 4 |
2 files changed, 21 insertions, 14 deletions
diff --git a/drivers/md/md.c b/drivers/md/md.c index 00c3fde39a12..03dcbfbe250f 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c | |||
@@ -580,13 +580,17 @@ static void mddev_unlock(mddev_t * mddev) | |||
580 | * an access to the files will try to take reconfig_mutex | 580 | * an access to the files will try to take reconfig_mutex |
581 | * while holding the file unremovable, which leads to | 581 | * while holding the file unremovable, which leads to |
582 | * a deadlock. | 582 | * a deadlock. |
583 | * So hold open_mutex instead - we are allowed to take | 583 | * So hold set sysfs_active while the remove in happeing, |
584 | * it while holding reconfig_mutex, and md_run can | 584 | * and anything else which might set ->to_remove or my |
585 | * use it to wait for the remove to complete. | 585 | * otherwise change the sysfs namespace will fail with |
586 | * -EBUSY if sysfs_active is still set. | ||
587 | * We set sysfs_active under reconfig_mutex and elsewhere | ||
588 | * test it under the same mutex to ensure its correct value | ||
589 | * is seen. | ||
586 | */ | 590 | */ |
587 | struct attribute_group *to_remove = mddev->to_remove; | 591 | struct attribute_group *to_remove = mddev->to_remove; |
588 | mddev->to_remove = NULL; | 592 | mddev->to_remove = NULL; |
589 | mutex_lock(&mddev->open_mutex); | 593 | mddev->sysfs_active = 1; |
590 | mutex_unlock(&mddev->reconfig_mutex); | 594 | mutex_unlock(&mddev->reconfig_mutex); |
591 | 595 | ||
592 | if (mddev->kobj.sd) { | 596 | if (mddev->kobj.sd) { |
@@ -600,7 +604,7 @@ static void mddev_unlock(mddev_t * mddev) | |||
600 | mddev->sysfs_action = NULL; | 604 | mddev->sysfs_action = NULL; |
601 | } | 605 | } |
602 | } | 606 | } |
603 | mutex_unlock(&mddev->open_mutex); | 607 | mddev->sysfs_active = 0; |
604 | } else | 608 | } else |
605 | mutex_unlock(&mddev->reconfig_mutex); | 609 | mutex_unlock(&mddev->reconfig_mutex); |
606 | 610 | ||
@@ -3008,7 +3012,9 @@ level_store(mddev_t *mddev, const char *buf, size_t len) | |||
3008 | * - new personality will access other array. | 3012 | * - new personality will access other array. |
3009 | */ | 3013 | */ |
3010 | 3014 | ||
3011 | if (mddev->sync_thread || mddev->reshape_position != MaxSector) | 3015 | if (mddev->sync_thread || |
3016 | mddev->reshape_position != MaxSector || | ||
3017 | mddev->sysfs_active) | ||
3012 | return -EBUSY; | 3018 | return -EBUSY; |
3013 | 3019 | ||
3014 | if (!mddev->pers->quiesce) { | 3020 | if (!mddev->pers->quiesce) { |
@@ -4393,13 +4399,9 @@ int md_run(mddev_t *mddev) | |||
4393 | 4399 | ||
4394 | if (mddev->pers) | 4400 | if (mddev->pers) |
4395 | return -EBUSY; | 4401 | return -EBUSY; |
4396 | 4402 | /* Cannot run until previous stop completes properly */ | |
4397 | /* These two calls synchronise us with the | 4403 | if (mddev->sysfs_active) |
4398 | * sysfs_remove_group calls in mddev_unlock, | 4404 | return -EBUSY; |
4399 | * so they must have completed. | ||
4400 | */ | ||
4401 | mutex_lock(&mddev->open_mutex); | ||
4402 | mutex_unlock(&mddev->open_mutex); | ||
4403 | 4405 | ||
4404 | /* | 4406 | /* |
4405 | * Analyze all RAID superblock(s) | 4407 | * Analyze all RAID superblock(s) |
@@ -4770,7 +4772,8 @@ static int do_md_stop(mddev_t * mddev, int mode, int is_open) | |||
4770 | mdk_rdev_t *rdev; | 4772 | mdk_rdev_t *rdev; |
4771 | 4773 | ||
4772 | mutex_lock(&mddev->open_mutex); | 4774 | mutex_lock(&mddev->open_mutex); |
4773 | if (atomic_read(&mddev->openers) > is_open) { | 4775 | if (atomic_read(&mddev->openers) > is_open || |
4776 | mddev->sysfs_active) { | ||
4774 | printk("md: %s still in use.\n",mdname(mddev)); | 4777 | printk("md: %s still in use.\n",mdname(mddev)); |
4775 | err = -EBUSY; | 4778 | err = -EBUSY; |
4776 | } else if (mddev->pers) { | 4779 | } else if (mddev->pers) { |
diff --git a/drivers/md/md.h b/drivers/md/md.h index cccbadb31bae..6f797eceae31 100644 --- a/drivers/md/md.h +++ b/drivers/md/md.h | |||
@@ -145,6 +145,10 @@ struct mddev_s | |||
145 | int suspended; | 145 | int suspended; |
146 | atomic_t active_io; | 146 | atomic_t active_io; |
147 | int ro; | 147 | int ro; |
148 | int sysfs_active; /* set when sysfs deletes | ||
149 | * are happening, so run/ | ||
150 | * takeover/stop are not safe | ||
151 | */ | ||
148 | 152 | ||
149 | struct gendisk *gendisk; | 153 | struct gendisk *gendisk; |
150 | 154 | ||