* [PATCH 0/2] loop: don't hang on lo_ctl_mutex in ioctls
@ 2018-03-27 4:39 Omar Sandoval
2018-03-27 4:39 ` [PATCH v2 1/2] loop: don't call into filesystem while holding lo_ctl_mutex Omar Sandoval
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Omar Sandoval @ 2018-03-27 4:39 UTC (permalink / raw)
To: linux-block; +Cc: Jens Axboe, kernel-team, linux-fsdevel
From: Omar Sandoval <osandov@fb.com>
Hi, Jens,
We hit an issue where a loop device on NFS (yes, I know) got stuck and a
bunch of losetup processes got stuck in uninterruptible sleep waiting
for lo_ctl_mutex as a result. Calling into the filesystem while holding
lo_ctl_mutex isn't necessary, and there's no reason to be
uninterruptible here. These two patches fix this issue, based on your
for-next branch.
Changed from v1: use mutex_lock_killable() instead of
mutex_lock_interruptible().
Thanks!
Omar Sandoval (2):
loop: don't call into filesystem while holding lo_ctl_mutex
loop: use killable lock in ioctls
drivers/block/loop.c | 65 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 42 insertions(+), 23 deletions(-)
--
2.16.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] loop: don't call into filesystem while holding lo_ctl_mutex
2018-03-27 4:39 [PATCH 0/2] loop: don't hang on lo_ctl_mutex in ioctls Omar Sandoval
@ 2018-03-27 4:39 ` Omar Sandoval
2018-03-27 4:39 ` [PATCH v2 2/2] loop: use killable lock in ioctls Omar Sandoval
2018-03-27 20:21 ` [PATCH 0/2] loop: don't hang on lo_ctl_mutex " Jens Axboe
2 siblings, 0 replies; 5+ messages in thread
From: Omar Sandoval @ 2018-03-27 4:39 UTC (permalink / raw)
To: linux-block; +Cc: Jens Axboe, kernel-team, linux-fsdevel
From: Omar Sandoval <osandov@fb.com>
We hit an issue where a loop device on NFS was stuck in
loop_get_status() doing vfs_getattr() after the NFS server died, which
caused a pile-up of uninterruptible processes waiting on lo_ctl_mutex.
There's no reason to hold this lock while we wait on the filesystem;
let's drop it so that other processes can do their thing. We need to
grab a reference on lo_backing_file while we use it, and we can get rid
of the check on lo_device, which has been unnecessary since commit
a34c0ae9ebd6 ("[PATCH] loop: remove the bio remapping capability") in
the linux-history tree.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
drivers/block/loop.c | 38 ++++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index ed6fafdc5377..93a60bda7608 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1167,21 +1167,17 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
static int
loop_get_status(struct loop_device *lo, struct loop_info64 *info)
{
- struct file *file = lo->lo_backing_file;
+ struct file *file;
struct kstat stat;
- int error;
+ int ret;
- if (lo->lo_state != Lo_bound)
+ if (lo->lo_state != Lo_bound) {
+ mutex_unlock(&lo->lo_ctl_mutex);
return -ENXIO;
- error = vfs_getattr(&file->f_path, &stat,
- STATX_INO, AT_STATX_SYNC_AS_STAT);
- if (error)
- return error;
+ }
+
memset(info, 0, sizeof(*info));
info->lo_number = lo->lo_number;
- info->lo_device = huge_encode_dev(stat.dev);
- info->lo_inode = stat.ino;
- info->lo_rdevice = huge_encode_dev(lo->lo_device ? stat.rdev : stat.dev);
info->lo_offset = lo->lo_offset;
info->lo_sizelimit = lo->lo_sizelimit;
info->lo_flags = lo->lo_flags;
@@ -1194,7 +1190,19 @@ loop_get_status(struct loop_device *lo, struct loop_info64 *info)
memcpy(info->lo_encrypt_key, lo->lo_encrypt_key,
lo->lo_encrypt_key_size);
}
- return 0;
+
+ /* Drop lo_ctl_mutex while we call into the filesystem. */
+ file = get_file(lo->lo_backing_file);
+ mutex_unlock(&lo->lo_ctl_mutex);
+ ret = vfs_getattr(&file->f_path, &stat, STATX_INO,
+ AT_STATX_SYNC_AS_STAT);
+ if (!ret) {
+ info->lo_device = huge_encode_dev(stat.dev);
+ info->lo_inode = stat.ino;
+ info->lo_rdevice = huge_encode_dev(stat.rdev);
+ }
+ fput(file);
+ return ret;
}
static void
@@ -1374,7 +1382,8 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
break;
case LOOP_GET_STATUS:
err = loop_get_status_old(lo, (struct loop_info __user *) arg);
- break;
+ /* loop_get_status() unlocks lo_ctl_mutex */
+ goto out_unlocked;
case LOOP_SET_STATUS64:
err = -EPERM;
if ((mode & FMODE_WRITE) || capable(CAP_SYS_ADMIN))
@@ -1383,7 +1392,8 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
break;
case LOOP_GET_STATUS64:
err = loop_get_status64(lo, (struct loop_info64 __user *) arg);
- break;
+ /* loop_get_status() unlocks lo_ctl_mutex */
+ goto out_unlocked;
case LOOP_SET_CAPACITY:
err = -EPERM;
if ((mode & FMODE_WRITE) || capable(CAP_SYS_ADMIN))
@@ -1544,7 +1554,7 @@ static int lo_compat_ioctl(struct block_device *bdev, fmode_t mode,
mutex_lock(&lo->lo_ctl_mutex);
err = loop_get_status_compat(
lo, (struct compat_loop_info __user *) arg);
- mutex_unlock(&lo->lo_ctl_mutex);
+ /* loop_get_status() unlocks lo_ctl_mutex */
break;
case LOOP_SET_CAPACITY:
case LOOP_CLR_FD:
--
2.16.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] loop: use killable lock in ioctls
2018-03-27 4:39 [PATCH 0/2] loop: don't hang on lo_ctl_mutex in ioctls Omar Sandoval
2018-03-27 4:39 ` [PATCH v2 1/2] loop: don't call into filesystem while holding lo_ctl_mutex Omar Sandoval
@ 2018-03-27 4:39 ` Omar Sandoval
2018-03-27 20:21 ` [PATCH 0/2] loop: don't hang on lo_ctl_mutex " Jens Axboe
2 siblings, 0 replies; 5+ messages in thread
From: Omar Sandoval @ 2018-03-27 4:39 UTC (permalink / raw)
To: linux-block; +Cc: Jens Axboe, kernel-team, linux-fsdevel
From: Omar Sandoval <osandov@fb.com>
Even after the previous patch to drop lo_ctl_mutex while calling
vfs_getattr(), there are other cases where we can end up sleeping for a
long time while holding lo_ctl_mutex. Let's avoid the uninterruptible
sleep from the ioctls.
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
drivers/block/loop.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 93a60bda7608..264abaaff662 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1360,7 +1360,10 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
struct loop_device *lo = bdev->bd_disk->private_data;
int err;
- mutex_lock_nested(&lo->lo_ctl_mutex, 1);
+ err = mutex_lock_killable_nested(&lo->lo_ctl_mutex, 1);
+ if (err)
+ goto out_unlocked;
+
switch (cmd) {
case LOOP_SET_FD:
err = loop_set_fd(lo, mode, bdev, arg);
@@ -1545,16 +1548,20 @@ static int lo_compat_ioctl(struct block_device *bdev, fmode_t mode,
switch(cmd) {
case LOOP_SET_STATUS:
- mutex_lock(&lo->lo_ctl_mutex);
- err = loop_set_status_compat(
- lo, (const struct compat_loop_info __user *) arg);
- mutex_unlock(&lo->lo_ctl_mutex);
+ err = mutex_lock_killable(&lo->lo_ctl_mutex);
+ if (!err) {
+ err = loop_set_status_compat(lo,
+ (const struct compat_loop_info __user *)arg);
+ mutex_unlock(&lo->lo_ctl_mutex);
+ }
break;
case LOOP_GET_STATUS:
- mutex_lock(&lo->lo_ctl_mutex);
- err = loop_get_status_compat(
- lo, (struct compat_loop_info __user *) arg);
- /* loop_get_status() unlocks lo_ctl_mutex */
+ err = mutex_lock_killable(&lo->lo_ctl_mutex);
+ if (!err) {
+ err = loop_get_status_compat(lo,
+ (struct compat_loop_info __user *)arg);
+ /* loop_get_status() unlocks lo_ctl_mutex */
+ }
break;
case LOOP_SET_CAPACITY:
case LOOP_CLR_FD:
@@ -1959,7 +1966,9 @@ static long loop_control_ioctl(struct file *file, unsigned int cmd,
ret = loop_lookup(&lo, parm);
if (ret < 0)
break;
- mutex_lock(&lo->lo_ctl_mutex);
+ ret = mutex_lock_killable(&lo->lo_ctl_mutex);
+ if (ret)
+ break;
if (lo->lo_state != Lo_unbound) {
ret = -EBUSY;
mutex_unlock(&lo->lo_ctl_mutex);
--
2.16.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] loop: don't hang on lo_ctl_mutex in ioctls
2018-03-27 4:39 [PATCH 0/2] loop: don't hang on lo_ctl_mutex in ioctls Omar Sandoval
2018-03-27 4:39 ` [PATCH v2 1/2] loop: don't call into filesystem while holding lo_ctl_mutex Omar Sandoval
2018-03-27 4:39 ` [PATCH v2 2/2] loop: use killable lock in ioctls Omar Sandoval
@ 2018-03-27 20:21 ` Jens Axboe
2 siblings, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2018-03-27 20:21 UTC (permalink / raw)
To: Omar Sandoval, linux-block; +Cc: kernel-team, linux-fsdevel
On 3/26/18 10:39 PM, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> Hi, Jens,
>
> We hit an issue where a loop device on NFS (yes, I know) got stuck and a
> bunch of losetup processes got stuck in uninterruptible sleep waiting
> for lo_ctl_mutex as a result. Calling into the filesystem while holding
> lo_ctl_mutex isn't necessary, and there's no reason to be
> uninterruptible here. These two patches fix this issue, based on your
> for-next branch.
Applied for 4.17, thanks Omar.
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 0/2] loop: don't hang on lo_ctl_mutex in ioctls
@ 2018-03-26 23:16 Omar Sandoval
0 siblings, 0 replies; 5+ messages in thread
From: Omar Sandoval @ 2018-03-26 23:16 UTC (permalink / raw)
To: linux-block; +Cc: Jens Axboe, kernel-team, linux-fsdevel
From: Omar Sandoval <osandov@fb.com>
Hi, Jens,
We hit an issue where a loop device on NFS (yes, I know) got stuck and a
bunch of losetup processes got stuck in uninterruptible sleep waiting
for lo_ctl_mutex as a result. Calling into the filesystem while holding
lo_ctl_mutex isn't necessary, and there's no reason to be
uninterruptible here. These two patches fix this issue, based on your
for-next branch.
Thanks!
Omar Sandoval (2):
loop: don't call into filesystem while holding lo_ctl_mutex
loop: use interruptible lock in ioctls
drivers/block/loop.c | 65 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 42 insertions(+), 23 deletions(-)
--
2.16.2
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-03-27 20:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-27 4:39 [PATCH 0/2] loop: don't hang on lo_ctl_mutex in ioctls Omar Sandoval
2018-03-27 4:39 ` [PATCH v2 1/2] loop: don't call into filesystem while holding lo_ctl_mutex Omar Sandoval
2018-03-27 4:39 ` [PATCH v2 2/2] loop: use killable lock in ioctls Omar Sandoval
2018-03-27 20:21 ` [PATCH 0/2] loop: don't hang on lo_ctl_mutex " Jens Axboe
-- strict thread matches above, loose matches on Subject: below --
2018-03-26 23:16 Omar Sandoval
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).