aboutsummaryrefslogtreecommitdiffstats
path: root/drivers/block
diff options
context:
space:
mode:
authorSergey Senozhatsky <sergey.senozhatsky@gmail.com>2015-02-12 18:00:39 -0500
committerLinus Torvalds <torvalds@linux-foundation.org>2015-02-12 21:54:11 -0500
commita096cafc31862c54da0b56c8441dc14023437008 (patch)
treee79e5326429f2c91b180091e114b11ec8f2eaf39 /drivers/block
parentba6b17d68c8e3aa8d55d0474299cb931965c5ea5 (diff)
zram: rework reset and destroy path
We need to return set_capacity(disk, 0) from reset_store() back to zram_reset_device(), a catch by Ganesh Mahendran. Potentially, we can race set_capacity() calls from init and reset paths. The problem is that zram_reset_device() is also getting called from zram_exit(), which performs operations in misleading reversed order -- we first create_device() and then init it, while zram_exit() perform destroy_device() first and then does zram_reset_device(). This is done to remove sysfs group before we reset device, so we can continue with device reset/destruction not being raced by sysfs attr write (f.e. disksize). Apart from that, destroy_device() releases zram->disk (but we still have ->disk pointer), so we cannot acces zram->disk in later zram_reset_device() call, which may cause additional errors in the future. So, this patch rework and cleanup destroy path. 1) remove several unneeded goto labels in zram_init() 2) factor out zram_init() error path and zram_exit() into destroy_devices() function, which takes the number of devices to destroy as its argument. 3) remove sysfs group in destroy_devices() first, so we can reorder operations -- reset device (as expected) goes before disk destroy and queue cleanup. So we can always access ->disk in zram_reset_device(). 4) and, finally, return set_capacity() back under ->init_lock. [akpm@linux-foundation.org: tweak comment] Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> Reported-by: Ganesh Mahendran <opensource.ganesh@gmail.com> Cc: Minchan Kim <minchan@kernel.org> Cc: Jerome Marchand <jmarchan@redhat.com> Cc: Nitin Gupta <ngupta@vflare.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'drivers/block')
-rw-r--r--drivers/block/zram/zram_drv.c75
1 files changed, 33 insertions, 42 deletions
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 2607bd9f4955..81ac8fd53340 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -732,8 +732,9 @@ static void zram_reset_device(struct zram *zram)
732 zram->meta = NULL; 732 zram->meta = NULL;
733 /* Reset stats */ 733 /* Reset stats */
734 memset(&zram->stats, 0, sizeof(zram->stats)); 734 memset(&zram->stats, 0, sizeof(zram->stats));
735
736 zram->disksize = 0; 735 zram->disksize = 0;
736 set_capacity(zram->disk, 0);
737
737 up_write(&zram->init_lock); 738 up_write(&zram->init_lock);
738} 739}
739 740
@@ -826,7 +827,6 @@ static ssize_t reset_store(struct device *dev,
826 /* Make sure all pending I/O is finished */ 827 /* Make sure all pending I/O is finished */
827 fsync_bdev(bdev); 828 fsync_bdev(bdev);
828 zram_reset_device(zram); 829 zram_reset_device(zram);
829 set_capacity(zram->disk, 0);
830 830
831 mutex_unlock(&bdev->bd_mutex); 831 mutex_unlock(&bdev->bd_mutex);
832 revalidate_disk(zram->disk); 832 revalidate_disk(zram->disk);
@@ -1112,15 +1112,31 @@ out:
1112 return ret; 1112 return ret;
1113} 1113}
1114 1114
1115static void destroy_device(struct zram *zram) 1115static void destroy_devices(unsigned int nr)
1116{ 1116{
1117 sysfs_remove_group(&disk_to_dev(zram->disk)->kobj, 1117 struct zram *zram;
1118 &zram_disk_attr_group); 1118 unsigned int i;
1119 1119
1120 del_gendisk(zram->disk); 1120 for (i = 0; i < nr; i++) {
1121 put_disk(zram->disk); 1121 zram = &zram_devices[i];
1122 /*
1123 * Remove sysfs first, so no one will perform a disksize
1124 * store while we destroy the devices
1125 */
1126 sysfs_remove_group(&disk_to_dev(zram->disk)->kobj,
1127 &zram_disk_attr_group);
1122 1128
1123 blk_cleanup_queue(zram->queue); 1129 zram_reset_device(zram);
1130
1131 del_gendisk(zram->disk);
1132 put_disk(zram->disk);
1133
1134 blk_cleanup_queue(zram->queue);
1135 }
1136
1137 kfree(zram_devices);
1138 unregister_blkdev(zram_major, "zram");
1139 pr_info("Destroyed %u device(s)\n", nr);
1124} 1140}
1125 1141
1126static int __init zram_init(void) 1142static int __init zram_init(void)
@@ -1130,64 +1146,39 @@ static int __init zram_init(void)
1130 if (num_devices > max_num_devices) { 1146 if (num_devices > max_num_devices) {
1131 pr_warn("Invalid value for num_devices: %u\n", 1147 pr_warn("Invalid value for num_devices: %u\n",
1132 num_devices); 1148 num_devices);
1133 ret = -EINVAL; 1149 return -EINVAL;
1134 goto out;
1135 } 1150 }
1136 1151
1137 zram_major = register_blkdev(0, "zram"); 1152 zram_major = register_blkdev(0, "zram");
1138 if (zram_major <= 0) { 1153 if (zram_major <= 0) {
1139 pr_warn("Unable to get major number\n"); 1154 pr_warn("Unable to get major number\n");
1140 ret = -EBUSY; 1155 return -EBUSY;
1141 goto out;
1142 } 1156 }
1143 1157
1144 /* Allocate the device array and initialize each one */ 1158 /* Allocate the device array and initialize each one */
1145 zram_devices = kzalloc(num_devices * sizeof(struct zram), GFP_KERNEL); 1159 zram_devices = kzalloc(num_devices * sizeof(struct zram), GFP_KERNEL);
1146 if (!zram_devices) { 1160 if (!zram_devices) {
1147 ret = -ENOMEM; 1161 unregister_blkdev(zram_major, "zram");
1148 goto unregister; 1162 return -ENOMEM;
1149 } 1163 }
1150 1164
1151 for (dev_id = 0; dev_id < num_devices; dev_id++) { 1165 for (dev_id = 0; dev_id < num_devices; dev_id++) {
1152 ret = create_device(&zram_devices[dev_id], dev_id); 1166 ret = create_device(&zram_devices[dev_id], dev_id);
1153 if (ret) 1167 if (ret)
1154 goto free_devices; 1168 goto out_error;
1155 } 1169 }
1156 1170
1157 pr_info("Created %u device(s) ...\n", num_devices); 1171 pr_info("Created %u device(s)\n", num_devices);
1158
1159 return 0; 1172 return 0;
1160 1173
1161free_devices: 1174out_error:
1162 while (dev_id) 1175 destroy_devices(dev_id);
1163 destroy_device(&zram_devices[--dev_id]);
1164 kfree(zram_devices);
1165unregister:
1166 unregister_blkdev(zram_major, "zram");
1167out:
1168 return ret; 1176 return ret;
1169} 1177}
1170 1178
1171static void __exit zram_exit(void) 1179static void __exit zram_exit(void)
1172{ 1180{
1173 int i; 1181 destroy_devices(num_devices);
1174 struct zram *zram;
1175
1176 for (i = 0; i < num_devices; i++) {
1177 zram = &zram_devices[i];
1178
1179 destroy_device(zram);
1180 /*
1181 * Shouldn't access zram->disk after destroy_device
1182 * because destroy_device already released zram->disk.
1183 */
1184 zram_reset_device(zram);
1185 }
1186
1187 unregister_blkdev(zram_major, "zram");
1188
1189 kfree(zram_devices);
1190 pr_debug("Cleanup done!\n");
1191} 1182}
1192 1183
1193module_init(zram_init); 1184module_init(zram_init);