All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Jens Axboe <axboe@kernel.dk>
Cc: <linux-block@vger.kernel.org>,
	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	Jan Kara <jack@suse.cz>
Subject: [PATCH 07/16] loop: Push loop_ctl_mutex down into loop_clr_fd()
Date: Thu,  8 Nov 2018 14:01:07 +0100	[thread overview]
Message-ID: <20181108130116.12140-8-jack@suse.cz> (raw)
In-Reply-To: <20181108130116.12140-1-jack@suse.cz>

loop_clr_fd() has a weird locking convention that is expects
loop_ctl_mutex held, releases it on success and keeps it on failure.
Untangle the mess by moving locking of loop_ctl_mutex into
loop_clr_fd().

Signed-off-by: Jan Kara <jack@suse.cz>
---
 drivers/block/loop.c | 49 +++++++++++++++++++++++++++++--------------------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index eb01a685da4e..d8a7b5da881b 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1027,15 +1027,22 @@ loop_init_xfer(struct loop_device *lo, struct loop_func_table *xfer,
 
 static int __loop_clr_fd(struct loop_device *lo)
 {
-	struct file *filp = lo->lo_backing_file;
+	struct file *filp = NULL;
 	gfp_t gfp = lo->old_gfp_mask;
 	struct block_device *bdev = lo->lo_device;
+	int err = 0;
 
-	if (WARN_ON_ONCE(lo->lo_state != Lo_rundown))
-		return -ENXIO;
+	mutex_lock(&loop_ctl_mutex);
+	if (WARN_ON_ONCE(lo->lo_state != Lo_rundown)) {
+		err = -ENXIO;
+		goto out_unlock;
+	}
 
-	if (filp == NULL)
-		return -EINVAL;
+	filp = lo->lo_backing_file;
+	if (filp == NULL) {
+		err = -EINVAL;
+		goto out_unlock;
+	}
 
 	/* freeze request queue during the transition */
 	blk_mq_freeze_queue(lo->lo_queue);
@@ -1082,6 +1089,7 @@ static int __loop_clr_fd(struct loop_device *lo)
 	if (!part_shift)
 		lo->lo_disk->flags |= GENHD_FL_NO_PART_SCAN;
 	loop_unprepare_queue(lo);
+out_unlock:
 	mutex_unlock(&loop_ctl_mutex);
 	/*
 	 * Need not hold loop_ctl_mutex to fput backing file.
@@ -1089,14 +1097,22 @@ static int __loop_clr_fd(struct loop_device *lo)
 	 * lock dependency possibility warning as fput can take
 	 * bd_mutex which is usually taken before loop_ctl_mutex.
 	 */
-	fput(filp);
-	return 0;
+	if (filp)
+		fput(filp);
+	return err;
 }
 
 static int loop_clr_fd(struct loop_device *lo)
 {
-	if (lo->lo_state != Lo_bound)
+	int err;
+
+	err = mutex_lock_killable_nested(&loop_ctl_mutex, 1);
+	if (err)
+		return err;
+	if (lo->lo_state != Lo_bound) {
+		mutex_unlock(&loop_ctl_mutex);
 		return -ENXIO;
+	}
 	/*
 	 * If we've explicitly asked to tear down the loop device,
 	 * and it has an elevated reference count, set it for auto-teardown when
@@ -1113,6 +1129,7 @@ static int loop_clr_fd(struct loop_device *lo)
 		return 0;
 	}
 	lo->lo_state = Lo_rundown;
+	mutex_unlock(&loop_ctl_mutex);
 
 	return __loop_clr_fd(lo);
 }
@@ -1447,14 +1464,7 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
 		mutex_unlock(&loop_ctl_mutex);
 		break;
 	case LOOP_CLR_FD:
-		err = mutex_lock_killable_nested(&loop_ctl_mutex, 1);
-		if (err)
-			return err;
-		/* loop_clr_fd would have unlocked loop_ctl_mutex on success */
-		err = loop_clr_fd(lo);
-		if (err)
-			mutex_unlock(&loop_ctl_mutex);
-		break;
+		return loop_clr_fd(lo);
 	case LOOP_SET_STATUS:
 		err = -EPERM;
 		if ((mode & FMODE_WRITE) || capable(CAP_SYS_ADMIN)) {
@@ -1690,7 +1700,6 @@ static int lo_open(struct block_device *bdev, fmode_t mode)
 static void lo_release(struct gendisk *disk, fmode_t mode)
 {
 	struct loop_device *lo;
-	int err;
 
 	mutex_lock(&loop_ctl_mutex);
 	lo = disk->private_data;
@@ -1701,13 +1710,13 @@ static void lo_release(struct gendisk *disk, fmode_t mode)
 		if (lo->lo_state != Lo_bound)
 			goto out_unlock;
 		lo->lo_state = Lo_rundown;
+		mutex_unlock(&loop_ctl_mutex);
 		/*
 		 * In autoclear mode, stop the loop thread
 		 * and remove configuration after last close.
 		 */
-		err = __loop_clr_fd(lo);
-		if (!err)
-			return;
+		__loop_clr_fd(lo);
+		return;
 	} else if (lo->lo_state == Lo_bound) {
 		/*
 		 * Otherwise keep thread (if running) and config,
-- 
2.16.4

  parent reply	other threads:[~2018-11-08 22:36 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-08 13:01 [PATCH 0/16 v3] loop: Fix oops and possible deadlocks Jan Kara
2018-11-08 13:01 ` [PATCH 01/16] block/loop: Don't grab "struct file" for vfs_getattr() operation Jan Kara
2018-11-08 13:01 ` [PATCH 02/16] block/loop: Use global lock for ioctl() operation Jan Kara
2018-11-08 13:01 ` [PATCH 03/16] loop: Fold __loop_release into loop_release Jan Kara
2018-11-08 13:01 ` [PATCH 04/16] loop: Get rid of loop_index_mutex Jan Kara
2018-11-08 13:01 ` [PATCH 05/16] loop: Push lo_ctl_mutex down into individual ioctls Jan Kara
2018-11-08 13:01 ` [PATCH 06/16] loop: Split setting of lo_state from loop_clr_fd Jan Kara
2018-11-08 13:01 ` Jan Kara [this message]
2018-11-08 13:01 ` [PATCH 08/16] loop: Push loop_ctl_mutex down to loop_get_status() Jan Kara
2018-11-08 13:01 ` [PATCH 09/16] loop: Push loop_ctl_mutex down to loop_set_status() Jan Kara
2018-11-08 13:01 ` [PATCH 10/16] loop: Push loop_ctl_mutex down to loop_set_fd() Jan Kara
2018-11-08 13:01 ` [PATCH 11/16] loop: Push loop_ctl_mutex down to loop_change_fd() Jan Kara
2018-11-08 13:01 ` [PATCH 12/16] loop: Move special partition reread handling in loop_clr_fd() Jan Kara
2018-11-08 13:01 ` [PATCH 13/16] loop: Move loop_reread_partitions() out of loop_ctl_mutex Jan Kara
2018-11-08 13:01 ` [PATCH 14/16] loop: Fix deadlock when calling blkdev_reread_part() Jan Kara
2018-11-08 13:01 ` [PATCH 15/16] loop: Avoid circular locking dependency between loop_ctl_mutex and bd_mutex Jan Kara
2018-11-08 13:01 ` [PATCH 16/16] loop: Get rid of 'nested' acquisition of loop_ctl_mutex Jan Kara
2018-11-08 13:21 ` [PATCH 0/16 v3] loop: Fix oops and possible deadlocks Jens Axboe
2018-11-08 13:25   ` Jan Kara
2018-11-08 13:31     ` Jens Axboe
2018-11-08 21:28 ` Theodore Y. Ts'o
2018-11-12 10:15   ` Jan Kara

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=20181108130116.12140-8-jack@suse.cz \
    --to=jack@suse.cz \
    --cc=axboe@kernel.dk \
    --cc=linux-block@vger.kernel.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.