Skip to content

Commit

Permalink
zram: close race by open overriding
Browse files Browse the repository at this point in the history
[ Original patch from Minchan Kim <minchan@kernel.org> ]

Commit ba6b17d ("zram: fix umount-reset_store-mount race
condition") introduced bdev->bd_mutex to protect a race between mount
and reset.  At that time, we don't have dynamic zram-add/remove feature
so it was okay.

However, as we introduce dynamic device feature, bd_mutex became
trouble.

	CPU 0

echo 1 > /sys/block/zram<id>/reset
  -> kernfs->s_active(A)
    -> zram:reset_store->bd_mutex(B)

	CPU 1

echo <id> > /sys/class/zram/zram-remove
  ->zram:zram_remove: bd_mutex(B)
  -> sysfs_remove_group
    -> kernfs->s_active(A)

IOW, AB -> BA deadlock

The reason we are holding bd_mutex for zram_remove is to prevent
any incoming open /dev/zram[0-9]. Otherwise, we could remove zram
others already have opened. But it causes above deadlock problem.

To fix the problem, this patch overrides block_device.open and
it returns -EBUSY if zram asserts he claims zram to reset so any
incoming open will be failed so we don't need to hold bd_mutex
for zram_remove ayn more.

This patch is to prepare for zram-add/remove feature.

[sergey.senozhatsky@gmail.com: simplify reset_store()]
Signed-off-by: Minchan Kim <minchan@kernel.org>
Acked-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
sergey-senozhatsky authored and torvalds committed Jun 26, 2015
1 parent 92ff152 commit f405c44
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 19 deletions.
53 changes: 34 additions & 19 deletions drivers/block/zram/zram_drv.c
Original file line number Diff line number Diff line change
Expand Up @@ -1070,45 +1070,60 @@ static ssize_t reset_store(struct device *dev,
struct zram *zram;
struct block_device *bdev;

ret = kstrtou16(buf, 10, &do_reset);
if (ret)
return ret;

if (!do_reset)
return -EINVAL;

zram = dev_to_zram(dev);
bdev = bdget_disk(zram->disk, 0);

if (!bdev)
return -ENOMEM;

mutex_lock(&bdev->bd_mutex);
/* Do not reset an active device! */
if (bdev->bd_openers) {
ret = -EBUSY;
goto out;
/* Do not reset an active device or claimed device */
if (bdev->bd_openers || zram->claim) {
mutex_unlock(&bdev->bd_mutex);
bdput(bdev);
return -EBUSY;
}

ret = kstrtou16(buf, 10, &do_reset);
if (ret)
goto out;

if (!do_reset) {
ret = -EINVAL;
goto out;
}
/* From now on, anyone can't open /dev/zram[0-9] */
zram->claim = true;
mutex_unlock(&bdev->bd_mutex);

/* Make sure all pending I/O is finished */
/* Make sure all the pending I/O are finished */
fsync_bdev(bdev);
zram_reset_device(zram);

mutex_unlock(&bdev->bd_mutex);
revalidate_disk(zram->disk);
bdput(bdev);

mutex_lock(&bdev->bd_mutex);
zram->claim = false;
mutex_unlock(&bdev->bd_mutex);

return len;
}

static int zram_open(struct block_device *bdev, fmode_t mode)
{
int ret = 0;
struct zram *zram;

WARN_ON(!mutex_is_locked(&bdev->bd_mutex));

zram = bdev->bd_disk->private_data;
/* zram was claimed to reset so open request fails */
if (zram->claim)
ret = -EBUSY;

out:
mutex_unlock(&bdev->bd_mutex);
bdput(bdev);
return ret;
}

static const struct block_device_operations zram_devops = {
.open = zram_open,
.swap_slot_free_notify = zram_slot_free_notify,
.rw_page = zram_rw_page,
.owner = THIS_MODULE
Expand Down
4 changes: 4 additions & 0 deletions drivers/block/zram/zram_drv.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,5 +115,9 @@ struct zram {
*/
u64 disksize; /* bytes */
char compressor[10];
/*
* zram is claimed so open request will be failed
*/
bool claim; /* Protected by bdev->bd_mutex */
};
#endif

0 comments on commit f405c44

Please sign in to comment.