Messages in this thread | | | Date | Mon, 6 Apr 2015 00:28:22 +0800 | Subject | Re: [PATCH 2/6] block: loop: don't hold lo_ctl_mutex in lo_open | From | Ming Lei <> |
| |
On Sun, Apr 5, 2015 at 3:24 PM, Ming Lei <ming.lei@canonical.com> wrote: > The lo_ctl_mutex is held for running all ioctl handlers, and > in some ioctl handlers, ioctl_by_bdev(BLKRRPART) is called for > rereading partitions, which requires bd_mutex. > > So it is easy to cause failure because trylock(bd_mutex) may > fail inside blkdev_reread_part(), and follows the lock context: > > blkid or other application: > ->open() > ->mutex_lock(bd_mutex) > ->lo_open() > ->mutex_lock(lo_ctl_mutex) > > losetup(set fd ioctl): > ->mutex_lock(lo_ctl_mutex) > ->ioctl_by_bdev(BLKRRPART) > ->trylock(bd_mutex) > > This patch trys to eliminate the ABBA lock dependency by removing > lo_ctl_mutext in lo_open() with the following approach: > > 1) introduce lo_open_mutex to protect lo_refcnt and avoid acquiring > lo_ctl_mutex in lo_open(): > - for open vs. add/del loop, no any problem because of loop_index_mutex > - lo_open_mutex is used for syncing open() and loop_clr_fd() > - both open() and release() have been serialized by bd_mutex already > > 2) don't hold lo_ctl_mutex for decreasing/checking lo_refcnt in > lo_release(), then lo_ctl_mutex is only required for the last release.
Another simpler way is to make lo_refcnt as atomic_t and remove lo_ctrl_mutext in lo_open(), and freeze request queue during clearing fd, and better to freeze queue during setting fd too, so will update in v1 with this way.
Thanks, Ming Lei
| |