From: Asias He <asias@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] block: Fix lock unbalance caused by lock disconnect
Date: Mon, 28 May 2012 10:15:18 +0800 [thread overview]
Message-ID: <4FC2DFB6.6080701@redhat.com> (raw)
In-Reply-To: <20120528000749.GA8305@dhcp-172-17-108-109.mtv.corp.google.com>
On 05/28/2012 08:07 AM, Tejun Heo wrote:
> On Fri, May 25, 2012 at 10:10:59AM +0800, Asias He wrote:
>> Commit 777eb1bf15b8532c396821774bf6451e563438f5 disconnects externally
>> supplied queue_lock before blk_drain_queue(). This would introduce lock
>> unbalance because theads which have taken the external lock might unlock
>> the internal lock in the during the queue drain.
>>
>> This patch fixes this by disconnecting the lock after the queue drain.
>
> I don't think the patch description is correct. The lock switcihng is
> inherently broken and the patch doesn't really fix the problem
> although it *might* make the problem less likely. Trying to switch
> locks while there are other accessors of the lock is simply broken, it
> can never work without outer synchronization.
Since the lock switching is broken, is it a good idea to force all the
drivers to use the block layer provided lock? i.e. Change the API from
blk_init_queue(rfn, driver_lock) to blk_init_queue(rfn). Any reason not
to use the block layer provided one.
> Your patch might make
> the problem somewhat less likely simply because queue draining makes a
> lot of request_queue users go away.
Who will use the request_queue after blk_cleanup_queue()?
> So, can you please update the commit description? It doesn't really
> *fix* anything but I think we're still better off with the change.
Sure. Will send v2.
--
Asias
next prev parent reply other threads:[~2012-05-28 2:14 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-25 2:10 [PATCH] block: Fix lock unbalance caused by lock disconnect Asias He
2012-05-28 0:07 ` Tejun Heo
2012-05-28 2:15 ` Asias He [this message]
2012-05-28 10:20 ` Tejun Heo
2012-05-29 1:49 ` Asias He
2012-06-01 9:28 ` Michael S. Tsirkin
2012-05-28 2:20 ` [PATCH v2] block: Mitigate " Asias He
2012-05-28 10:22 ` Tejun Heo
2012-05-29 1:39 ` [PATCH V3] block: Mitigate lock unbalance caused by lock switching Asias He
2012-05-29 1:39 ` [PATCH] " Asias He
2012-05-29 1:41 ` [PATCH V3] " Tejun Heo
2012-05-29 13:45 ` Tim Gardner
2012-05-30 6:28 ` Asias He
2012-05-30 6:28 ` Tejun Heo
2012-05-30 6:50 ` Asias He
2012-06-01 9:31 ` Jens Axboe
2012-06-06 2:12 ` Asias He
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4FC2DFB6.6080701@redhat.com \
--to=asias@redhat.com \
--cc=axboe@kernel.dk \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).