From 5e2e1cc4131cf4d21629c94331f2351b7dc8b87c Mon Sep 17 00:00:00 2001 From: Luis Chamberlain Date: Fri, 15 Oct 2021 16:52:14 -0700 Subject: zram: add error handling support for add_disk() We never checked for errors on add_disk() as this function returned void. Now that this is fixed, use the shiny new error handling. Signed-off-by: Luis Chamberlain Acked-by: Minchan Kim Link: https://lore.kernel.org/r/20211015235219.2191207-9-mcgrof@kernel.org Signed-off-by: Jens Axboe --- drivers/block/zram/zram_drv.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/block/zram') diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index a68297fb51a2..876bf191e2ca 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -1949,7 +1949,9 @@ static int zram_add(void) blk_queue_max_write_zeroes_sectors(zram->disk->queue, UINT_MAX); blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, zram->disk->queue); - device_add_disk(NULL, zram->disk, zram_disk_attr_groups); + ret = device_add_disk(NULL, zram->disk, zram_disk_attr_groups); + if (ret) + goto out_cleanup_disk; strlcpy(zram->compressor, default_compressor, sizeof(zram->compressor)); @@ -1957,6 +1959,8 @@ static int zram_add(void) pr_info("Added device: %s\n", zram->disk->disk_name); return device_id; +out_cleanup_disk: + blk_cleanup_disk(zram->disk); out_free_idr: idr_remove(&zram_index_idr, device_id); out_free_dev: -- cgit v1.2.3 From 6f1637795f2827d36aec9e0246487f5852e8abf7 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Mon, 25 Oct 2021 10:54:23 +0800 Subject: zram: fix race between zram_reset_device() and disksize_store() When the ->init_lock is released in zram_reset_device(), disksize_store() can come in and try to allocate meta, but zram_reset_device() is freeing free meta, so cause races. Link: https://lore.kernel.org/linux-block/20210927163805.808907-1-mcgrof@kernel.org/T/#mc617f865a3fa2778e40f317ddf48f6447c20c073 Reported-by: Luis Chamberlain Reviewed-by: Luis Chamberlain Signed-off-by: Ming Lei Acked-by: Minchan Kim Link: https://lore.kernel.org/r/20211025025426.2815424-2-ming.lei@redhat.com Signed-off-by: Jens Axboe --- drivers/block/zram/zram_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/block/zram') diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 876bf191e2ca..9b38e5f749c6 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -1704,12 +1704,13 @@ static void zram_reset_device(struct zram *zram) set_capacity_and_notify(zram->disk, 0); part_stat_set_all(zram->disk->part0, 0); - up_write(&zram->init_lock); /* I/O operation under all of CPU are done so let's free */ zram_meta_free(zram, disksize); memset(&zram->stats, 0, sizeof(zram->stats)); zcomp_destroy(comp); reset_bdev(zram); + + up_write(&zram->init_lock); } static ssize_t disksize_store(struct device *dev, -- cgit v1.2.3 From 8c54499a59b026a3dc2afccf6e1b36d5700d2fef Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Mon, 25 Oct 2021 10:54:24 +0800 Subject: zram: don't fail to remove zram during unloading module When the zram module is being unloaded, no one should be using the zram disks. However even while being unloaded the zram module's sysfs attributes might be poked at to re-configure zram devices. This is expected, and kernfs ensures that these operations complete before device_del() completes. But reset_store() may set ->claim which will fail zram_remove(), when this happens, zram_reset_device() is bypassed, and zram->comp can't be destroyed, so the warning of 'Error: Removing state 63 which has instances left.' is triggered during unloading module, together with memory leak and sort of thing. Fixes the issue by not failing zram_remove() if ->claim is set, and we actually need to do nothing in case that zram_reset() is running since del_gendisk() will wait until zram_reset() is done. Reported-by: Luis Chamberlain Reviewed-by: Luis Chamberlain Signed-off-by: Ming Lei Acked-by: Minchan Kim Link: https://lore.kernel.org/r/20211025025426.2815424-3-ming.lei@redhat.com Signed-off-by: Jens Axboe --- drivers/block/zram/zram_drv.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) (limited to 'drivers/block/zram') diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 9b38e5f749c6..13b65ebbab8d 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -1972,25 +1972,40 @@ out_free_dev: static int zram_remove(struct zram *zram) { struct block_device *bdev = zram->disk->part0; + bool claimed; mutex_lock(&bdev->bd_disk->open_mutex); - if (bdev->bd_openers || zram->claim) { + if (bdev->bd_openers) { mutex_unlock(&bdev->bd_disk->open_mutex); return -EBUSY; } - zram->claim = true; + claimed = zram->claim; + if (!claimed) + zram->claim = true; mutex_unlock(&bdev->bd_disk->open_mutex); zram_debugfs_unregister(zram); - /* Make sure all the pending I/O are finished */ - fsync_bdev(bdev); - zram_reset_device(zram); + if (claimed) { + /* + * If we were claimed by reset_store(), del_gendisk() will + * wait until reset_store() is done, so nothing need to do. + */ + ; + } else { + /* Make sure all the pending I/O are finished */ + fsync_bdev(bdev); + zram_reset_device(zram); + } pr_info("Removed device: %s\n", zram->disk->disk_name); del_gendisk(zram->disk); + + /* del_gendisk drains pending reset_store */ + WARN_ON_ONCE(claimed && zram->claim); + blk_cleanup_disk(zram->disk); kfree(zram); return 0; @@ -2067,7 +2082,7 @@ static struct class zram_control_class = { static int zram_remove_cb(int id, void *ptr, void *data) { - zram_remove(ptr); + WARN_ON_ONCE(zram_remove(ptr)); return 0; } -- cgit v1.2.3 From 5a4b653655d554b5f51a5d2252882708c56a6f7e Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Mon, 25 Oct 2021 10:54:25 +0800 Subject: zram: avoid race between zram_remove and disksize_store After resetting device in zram_remove(), disksize_store still may come and allocate resources again before deleting gendisk, fix the race by resetting zram after del_gendisk() returns. At that time, disksize_store can't come any more. Reported-by: Luis Chamberlain Reviewed-by: Luis Chamberlain Signed-off-by: Ming Lei Acked-by: Minchan Kim Link: https://lore.kernel.org/r/20211025025426.2815424-4-ming.lei@redhat.com Signed-off-by: Jens Axboe --- drivers/block/zram/zram_drv.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/block/zram') diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 13b65ebbab8d..2dfa3a396c7c 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -2006,6 +2006,13 @@ static int zram_remove(struct zram *zram) /* del_gendisk drains pending reset_store */ WARN_ON_ONCE(claimed && zram->claim); + /* + * disksize_store() may be called in between zram_reset_device() + * and del_gendisk(), so run the last reset to avoid leaking + * anything allocated with disksize_store() + */ + zram_reset_device(zram); + blk_cleanup_disk(zram->disk); kfree(zram); return 0; -- cgit v1.2.3 From 00c5495c54f785beb0f6a34f7a3674d3ea0997d5 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Mon, 25 Oct 2021 10:54:26 +0800 Subject: zram: replace fsync_bdev with sync_blockdev When calling fsync_bdev(), zram driver guarantees that the bdev won't be opened by anyone, then there can't be one active fs/superblock over the zram bdev, so replace fsync_bdev with sync_blockdev. Reviewed-by: Luis Chamberlain Signed-off-by: Ming Lei Acked-by: Minchan Kim Link: https://lore.kernel.org/r/20211025025426.2815424-5-ming.lei@redhat.com Signed-off-by: Jens Axboe --- drivers/block/zram/zram_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/block/zram') diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index 2dfa3a396c7c..edc6bd640559 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -1790,7 +1790,7 @@ static ssize_t reset_store(struct device *dev, mutex_unlock(&bdev->bd_disk->open_mutex); /* Make sure all the pending I/O are finished */ - fsync_bdev(bdev); + sync_blockdev(bdev); zram_reset_device(zram); mutex_lock(&bdev->bd_disk->open_mutex); @@ -1995,7 +1995,7 @@ static int zram_remove(struct zram *zram) ; } else { /* Make sure all the pending I/O are finished */ - fsync_bdev(bdev); + sync_blockdev(bdev); zram_reset_device(zram); } -- cgit v1.2.3