aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlex Chiang <achiang@hp.com>2009-03-25 17:11:36 -0400
committerGreg Kroah-Hartman <gregkh@suse.de>2009-04-16 19:17:08 -0400
commitd110271e1f4140a9fb06d968b1afe9ca56a6064e (patch)
treeeb9ecf34607b5bb40ed26e339c5185cdb341e92f
parentc19f83669a02d4fa047d0d40f518e90f6f19c4c6 (diff)
sysfs: don't use global workqueue in sysfs_schedule_callback()
A sysfs attribute using sysfs_schedule_callback() to commit suicide may end up calling device_unregister(), which will eventually call a driver's ->remove function. Drivers may call flush_scheduled_work() in their shutdown routines, in which case lockdep will complain with something like the following: ============================================= [ INFO: possible recursive locking detected ] 2.6.29-rc8-kk #1 --------------------------------------------- events/4/56 is trying to acquire lock: (events){--..}, at: [<ffffffff80257fc0>] flush_workqueue+0x0/0xa0 but task is already holding lock: (events){--..}, at: [<ffffffff80257648>] run_workqueue+0x108/0x230 other info that might help us debug this: 3 locks held by events/4/56: #0: (events){--..}, at: [<ffffffff80257648>] run_workqueue+0x108/0x230 #1: (&ss->work){--..}, at: [<ffffffff80257648>] run_workqueue+0x108/0x230 #2: (pci_remove_rescan_mutex){--..}, at: [<ffffffff803c10d1>] remove_callback+0x21/0x40 stack backtrace: Pid: 56, comm: events/4 Not tainted 2.6.29-rc8-kk #1 Call Trace: [<ffffffff8026dfcd>] validate_chain+0xb7d/0x1260 [<ffffffff8026eade>] __lock_acquire+0x42e/0xa40 [<ffffffff8026f148>] lock_acquire+0x58/0x80 [<ffffffff80257fc0>] ? flush_workqueue+0x0/0xa0 [<ffffffff8025800d>] flush_workqueue+0x4d/0xa0 [<ffffffff80257fc0>] ? flush_workqueue+0x0/0xa0 [<ffffffff80258070>] flush_scheduled_work+0x10/0x20 [<ffffffffa0144065>] e1000_remove+0x55/0xfe [e1000e] [<ffffffff8033ee30>] ? sysfs_schedule_callback_work+0x0/0x50 [<ffffffff803bfeb2>] pci_device_remove+0x32/0x70 [<ffffffff80441da9>] __device_release_driver+0x59/0x90 [<ffffffff80441edb>] device_release_driver+0x2b/0x40 [<ffffffff804419d6>] bus_remove_device+0xa6/0x120 [<ffffffff8043e46b>] device_del+0x12b/0x190 [<ffffffff8043e4f6>] device_unregister+0x26/0x70 [<ffffffff803ba969>] pci_stop_dev+0x49/0x60 [<ffffffff803baab0>] pci_remove_bus_device+0x40/0xc0 [<ffffffff803c10d9>] remove_callback+0x29/0x40 [<ffffffff8033ee4f>] sysfs_schedule_callback_work+0x1f/0x50 [<ffffffff8025769a>] run_workqueue+0x15a/0x230 [<ffffffff80257648>] ? run_workqueue+0x108/0x230 [<ffffffff8025846f>] worker_thread+0x9f/0x100 [<ffffffff8025bce0>] ? autoremove_wake_function+0x0/0x40 [<ffffffff802583d0>] ? worker_thread+0x0/0x100 [<ffffffff8025b89d>] kthread+0x4d/0x80 [<ffffffff8020d4ba>] child_rip+0xa/0x20 [<ffffffff8020cebc>] ? restore_args+0x0/0x30 [<ffffffff8025b850>] ? kthread+0x0/0x80 [<ffffffff8020d4b0>] ? child_rip+0x0/0x20 Although we know that the device_unregister path will never acquire a lock that a driver might try to acquire in its ->remove, in general we should never attempt to flush a workqueue from within the same workqueue, and lockdep rightly complains. So as long as sysfs attributes cannot commit suicide directly and we are stuck with this callback mechanism, put the sysfs callbacks on their own workqueue instead of the global one. This has the side benefit that if a suicidal sysfs attribute kicks off a long chain of ->remove callbacks, we no longer induce a long delay on the global queue. This also fixes a missing module_put in the error path introduced by sysfs-only-allow-one-scheduled-removal-callback-per-kobj.patch. We never destroy the workqueue, but I'm not sure that's a problem. Reported-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com> Tested-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com> Signed-off-by: Alex Chiang <achiang@hp.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
-rw-r--r--fs/sysfs/file.c12
1 files changed, 11 insertions, 1 deletions
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 289c43a47263..979e9379fb5a 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -667,6 +667,7 @@ struct sysfs_schedule_callback_struct {
667 struct work_struct work; 667 struct work_struct work;
668}; 668};
669 669
670static struct workqueue_struct *sysfs_workqueue;
670static DEFINE_MUTEX(sysfs_workq_mutex); 671static DEFINE_MUTEX(sysfs_workq_mutex);
671static LIST_HEAD(sysfs_workq); 672static LIST_HEAD(sysfs_workq);
672static void sysfs_schedule_callback_work(struct work_struct *work) 673static void sysfs_schedule_callback_work(struct work_struct *work)
@@ -715,11 +716,20 @@ int sysfs_schedule_callback(struct kobject *kobj, void (*func)(void *),
715 mutex_lock(&sysfs_workq_mutex); 716 mutex_lock(&sysfs_workq_mutex);
716 list_for_each_entry_safe(ss, tmp, &sysfs_workq, workq_list) 717 list_for_each_entry_safe(ss, tmp, &sysfs_workq, workq_list)
717 if (ss->kobj == kobj) { 718 if (ss->kobj == kobj) {
719 module_put(owner);
718 mutex_unlock(&sysfs_workq_mutex); 720 mutex_unlock(&sysfs_workq_mutex);
719 return -EAGAIN; 721 return -EAGAIN;
720 } 722 }
721 mutex_unlock(&sysfs_workq_mutex); 723 mutex_unlock(&sysfs_workq_mutex);
722 724
725 if (sysfs_workqueue == NULL) {
726 sysfs_workqueue = create_workqueue("sysfsd");
727 if (sysfs_workqueue == NULL) {
728 module_put(owner);
729 return -ENOMEM;
730 }
731 }
732
723 ss = kmalloc(sizeof(*ss), GFP_KERNEL); 733 ss = kmalloc(sizeof(*ss), GFP_KERNEL);
724 if (!ss) { 734 if (!ss) {
725 module_put(owner); 735 module_put(owner);
@@ -735,7 +745,7 @@ int sysfs_schedule_callback(struct kobject *kobj, void (*func)(void *),
735 mutex_lock(&sysfs_workq_mutex); 745 mutex_lock(&sysfs_workq_mutex);
736 list_add_tail(&ss->workq_list, &sysfs_workq); 746 list_add_tail(&ss->workq_list, &sysfs_workq);
737 mutex_unlock(&sysfs_workq_mutex); 747 mutex_unlock(&sysfs_workq_mutex);
738 schedule_work(&ss->work); 748 queue_work(sysfs_workqueue, &ss->work);
739 return 0; 749 return 0;
740} 750}
741EXPORT_SYMBOL_GPL(sysfs_schedule_callback); 751EXPORT_SYMBOL_GPL(sysfs_schedule_callback);