public inbox for linux-s390@vger.kernel.org
 help / color / mirror / Atom feed
From: Jarod Wilson <jarod@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: Ming Lei <ming.lei@canonical.com>,
	Christoph Hellwig <hch@infradead.org>,
	Jens Axboe <axboe@kernel.dk>, Tejun Heo <tj@kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Markus Pargmann <mpa@pengutronix.de>,
	Stefan Weinhuber <wein@de.ibm.com>,
	Stefan Haberland <stefan.haberland@de.ibm.com>,
	Sebastian Ott <sebott@linux.vnet.ibm.com>,
	Fabian Frederick <fabf@skynet.be>,
	David Herrmann <dh.herrmann@gmail.com>,
	Mike Galbraith <bitbucket@online.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Peter Zijlstra <peterz@infradead.org>,
	nbd-general@lists.sourceforge.net, linux-s390@vger.kernel.org,
	Jarod Wilson <jarod@redhat.com>
Subject: [PATCH 6/7] block: replace trylock with mutex_lock in blkdev_reread_part()
Date: Wed,  8 Apr 2015 02:23:45 -0400	[thread overview]
Message-ID: <1428474226-27386-7-git-send-email-jarod@redhat.com> (raw)
In-Reply-To: <1428474226-27386-1-git-send-email-jarod@redhat.com>

From: Ming Lei <ming.lei@canonical.com>

The only possible problem of using mutex_lock() instead of trylock
is about deadlock.

If there aren't any locks held before calling blkdev_reread_part(lock),
deadlock can't be caused by this conversion.

If there are locks held before calling blkdev_reread_part(lock),
and if these locks arn't required in open, close handler and I/O
path, deadlock shouldn't be caused too.

Both user space's ioctl(BLKRRPART) and md_setup_drive() from
init/do_mounts_md.c belongs to the 1st case, so the conversion is safe
for the two cases.

For loop, the previous patches in this pathset has fixed the ABBA lock
dependency, so the conversion is OK.

For nbd, tx_lock is held when calling the function:

        - both open and release won't hold the lock
        - when blkdev_reread_part() is run, I/O thread has been stopped
        already, so tx_lock won't be acquired in I/O path at that time.
        - so the conversion won't cause deadlock for nbd

For dasd, both dasd_open(), dasd_release() and request function don't
acquire any mutex/semphone, so the conversion should be safe.

[jarod: update for modifications earlier in series.]

