md: fix some lockdep issues between md and sysfs.

======
This fix is related to
    http://bugzilla.kernel.org/show_bug.cgi?id=15142
but does not address that exact issue.
======

sysfs does like attributes being removed while they are being accessed
(i.e. read or written) and waits for the access to complete.

As accessing some md attributes takes the same lock that is held while
removing those attributes a deadlock can occur.

This patch addresses 3 issues in md that could lead to this deadlock.

Two relate to calling flush_scheduled_work while the lock is held.
This is probably a bad idea in general and as we use schedule_work to
delete various sysfs objects it is particularly bad.

In one case flush_scheduled_work is called from md_alloc (called by
md_probe) called from do_md_run which holds the lock.  This call is
only present to ensure that ->gendisk is set.  However we can be sure
that gendisk is always set (though possibly we couldn't when that code
was originally written.  This is because do_md_run is called in three
different contexts:
  1/ from md_ioctl.  This requires that md_open has succeeded, and it
     fails if ->gendisk is not set.
  2/ from writing a sysfs attribute.  This can only happen if the
     mddev has been registered in sysfs which happens in md_alloc
     after ->gendisk has been set.
  3/ from autorun_array which is only called by autorun_devices, which
     checks for ->gendisk to be set before calling autorun_array.
So the call to md_probe in do_md_run can be removed, and the check on
->gendisk can also go.

In the other case flush_scheduled_work is being called in do_md_stop,
purportedly to wait for all md_delayed_delete calls (which delete the
component rdevs) to complete.  However there really isn't any need to
wait for them - they have already been disconnected in all important
ways.

The third issue is that raid5->stop() removes some attribute names
while the lock is held.  There is already some infrastructure in place
to delay attribute removal until after the lock is released (using
schedule_work).  So extend that infrastructure to remove the
raid5_attrs_group.

This does not address all lockdep issues related to the sysfs
"s_active" lock.  The rest can be address by splitting that lockdep
context between symlinks and non-symlinks which hopefully will happen.

Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/drivers/md/md.c b/drivers/md/md.c
index dd3dfe42d5a9d081c590018f1ac6325cfecdaf12..a20a71e5efd3ce0607993a84795e943a1706359d 100644 (file)
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -4075,8 +4075,10 @@ static void mddev_delayed_delete(struct work_struct *ws)
 {
        mddev_t *mddev = container_of(ws, mddev_t, del_work);
 
-       if (mddev->private == &md_redundancy_group) {
+       if (mddev->private) {
                sysfs_remove_group(&mddev->kobj, &md_redundancy_group);
+               if (mddev->private != (void*)1)
+                       sysfs_remove_group(&mddev->kobj, mddev->private);
                if (mddev->sysfs_action)
                        sysfs_put(mddev->sysfs_action);
                mddev->sysfs_action = NULL;
@@ -4287,10 +4289,7 @@ static int do_md_run(mddev_t * mddev)
                sysfs_notify_dirent(rdev->sysfs_state);
        }
 
-       md_probe(mddev->unit, NULL, NULL);
        disk = mddev->gendisk;
-       if (!disk)
-               return -ENOMEM;
 
        spin_lock(&pers_lock);
        pers = find_pers(mddev->level, mddev->clevel);
@@ -4530,8 +4529,8 @@ static int do_md_stop(mddev_t * mddev, int mode, int is_open)
                        mddev->queue->unplug_fn = NULL;
                        mddev->queue->backing_dev_info.congested_fn = NULL;
                        module_put(mddev->pers->owner);
-                       if (mddev->pers->sync_request)
-                               mddev->private = &md_redundancy_group;
+                       if (mddev->pers->sync_request && mddev->private == NULL)
+                               mddev->private = (void*)1;
                        mddev->pers = NULL;
                        /* tell userspace to handle 'inactive' */
                        sysfs_notify_dirent(mddev->sysfs_state);
@@ -4578,9 +4577,6 @@ out:
                }
                mddev->bitmap_info.offset = 0;
 
-               /* make sure all md_delayed_delete calls have finished */
-               flush_scheduled_work();
-
                export_array(mddev);
 
                mddev->array_sectors = 0;

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index b5629c3e14fad7583d2bc8ace875c9adaf8c2bd6..ceb24afdc147aada31741b10213443375617257f 100644 (file)
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5136,9 +5136,8 @@ static int stop(mddev_t *mddev)
        mddev->thread = NULL;
        mddev->queue->backing_dev_info.congested_fn = NULL;
        blk_sync_queue(mddev->queue); /* the unplug fn references 'conf'*/
-       sysfs_remove_group(&mddev->kobj, &raid5_attrs_group);
        free_conf(conf);
-       mddev->private = NULL;
+       mddev->private = &raid5_attrs_group;
        return 0;
 }