loop: fix circular locking in loop_clr_fd()
authorNikanth Karthikesan <knikanth@suse.de>
Tue, 24 Mar 2009 11:33:41 +0000 (12:33 +0100)
committerJens Axboe <jens.axboe@oracle.com>
Thu, 26 Mar 2009 10:01:19 +0000 (11:01 +0100)
commitf028f3b2f987ebc61cef382ab7a5c449917b728e
tree6006b562900361a0af3cc50f564466ffce29c2a2
parent68db1961bbf4e16c220ccec4a780e966bc1fece3
loop: fix circular locking in loop_clr_fd()

With CONFIG_PROVE_LOCKING enabled

$ losetup /dev/loop0 file
$ losetup -o 32256 /dev/loop1 /dev/loop0

$ losetup -d /dev/loop1
$ losetup -d /dev/loop0

triggers a [ INFO: possible circular locking dependency detected ]

I think this warning is a false positive.

Open/close on a loop device acquires bd_mutex of the device before
acquiring lo_ctl_mutex of the same device. For ioctl(LOOP_CLR_FD) after
acquiring lo_ctl_mutex, fput on the backing_file might acquire the bd_mutex of
a device, if backing file is a device and this is the last reference to the
file being dropped . But it is guaranteed that it is impossible to have a
circular list of backing devices.(say loop2->loop1->loop0->loop2 is not
possible), which guarantees that this can never deadlock.

So this warning should be suppressed. It is very difficult to annotate lockdep
not to warn here in the correct way. A simple way to silence lockdep could be
to mark the lo_ctl_mutex in ioctl to be a sub class, but this might mask some
other real bugs.

@@ -1164,7 +1164,7 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
  struct loop_device *lo = bdev->bd_disk->private_data;
  int err;

- mutex_lock(&lo->lo_ctl_mutex);
+ mutex_lock_nested(&lo->lo_ctl_mutex, 1);
  switch (cmd) {
  case LOOP_SET_FD:
  err = loop_set_fd(lo, mode, bdev, arg);

Or actually marking the bd_mutex after lo_ctl_mutex as a sub class could be
a better solution.

Luckily it is easy to avoid calling fput on backing file with lo_ctl_mutex
held, so no lockdep annotation is required.

If you do not like the special handling of the lo_ctl_mutex just for the
LOOP_CLR_FD ioctl in lo_ioctl(), the mutex handling could be moved inside
each of the individual ioctl handlers and I could send you another patch.

Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
drivers/block/loop.c