diff for duplicates of <50AE08D4.7040602@redhat.com> diff --git a/a/1.txt b/N1/1.txt index 4f2a731..c6efdc3 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -38,3 +38,248 @@ The following (quickly tested) patch should prevent lockdep complain. Jerome --- +>From ebb3514c4ee18276da7c5ca08025991b493ac204 Mon Sep 17 00:00:00 2001 +From: Jerome Marchand <jmarchan@redhat.com> +Date: Thu, 22 Nov 2012 09:07:40 +0100 +Subject: [PATCH] staging: zram: Avoid lockdep warning + +zram triggers a lockdep warning. The cause of it is the call to +zram_init_device() from zram_make_request(). The memory allocation in +zram_init_device() could start a memory reclaim which in turn could +cause swapout and (as it appears to lockdep) a call to +zram_make_request(). However this is a false positive: an +unititialized device can't be used as swap. +A solution is to split init_lock in two lock. One mutex that protects +init, reset and size setting and a rw_semaphore that protects requests +and reset. Thus init and request would be protected by different locks +and lockdep will be happy. + +Signed-off-by: Jerome Marchand <jmarchan@redhat.com> +--- + drivers/staging/zram/zram_drv.c | 41 +++++++++++++++++++----------------- + drivers/staging/zram/zram_drv.h | 16 ++++++++++--- + drivers/staging/zram/zram_sysfs.c | 20 +++++++++--------- + 3 files changed, 44 insertions(+), 33 deletions(-) + +diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c +index fb4a7c9..b3bc3c4 100644 +--- a/drivers/staging/zram/zram_drv.c ++++ b/drivers/staging/zram/zram_drv.c +@@ -470,11 +470,11 @@ static void zram_make_request(struct request_queue *queue, struct bio *bio) + { + struct zram *zram = queue->queuedata; + +- if (unlikely(!zram->init_done) && zram_init_device(zram)) ++ if (unlikely(!is_initialized(zram)) && zram_init_device(zram)) + goto error; + +- down_read(&zram->init_lock); +- if (unlikely(!zram->init_done)) ++ down_read(&zram->req_lock); ++ if (unlikely(!is_initialized(zram))) + goto error_unlock; + + if (!valid_io_request(zram, bio)) { +@@ -483,12 +483,12 @@ static void zram_make_request(struct request_queue *queue, struct bio *bio) + } + + __zram_make_request(zram, bio, bio_data_dir(bio)); +- up_read(&zram->init_lock); ++ up_read(&zram->req_lock); + + return; + + error_unlock: +- up_read(&zram->init_lock); ++ up_read(&zram->req_lock); + error: + bio_io_error(bio); + } +@@ -497,7 +497,7 @@ void __zram_reset_device(struct zram *zram) + { + size_t index; + +- zram->init_done = 0; ++ atomic_set(&zram->init_done, 0); + + /* Free various per-device buffers */ + kfree(zram->compress_workmem); +@@ -529,9 +529,12 @@ void __zram_reset_device(struct zram *zram) + + void zram_reset_device(struct zram *zram) + { +- down_write(&zram->init_lock); +- __zram_reset_device(zram); +- up_write(&zram->init_lock); ++ mutex_lock(&zram->init_lock); ++ down_write(&zram->req_lock); ++ if (is_initialized(zram)) ++ __zram_reset_device(zram); ++ up_write(&zram->req_lock); ++ mutex_unlock(&zram->init_lock); + } + + int zram_init_device(struct zram *zram) +@@ -539,10 +542,10 @@ int zram_init_device(struct zram *zram) + int ret; + size_t num_pages; + +- down_write(&zram->init_lock); ++ mutex_lock(&zram->init_lock); + +- if (zram->init_done) { +- up_write(&zram->init_lock); ++ if (is_initialized(zram)) { ++ mutex_unlock(&zram->init_lock); + return 0; + } + +@@ -583,8 +586,8 @@ int zram_init_device(struct zram *zram) + goto fail; + } + +- zram->init_done = 1; +- up_write(&zram->init_lock); ++ atomic_set(&zram->init_done, 1); ++ mutex_unlock(&zram->init_lock); + + pr_debug("Initialization done!\n"); + return 0; +@@ -594,7 +597,7 @@ fail_no_table: + zram->disksize = 0; + fail: + __zram_reset_device(zram); +- up_write(&zram->init_lock); ++ mutex_unlock(&zram->init_lock); + pr_err("Initialization failed: err=%d\n", ret); + return ret; + } +@@ -619,7 +622,8 @@ static int create_device(struct zram *zram, int device_id) + int ret = 0; + + init_rwsem(&zram->lock); +- init_rwsem(&zram->init_lock); ++ mutex_init(&zram->init_lock); ++ init_rwsem(&zram->req_lock); + spin_lock_init(&zram->stat64_lock); + + zram->queue = blk_alloc_queue(GFP_KERNEL); +@@ -672,7 +676,7 @@ static int create_device(struct zram *zram, int device_id) + goto out; + } + +- zram->init_done = 0; ++ atomic_set(&zram->init_done, 0); + + out: + return ret; +@@ -755,8 +759,7 @@ static void __exit zram_exit(void) + zram = &zram_devices[i]; + + destroy_device(zram); +- if (zram->init_done) +- zram_reset_device(zram); ++ zram_reset_device(zram); + } + + unregister_blkdev(zram_major, "zram"); +diff --git a/drivers/staging/zram/zram_drv.h b/drivers/staging/zram/zram_drv.h +index df2eec4..f6bcead 100644 +--- a/drivers/staging/zram/zram_drv.h ++++ b/drivers/staging/zram/zram_drv.h +@@ -96,9 +96,12 @@ struct zram { + * against concurrent read and writes */ + struct request_queue *queue; + struct gendisk *disk; +- int init_done; +- /* Prevent concurrent execution of device init, reset and R/W request */ +- struct rw_semaphore init_lock; ++ atomic_t init_done; ++ /* Prevent concurrent execution of device init, reset and ++ * disksize_store */ ++ struct mutex init_lock; ++ /* Prevent concurent execution device reset and R/W requests */ ++ struct rw_semaphore req_lock; + /* + * This is the limit on amount of *uncompressed* worth of data + * we can store in a disk. +@@ -108,6 +111,11 @@ struct zram { + struct zram_stats stats; + }; + ++static inline int is_initialized(struct zram *zram) ++{ ++ return atomic_read(&zram->init_done); ++} ++ + extern struct zram *zram_devices; + unsigned int zram_get_num_devices(void); + #ifdef CONFIG_SYSFS +@@ -115,6 +123,6 @@ extern struct attribute_group zram_disk_attr_group; + #endif + + extern int zram_init_device(struct zram *zram); +-extern void __zram_reset_device(struct zram *zram); ++extern void zram_reset_device(struct zram *zram); + + #endif +diff --git a/drivers/staging/zram/zram_sysfs.c b/drivers/staging/zram/zram_sysfs.c +index de1eacf..b300881 100644 +--- a/drivers/staging/zram/zram_sysfs.c ++++ b/drivers/staging/zram/zram_sysfs.c +@@ -62,16 +62,19 @@ static ssize_t disksize_store(struct device *dev, + if (!disksize) + return -EINVAL; + +- down_write(&zram->init_lock); +- if (zram->init_done) { +- up_write(&zram->init_lock); ++ mutex_lock(&zram->init_lock); ++ down_write(&zram->req_lock); ++ if (is_initialized(zram)) { ++ up_write(&zram->req_lock); ++ mutex_unlock(&zram->init_lock); + pr_info("Cannot change disksize for initialized device\n"); + return -EBUSY; + } + + zram->disksize = PAGE_ALIGN(disksize); + set_capacity(zram->disk, zram->disksize >> SECTOR_SHIFT); +- up_write(&zram->init_lock); ++ up_write(&zram->req_lock); ++ mutex_unlock(&zram->init_lock); + + return len; + } +@@ -81,7 +84,7 @@ static ssize_t initstate_show(struct device *dev, + { + struct zram *zram = dev_to_zram(dev); + +- return sprintf(buf, "%u\n", zram->init_done); ++ return sprintf(buf, "%u\n", atomic_read(&zram->init_done)); + } + + static ssize_t reset_store(struct device *dev, +@@ -110,10 +113,7 @@ static ssize_t reset_store(struct device *dev, + if (bdev) + fsync_bdev(bdev); + +- down_write(&zram->init_lock); +- if (zram->init_done) +- __zram_reset_device(zram); +- up_write(&zram->init_lock); ++ zram_reset_device(zram); + + return len; + } +@@ -186,7 +186,7 @@ static ssize_t mem_used_total_show(struct device *dev, + u64 val = 0; + struct zram *zram = dev_to_zram(dev); + +- if (zram->init_done) ++ if (is_initialized(zram)) + val = zs_get_total_size_bytes(zram->mem_pool); + + return sprintf(buf, "%llu\n", val); +-- +1.7.7.6 diff --git a/a/content_digest b/N1/content_digest index d9cadd2..4e1b52a 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -53,6 +53,251 @@ "\n" "Jerome\n" "\n" - --- + "---\n" + ">From ebb3514c4ee18276da7c5ca08025991b493ac204 Mon Sep 17 00:00:00 2001\n" + "From: Jerome Marchand <jmarchan@redhat.com>\n" + "Date: Thu, 22 Nov 2012 09:07:40 +0100\n" + "Subject: [PATCH] staging: zram: Avoid lockdep warning\n" + "\n" + "zram triggers a lockdep warning. The cause of it is the call to\n" + "zram_init_device() from zram_make_request(). The memory allocation in\n" + "zram_init_device() could start a memory reclaim which in turn could\n" + "cause swapout and (as it appears to lockdep) a call to\n" + "zram_make_request(). However this is a false positive: an\n" + "unititialized device can't be used as swap.\n" + "A solution is to split init_lock in two lock. One mutex that protects\n" + "init, reset and size setting and a rw_semaphore that protects requests\n" + "and reset. Thus init and request would be protected by different locks\n" + "and lockdep will be happy.\n" + "\n" + "Signed-off-by: Jerome Marchand <jmarchan@redhat.com>\n" + "---\n" + " drivers/staging/zram/zram_drv.c | 41 +++++++++++++++++++-----------------\n" + " drivers/staging/zram/zram_drv.h | 16 ++++++++++---\n" + " drivers/staging/zram/zram_sysfs.c | 20 +++++++++---------\n" + " 3 files changed, 44 insertions(+), 33 deletions(-)\n" + "\n" + "diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c\n" + "index fb4a7c9..b3bc3c4 100644\n" + "--- a/drivers/staging/zram/zram_drv.c\n" + "+++ b/drivers/staging/zram/zram_drv.c\n" + "@@ -470,11 +470,11 @@ static void zram_make_request(struct request_queue *queue, struct bio *bio)\n" + " {\n" + " \tstruct zram *zram = queue->queuedata;\n" + " \n" + "-\tif (unlikely(!zram->init_done) && zram_init_device(zram))\n" + "+\tif (unlikely(!is_initialized(zram)) && zram_init_device(zram))\n" + " \t\tgoto error;\n" + " \n" + "-\tdown_read(&zram->init_lock);\n" + "-\tif (unlikely(!zram->init_done))\n" + "+\tdown_read(&zram->req_lock);\n" + "+\tif (unlikely(!is_initialized(zram)))\n" + " \t\tgoto error_unlock;\n" + " \n" + " \tif (!valid_io_request(zram, bio)) {\n" + "@@ -483,12 +483,12 @@ static void zram_make_request(struct request_queue *queue, struct bio *bio)\n" + " \t}\n" + " \n" + " \t__zram_make_request(zram, bio, bio_data_dir(bio));\n" + "-\tup_read(&zram->init_lock);\n" + "+\tup_read(&zram->req_lock);\n" + " \n" + " \treturn;\n" + " \n" + " error_unlock:\n" + "-\tup_read(&zram->init_lock);\n" + "+\tup_read(&zram->req_lock);\n" + " error:\n" + " \tbio_io_error(bio);\n" + " }\n" + "@@ -497,7 +497,7 @@ void __zram_reset_device(struct zram *zram)\n" + " {\n" + " \tsize_t index;\n" + " \n" + "-\tzram->init_done = 0;\n" + "+\tatomic_set(&zram->init_done, 0);\n" + " \n" + " \t/* Free various per-device buffers */\n" + " \tkfree(zram->compress_workmem);\n" + "@@ -529,9 +529,12 @@ void __zram_reset_device(struct zram *zram)\n" + " \n" + " void zram_reset_device(struct zram *zram)\n" + " {\n" + "-\tdown_write(&zram->init_lock);\n" + "-\t__zram_reset_device(zram);\n" + "-\tup_write(&zram->init_lock);\n" + "+\tmutex_lock(&zram->init_lock);\n" + "+\tdown_write(&zram->req_lock);\n" + "+\tif (is_initialized(zram))\n" + "+\t\t__zram_reset_device(zram);\n" + "+\tup_write(&zram->req_lock);\n" + "+\tmutex_unlock(&zram->init_lock);\n" + " }\n" + " \n" + " int zram_init_device(struct zram *zram)\n" + "@@ -539,10 +542,10 @@ int zram_init_device(struct zram *zram)\n" + " \tint ret;\n" + " \tsize_t num_pages;\n" + " \n" + "-\tdown_write(&zram->init_lock);\n" + "+\tmutex_lock(&zram->init_lock);\n" + " \n" + "-\tif (zram->init_done) {\n" + "-\t\tup_write(&zram->init_lock);\n" + "+\tif (is_initialized(zram)) {\n" + "+\t\tmutex_unlock(&zram->init_lock);\n" + " \t\treturn 0;\n" + " \t}\n" + " \n" + "@@ -583,8 +586,8 @@ int zram_init_device(struct zram *zram)\n" + " \t\tgoto fail;\n" + " \t}\n" + " \n" + "-\tzram->init_done = 1;\n" + "-\tup_write(&zram->init_lock);\n" + "+\tatomic_set(&zram->init_done, 1);\n" + "+\tmutex_unlock(&zram->init_lock);\n" + " \n" + " \tpr_debug(\"Initialization done!\\n\");\n" + " \treturn 0;\n" + "@@ -594,7 +597,7 @@ fail_no_table:\n" + " \tzram->disksize = 0;\n" + " fail:\n" + " \t__zram_reset_device(zram);\n" + "-\tup_write(&zram->init_lock);\n" + "+\tmutex_unlock(&zram->init_lock);\n" + " \tpr_err(\"Initialization failed: err=%d\\n\", ret);\n" + " \treturn ret;\n" + " }\n" + "@@ -619,7 +622,8 @@ static int create_device(struct zram *zram, int device_id)\n" + " \tint ret = 0;\n" + " \n" + " \tinit_rwsem(&zram->lock);\n" + "-\tinit_rwsem(&zram->init_lock);\n" + "+\tmutex_init(&zram->init_lock);\n" + "+\tinit_rwsem(&zram->req_lock);\n" + " \tspin_lock_init(&zram->stat64_lock);\n" + " \n" + " \tzram->queue = blk_alloc_queue(GFP_KERNEL);\n" + "@@ -672,7 +676,7 @@ static int create_device(struct zram *zram, int device_id)\n" + " \t\tgoto out;\n" + " \t}\n" + " \n" + "-\tzram->init_done = 0;\n" + "+\tatomic_set(&zram->init_done, 0);\n" + " \n" + " out:\n" + " \treturn ret;\n" + "@@ -755,8 +759,7 @@ static void __exit zram_exit(void)\n" + " \t\tzram = &zram_devices[i];\n" + " \n" + " \t\tdestroy_device(zram);\n" + "-\t\tif (zram->init_done)\n" + "-\t\t\tzram_reset_device(zram);\n" + "+\t\tzram_reset_device(zram);\n" + " \t}\n" + " \n" + " \tunregister_blkdev(zram_major, \"zram\");\n" + "diff --git a/drivers/staging/zram/zram_drv.h b/drivers/staging/zram/zram_drv.h\n" + "index df2eec4..f6bcead 100644\n" + "--- a/drivers/staging/zram/zram_drv.h\n" + "+++ b/drivers/staging/zram/zram_drv.h\n" + "@@ -96,9 +96,12 @@ struct zram {\n" + " \t\t\t\t * against concurrent read and writes */\n" + " \tstruct request_queue *queue;\n" + " \tstruct gendisk *disk;\n" + "-\tint init_done;\n" + "-\t/* Prevent concurrent execution of device init, reset and R/W request */\n" + "-\tstruct rw_semaphore init_lock;\n" + "+\tatomic_t init_done;\n" + "+\t/* Prevent concurrent execution of device init, reset and\n" + "+\t * disksize_store */\n" + "+\tstruct mutex init_lock;\n" + "+\t/* Prevent concurent execution device reset and R/W requests */\n" + "+\tstruct rw_semaphore req_lock;\n" + " \t/*\n" + " \t * This is the limit on amount of *uncompressed* worth of data\n" + " \t * we can store in a disk.\n" + "@@ -108,6 +111,11 @@ struct zram {\n" + " \tstruct zram_stats stats;\n" + " };\n" + " \n" + "+static inline int is_initialized(struct zram *zram)\n" + "+{\n" + "+\treturn atomic_read(&zram->init_done);\n" + "+}\n" + "+\n" + " extern struct zram *zram_devices;\n" + " unsigned int zram_get_num_devices(void);\n" + " #ifdef CONFIG_SYSFS\n" + "@@ -115,6 +123,6 @@ extern struct attribute_group zram_disk_attr_group;\n" + " #endif\n" + " \n" + " extern int zram_init_device(struct zram *zram);\n" + "-extern void __zram_reset_device(struct zram *zram);\n" + "+extern void zram_reset_device(struct zram *zram);\n" + " \n" + " #endif\n" + "diff --git a/drivers/staging/zram/zram_sysfs.c b/drivers/staging/zram/zram_sysfs.c\n" + "index de1eacf..b300881 100644\n" + "--- a/drivers/staging/zram/zram_sysfs.c\n" + "+++ b/drivers/staging/zram/zram_sysfs.c\n" + "@@ -62,16 +62,19 @@ static ssize_t disksize_store(struct device *dev,\n" + " \tif (!disksize)\n" + " \t\treturn -EINVAL;\n" + " \n" + "-\tdown_write(&zram->init_lock);\n" + "-\tif (zram->init_done) {\n" + "-\t\tup_write(&zram->init_lock);\n" + "+\tmutex_lock(&zram->init_lock);\n" + "+\tdown_write(&zram->req_lock);\n" + "+\tif (is_initialized(zram)) {\n" + "+\t\tup_write(&zram->req_lock);\n" + "+\t\tmutex_unlock(&zram->init_lock);\n" + " \t\tpr_info(\"Cannot change disksize for initialized device\\n\");\n" + " \t\treturn -EBUSY;\n" + " \t}\n" + " \n" + " \tzram->disksize = PAGE_ALIGN(disksize);\n" + " \tset_capacity(zram->disk, zram->disksize >> SECTOR_SHIFT);\n" + "-\tup_write(&zram->init_lock);\n" + "+\tup_write(&zram->req_lock);\n" + "+\tmutex_unlock(&zram->init_lock);\n" + " \n" + " \treturn len;\n" + " }\n" + "@@ -81,7 +84,7 @@ static ssize_t initstate_show(struct device *dev,\n" + " {\n" + " \tstruct zram *zram = dev_to_zram(dev);\n" + " \n" + "-\treturn sprintf(buf, \"%u\\n\", zram->init_done);\n" + "+\treturn sprintf(buf, \"%u\\n\", atomic_read(&zram->init_done));\n" + " }\n" + " \n" + " static ssize_t reset_store(struct device *dev,\n" + "@@ -110,10 +113,7 @@ static ssize_t reset_store(struct device *dev,\n" + " \tif (bdev)\n" + " \t\tfsync_bdev(bdev);\n" + " \n" + "-\tdown_write(&zram->init_lock);\n" + "-\tif (zram->init_done)\n" + "-\t\t__zram_reset_device(zram);\n" + "-\tup_write(&zram->init_lock);\n" + "+\tzram_reset_device(zram);\n" + " \n" + " \treturn len;\n" + " }\n" + "@@ -186,7 +186,7 @@ static ssize_t mem_used_total_show(struct device *dev,\n" + " \tu64 val = 0;\n" + " \tstruct zram *zram = dev_to_zram(dev);\n" + " \n" + "-\tif (zram->init_done)\n" + "+\tif (is_initialized(zram))\n" + " \t\tval = zs_get_total_size_bytes(zram->mem_pool);\n" + " \n" + " \treturn sprintf(buf, \"%llu\\n\", val);\n" + "-- \n" + 1.7.7.6 -cf498dfa91ea107607ef491e25373cef774a675eff9c84fa98cb85791100aed0 +8cc6b3f3c7f2877797fcae0a89ff2edda4da134d07a9778108fd381f7ec178ed
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.