CC: Christoph Hellwig <hch@infradead.org>
CC: Jens Axboe <axboe@kernel.dk>
CC: Tejun Heo <tj@kernel.org>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Markus Pargmann <mpa@pengutronix.de>
CC: Stefan Weinhuber <wein@de.ibm.com>
CC: Stefan Haberland <stefan.haberland@de.ibm.com>
CC: Sebastian Ott <sebott@linux.vnet.ibm.com>
CC: Fabian Frederick <fabf@skynet.be>
CC: Ming Lei <ming.lei@canonical.com>
CC: David Herrmann <dh.herrmann@gmail.com>
CC: Mike Galbraith <bitbucket@online.de>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: nbd-general@lists.sourceforge.net
CC: linux-s390@vger.kernel.org
Signed-off-by: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 block/ioctl.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index 64a4fcb..47c8e6d 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -170,15 +170,19 @@ EXPORT_SYMBOL(__blkdev_reread_part);
 
 /*
  * This is an exported API for the block driver, and will
- * acquire bd_mutex. Make sure you aren't calling it with
- * bd_mutex already held, or we'll return -EBUSY.
+ * acquire bd_mutex.
+ *
+ * Make sure held locks aren't required in open()/close()
+ * handlers and I/O paths to avoid an ABBA deadlock:
+ * - bd_mutex is held before calling a block driver's open/close
+ *   handler
+ * - reading a partition table may submit I/O to the block device
  */
 int blkdev_reread_part(struct block_device *bdev)
 {
 	int res;
 
-	if (!mutex_trylock(&bdev->bd_mutex))
-		return -EBUSY;
+	mutex_lock(&bdev->bd_mutex);
 	res = __blkdev_reread_part(bdev);
 	mutex_unlock(&bdev->bd_mutex);
 
-- 
1.8.3.1

  parent reply	other threads:[~2015-04-08  6:23 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-05  7:24 [PATCH 0/6] block: reread partitions changes and fix for loop Ming Lei
2015-04-05  7:24 ` [PATCH 1/6] block: export blkdev_reread_part() Ming Lei
2015-04-05 16:12   ` Christoph Hellwig
2015-04-05 16:40     ` Ming Lei
2015-04-06 13:42       ` Jarod Wilson
2015-04-06 13:48         ` Jarod Wilson
2015-04-07  2:43         ` Ming Lei
2015-04-06 14:50       ` Christoph Hellwig
2015-04-07  2:11         ` Ming Lei
2015-04-05  7:24 ` [PATCH 2/6] block: loop: don't hold lo_ctl_mutex in lo_open Ming Lei
2015-04-05 16:28   ` Ming Lei
2015-04-06 14:49     ` Christoph Hellwig
2015-04-05  7:24 ` [PATCH 3/6] block: loop: fix another reread part failure Ming Lei
2015-04-05  7:24 ` [PATCH 4/6] block: nbd: convert to blkdev_reread_part() Ming Lei
2015-04-05  7:24 ` [PATCH 5/6] block: dasd_genhd: convert to blkdev_reread_part Ming Lei
2015-04-06 13:46   ` Jarod Wilson
2015-04-06 13:51     ` Jarod Wilson
2015-04-07  1:59       ` Ming Lei
2015-04-07  0:47     ` Ming Lei
2015-04-05  7:24 ` [RFC PATCH 6/6] block: replace trylock with mutex_lock in __blkdev_reread_part() Ming Lei
2015-04-08  6:23 ` [PATCH 0/7] block: reread partitions improvements Jarod Wilson
2015-04-08  6:23   ` [PATCH 1/7] block: export blkdev_reread_part() and __blkdev_reread_part() Jarod Wilson
2015-04-08 14:50     ` Ming Lei
2015-04-08 15:03       ` Peter Zijlstra
2015-04-08 15:27         ` Jarod Wilson
2015-04-08 15:28         ` Ming Lei
2015-04-08  6:23   ` [PATCH 2/7] block: loop: don't hold lo_ctl_mutex in lo_open Jarod Wilson
2015-04-08  6:50     ` Ming Lei
2015-04-08 13:40       ` Jarod Wilson
2015-04-08 14:00         ` Jarod Wilson
2015-04-08 14:20           ` Ming Lei
2015-04-08 15:28             ` Jarod Wilson
2015-04-08  6:23   ` [PATCH 3/7] block: loop: fix another reread part failure Jarod Wilson
2015-04-08  6:23   ` [PATCH 4/7] block: nbd: convert to blkdev_reread_part() Jarod Wilson
2015-04-08  6:23   ` [PATCH 5/7] block: dasd_genhd: convert to blkdev_reread_part Jarod Wilson
2015-04-08  6:23   ` Jarod Wilson [this message]
2015-04-08  6:23   ` [PATCH 7/7] s390/block/dasd: remove obsolete while -EBUSY loop Jarod Wilson

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=1428474226-27386-7-git-send-email-jarod@redhat.com \
    --to=jarod@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=bitbucket@online.de \
    --cc=dh.herrmann@gmail.com \
    --cc=fabf@skynet.be \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=ming.lei@canonical.com \
    --cc=mpa@pengutronix.de \
    --cc=nbd-general@lists.sourceforge.net \
    --cc=peterz@infradead.org \
    --cc=sebott@linux.vnet.ibm.com \
    --cc=stefan.haberland@de.ibm.com \
    --cc=tj@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=wein@de.ibm.com \
    /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