Re: [Nbd] [PATCH 2/7] block: loop: don't hold lo_ctl_mutex in lo_open
- To: Jarod Wilson <jarod@...696...>
- Cc: Jens Axboe <axboe@...161...>, Stefan Weinhuber <wein@...1296...>, "nbd-general@lists.sourceforge.net" <nbd-general@lists.sourceforge.net>, linux-s390@...25..., Mike Galbraith <bitbucket@...1990...>, Stefan Haberland <stefan.haberland@...1296...>, Sebastian Ott <sebott@...1294...>, Linux Kernel Mailing List <linux-kernel@...25...>, Christoph Hellwig <hch@...1270...>, Peter Zijlstra <peterz@...1270...>, Fabian Frederick <fabf@...1980...>, Alexander Viro <viro@...1300...>, David Herrmann <dh.herrmann@...17...>, Tejun Heo <tj@...1285...>, Andrew Morton <akpm@...133...>
- Subject: Re: [Nbd] [PATCH 2/7] block: loop: don't hold lo_ctl_mutex in lo_open
- From: Ming Lei <ming.lei@...1301...>
- Date: Wed, 8 Apr 2015 22:20:22 +0800
- Message-id: <CACVXFVM8qXs6nEMGURhAZFJ4cuGNvCc-gM_hryaVvERMTdTfpg@...18...>
- In-reply-to: <20150408140028.GI14217@...696...>
- References: <1428218688-4092-1-git-send-email-ming.lei@...1301...> <1428474226-27386-1-git-send-email-jarod@...696...> <1428474226-27386-3-git-send-email-jarod@...696...> <CACVXFVPtaG4EDjAJ5G=Qz29fE4Zfp==W0izdCK06ZnJm17jO8Q@...18...> <20150408134034.GH14217@...696...> <20150408140028.GI14217@...696...>
On Wed, Apr 8, 2015 at 10:00 PM, Jarod Wilson <jarod@...696...> wrote:
> On Wed, Apr 08, 2015 at 09:40:34AM -0400, Jarod Wilson wrote:
>> On Wed, Apr 08, 2015 at 02:50:59PM +0800, Ming Lei wrote:
>> > Hi Jarod,
>> >
>> > On Wed, Apr 8, 2015 at 2:23 PM, Jarod Wilson <jarod@...696...> wrote:
>> > > From: Ming Lei <ming.lei@...1301...>
>> > >
>> > > 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():
>> >
>> > It is a bit quick since I said the lo_open_mutex can be removed,
>> > and Christoph agreed that too.
>> >
>> > So looks we still need to post another version, :-)
>>
>> Ah. I missed that bit. Just trying to keep up momentum.
>
> Are you working on that atomic_t version then, or should I dive into it?
It has been ready, and I will post them out once it passes my tests.
Thanks,
Reply to: