Skip to content
  • Tejun Heo's avatar
    sysfs: use a separate locking class for open files depending on mmap · 027a485d
    Tejun Heo authored
    The following two commits implemented mmap support in the regular file
    path and merged bin file support into the regular path.
    
     73d97146 ("sysfs: copy bin mmap support from fs/sysfs/bin.c to fs/sysfs/file.c")
     3124eb16
    
     ("sysfs: merge regular and bin file handling")
    
    After the merge, the following commands trigger a spurious lockdep
    warning.  "test-mmap-read" simply mmaps the file and dumps the
    content.
    
      $ cat /sys/block/sda/trace/act_mask
      $ test-mmap-read /sys/devices/pci0000\:00/0000\:00\:03.0/resource0 4096
    
      ======================================================
      [ INFO: possible circular locking dependency detected ]
      3.12.0-work+ #378 Not tainted
      -------------------------------------------------------
      test-mmap-read/567 is trying to acquire lock:
       (&of->mutex){+.+.+.}, at: [<ffffffff8120a8df>] sysfs_bin_mmap+0x4f/0x120
    
      but task is already holding lock:
       (&mm->mmap_sem){++++++}, at: [<ffffffff8114b399>] vm_mmap_pgoff+0x49/0xa0
    
      which lock already depends on the new lock.
    
      the existing dependency chain (in reverse order) is:
    
      -> #3 (&mm->mmap_sem){++++++}:
      ...
      -> #2 (sr_mutex){+.+.+.}:
      ...
      -> #1 (&bdev->bd_mutex){+.+.+.}:
      ...
      -> #0 (&of->mutex){+.+.+.}:
      ...
    
      other info that might help us debug this:
    
      Chain exists of:
       &of->mutex --> sr_mutex --> &mm->mmap_sem
    
       Possible unsafe locking scenario:
    
    	 CPU0                    CPU1
    	 ----                    ----
        lock(&mm->mmap_sem);
    				 lock(sr_mutex);
    				 lock(&mm->mmap_sem);
        lock(&of->mutex);
    
       *** DEADLOCK ***
    
      1 lock held by test-mmap-read/567:
       #0:  (&mm->mmap_sem){++++++}, at: [<ffffffff8114b399>] vm_mmap_pgoff+0x49/0xa0
    
      stack backtrace:
      CPU: 3 PID: 567 Comm: test-mmap-read Not tainted 3.12.0-work+ #378
      Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
       ffffffff81ed41a0 ffff880009441bc8 ffffffff81611ad2 ffffffff81eccb80
       ffff880009441c08 ffffffff8160f215 ffff880009441c60 ffff880009c75208
       0000000000000000 ffff880009c751e0 ffff880009c75208 ffff880009c74ac0
      Call Trace:
       [<ffffffff81611ad2>] dump_stack+0x4e/0x7a
       [<ffffffff8160f215>] print_circular_bug+0x2b0/0x2bf
       [<ffffffff8109ca0a>] __lock_acquire+0x1a3a/0x1e60
       [<ffffffff8109d6ba>] lock_acquire+0x9a/0x1d0
       [<ffffffff81615547>] mutex_lock_nested+0x67/0x3f0
       [<ffffffff8120a8df>] sysfs_bin_mmap+0x4f/0x120
       [<ffffffff8115d363>] mmap_region+0x3b3/0x5b0
       [<ffffffff8115d8ae>] do_mmap_pgoff+0x34e/0x3d0
       [<ffffffff8114b3ba>] vm_mmap_pgoff+0x6a/0xa0
       [<ffffffff8115be3e>] SyS_mmap_pgoff+0xbe/0x250
       [<ffffffff81008282>] SyS_mmap+0x22/0x30
       [<ffffffff8161a4d2>] system_call_fastpath+0x16/0x1b
    
    This happens because one file nests sr_mutex, which nests mm->mmap_sem
    under it, under of->mutex while mmap implementation naturally nests
    of->mutex under mm->mmap_sem.  The warning is false positive as
    of->mutex is per open-file and the two paths belong to two different
    files.  This warning didn't trigger before regular and bin file
    supports were merged because only bin file supported mmap and the
    other side of locking happened only on regular files which used
    equivalent but separate locking.
    
    It'd be best if we give separate locking classes per file but we can't
    easily do that.  Let's differentiate on ->mmap() for now.  Later we'll
    add explicit file operations struct and can add per-ops lockdep key
    there.
    
    Signed-off-by: default avatarTejun Heo <tj@kernel.org>
    Reported-by: default avatarDave Jones <davej@redhat.com>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    027a485d