Skip to content
  • Alex Chiang's avatar
    sysfs: don't use global workqueue in sysfs_schedule_callback() · d110271e
    Alex Chiang authored
    
    
    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: default avatarKenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
    Tested-by: default avatarKenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
    Signed-off-by: default avatarAlex Chiang <achiang@hp.com>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
    d110271e