Hi Mike, Sonam Mandal, who also works on dm-dedup project, has addressed your comments about BUG() and BUG_ON(). She has also updated the code related to mark and sweep. The changes are staged here (on top of your dm-dedup repo): git://git.fsl.cs.stonybrook.edu/scm/git/linux-dmdedup branch: dm-dedup-devel I'm not sure if you want us to send the patches to device-mapper mailing list in addition to that. Let us know if we should do it. Thanks, Vasily On Mon, Sep 29, 2014 at 9:34 AM, Vasily Tarasov <tarasov@xxxxxxxxxxx> wrote: > Hi Mike, > > Thanks for staging the patches and fixing some issues! > > It totally makes sense to clone your repo and develop on top of it. > That should make things easier both for you and us. > > Let us work through the error paths and fix BUG() and BUG_ON() things > first. We'll try to get some patches ready by the end of the week. > > Thanks, > Vasily > > On Fri, Sep 26, 2014 at 11:24 AM, Mike Snitzer <snitzer@xxxxxxxxxx> wrote: >> Hi Vasily et al, >> >> I've rebased my dm-dedup branch to your v2 patchset. I then fixed >> various issues with the code -- please see the ~7 commits that follow >> your v2 patchset baseline: >> http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=dm-dedup >> >> I will soon transition to actually trying to use dm-dedup and will then >> focus primarily on the design (less on code style nits, etc). >> >> I'll still likely fixup the ERRORs listed below. Of note is the "ERROR: >> application of sizeof to pointer". I noticed that one during my >> code-review too but it still needs fixing. >> >> And BUG() and BUG_ON() are useful for early code development but they >> need to be removed before the code can advance to the next stage >> (e.g. upstream inclusion). >> >> So I would _really_ appreciate it if you could remove most (if not all) >> of the BUG() and BUG_ON() in the code. Please rework the error paths so >> that an error is returned and the error is propagated back to the >> various callers in a graceful (non-destructive way). >> >> Also, rather than posting v3 of the patchset, it'd probably be easiest >> if you just cloned my repo and forked my 'dm-dedup' branch and then >> submitted incremental patches to dm-devel. >> >> Here is a forward of the kernel.org autobuild email we were sent related >> to dm-dedup's excessive use of BUG() AND BUG_ON(), etc: >> >> ----- Forwarded message from Julia Lawall <julia.lawall@xxxxxxx> ----- >> >>> Date: Tue, 23 Sep 2014 13:54:42 +0200 (CEST) >>> From: Julia Lawall <julia.lawall@xxxxxxx> >>> To: kbuild test robot <fengguang.wu@xxxxxxxxx>, tarasov@xxxxxxxxxxx >>> cc: kbuild@xxxxxx, snitzer@xxxxxxxxxx >>> Subject: [snitzer:dm-dedup 12/20] drivers/md/dm-dedup-hash.c:81:3-6: WARNING: Use BUG_ON (fwd) >>> >>> All of the patches look good except for the one about unneeded variable >>> (the last one?). >>> >>> julia >>> >>> ---------- Forwarded message ---------- >>> Date: Tue, 23 Sep 2014 05:27:24 +0800 >>> From: kbuild test robot <fengguang.wu@xxxxxxxxx> >>> To: kbuild@xxxxxx >>> Cc: Julia Lawall <julia.lawall@xxxxxxx> >>> Subject: [snitzer:dm-dedup 12/20] drivers/md/dm-dedup-hash.c:81:3-6: WARNING: >>> Use BUG_ON >>> >>> TO: Vasily Tarasov <tarasov@xxxxxxxxxxx> >>> CC: Mike Snitzer <snitzer@xxxxxxxxxx> >>> >>> Hi Vasily, >>> >>> First bad commit (maybe != root cause): >>> >>> tree: git://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git dm-dedup >>> head: 6d716389dd3b8320da41db4341ee390e226083b2 >>> commit: 266d082b5a0b2f7f2008379f7a31b0a7f2b498b6 [12/20] dm-dedup: Kconfig changes >>> :::::: branch date: 3 hours ago >>> :::::: commit date: 6 hours ago >>> >>> >> drivers/md/dm-dedup-hash.c:81:3-6: WARNING: Use BUG_ON >>> -- >>> >> drivers/md/dm-dedup-rw.c:219:2-5: WARNING: Use BUG_ON >>> -- >>> >> drivers/md/dm-dedup-target.c:784:3-6: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:788:3-6: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:652:2-5: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:658:3-6: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:724:3-6: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:729:2-5: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:292:2-5: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:161:3-6: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:165:3-6: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:169:2-5: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:180:2-5: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:190:2-5: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:194:2-5: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:220:3-6: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:228:3-6: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:234:2-5: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:242:3-6: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:250:3-6: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:254:3-6: WARNING: Use BUG_ON >>> >> drivers/md/dm-dedup-target.c:130:2-5: WARNING: Use BUG_ON >>> -- >>> >> drivers/md/dm-dedup-cbt.c:343:8-10: ERROR: reference preceded by free on line 342 >>> >> drivers/md/dm-dedup-cbt.c:545:27-30: ERROR: reference preceded by free on line 544 >>> >> drivers/md/dm-dedup-cbt.c:738:27-30: ERROR: reference preceded by free on line 737 >>> -- >>> >> drivers/md/dm-dedup-rw.c:168:14-20: ERROR: application of sizeof to pointer >>> -- >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/xmmintrin.h:109:9-12: Unneeded variable: "__Y". Return "__Y" on line 110 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h:1180:9-12: Unneeded variable: "__Y". Return "__Y" on line 1181 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h:1173:10-13: Unneeded variable: "__Y". Return "__Y" on line 1174 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h:1187:10-13: Unneeded variable: "__Y". Return "__Y" on line 1188 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/emmintrin.h:93:10-13: Unneeded variable: "__Y". Return "__Y" on line 94 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/emmintrin.h:743:10-13: Unneeded variable: "__Y". Return "__Y" on line 744 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h:115:9-12: Unneeded variable: "__Y". Return "__Y" on line 116 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h:123:10-13: Unneeded variable: "__Y". Return "__Y" on line 124 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h:131:10-13: Unneeded variable: "__Y". Return "__Y" on line 132 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/xmmintrin.h:109:9-12: Unneeded variable: "__Y". Return "__Y" on line 110 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h:1180:9-12: Unneeded variable: "__Y". Return "__Y" on line 1181 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h:1173:10-13: Unneeded variable: "__Y". Return "__Y" on line 1174 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h:1187:10-13: Unneeded variable: "__Y". Return "__Y" on line 1188 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/emmintrin.h:93:10-13: Unneeded variable: "__Y". Return "__Y" on line 94 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/emmintrin.h:743:10-13: Unneeded variable: "__Y". Return "__Y" on line 744 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h:115:9-12: Unneeded variable: "__Y". Return "__Y" on line 116 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h:123:10-13: Unneeded variable: "__Y". Return "__Y" on line 124 >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h:131:10-13: Unneeded variable: "__Y". Return "__Y" on line 132 >>> >> drivers/md/dm-dedup-target.c:750:5-8: Unneeded variable: "ret". Return "0" on line 761 >>> >>> Please consider folding the attached diff :-) >>> >>> --- >>> 0-DAY kernel build testing backend Open Source Technology Center >>> http://lists.01.org/mailman/listinfo/kbuild Intel Corporation >> >>> From: Fengguang Wu <fengguang.wu@xxxxxxxxx> >>> >>> Subject: [PATCH] dm-dedup: fix coccinelle warnings >>> >>> TO: Mike Snitzer <snitzer@xxxxxxxxxx> >>> >>> CC: Vasily Tarasov <tarasov@xxxxxxxxxxx> >>> >>> CC: linux-raid@xxxxxxxxxxxxxxx (open list:SOFTWARE RAID >>> >>> CC: linux-kernel@xxxxxxxxxxxxxxx >>> >>> >>> >>> drivers/md/dm-dedup-hash.c:81:3-6: WARNING: Use BUG_ON >>> >>> >>> >>> Use BUG_ON instead of a if condition followed by BUG. >>> >>> >>> >>> Semantic patch information: >>> >>> This makes an effort to find cases where BUG() follows an if >>> >>> condition on an expression and replaces the if condition and BUG() >>> >>> with a BUG_ON having the conditional expression of the if statement >>> >>> as argument. >>> >>> >>> >>> Generated by: scripts/coccinelle/misc/bugon.cocci >>> >>> >>> >>> CC: Vasily Tarasov <tarasov@xxxxxxxxxxx> >>> >>> CC: Mike Snitzer <snitzer@xxxxxxxxxx> >>> >>> Signed-off-by: Fengguang Wu <fengguang.wu@xxxxxxxxx> >>> >>> --- >>> >>> >>> >>> Please take the patch only if it's a positive warning. Thanks! >>> >>> >>> >>> dm-dedup-hash.c | 3 +-- >>> >>> 1 file changed, 1 insertion(+), 2 deletions(-) >>> >>> >>> >>> --- a/drivers/md/dm-dedup-hash.c >>> >>> +++ b/drivers/md/dm-dedup-hash.c >>> >>> @@ -77,8 +77,7 @@ static int get_next_slot(struct hash_des >>> >>> int count = 0; >>> >>> >>> >>> do { >>> >>> - if (count == DEDUP_HASH_DESC_COUNT) >>> >>> - BUG(); >>> >>> + BUG_ON(count == DEDUP_HASH_DESC_COUNT); >>> >>> >>> >>> count++; >>> >>> num = atomic_long_inc_return(&(desc_table->slot_counter)); >>> >> >>> From: Fengguang Wu <fengguang.wu@xxxxxxxxx> >>> >>> Subject: [PATCH] dm-dedup: fix coccinelle warnings >>> >>> TO: Mike Snitzer <snitzer@xxxxxxxxxx> >>> >>> CC: Vasily Tarasov <tarasov@xxxxxxxxxxx> >>> >>> CC: linux-raid@xxxxxxxxxxxxxxx (open list:SOFTWARE RAID >>> >>> CC: linux-kernel@xxxxxxxxxxxxxxx >>> >>> >>> >>> drivers/md/dm-dedup-rw.c:219:2-5: WARNING: Use BUG_ON >>> >>> >>> >>> Use BUG_ON instead of a if condition followed by BUG. >>> >>> >>> >>> Semantic patch information: >>> >>> This makes an effort to find cases where BUG() follows an if >>> >>> condition on an expression and replaces the if condition and BUG() >>> >>> with a BUG_ON having the conditional expression of the if statement >>> >>> as argument. >>> >>> >>> >>> Generated by: scripts/coccinelle/misc/bugon.cocci >>> >>> >>> >>> CC: Vasily Tarasov <tarasov@xxxxxxxxxxx> >>> >>> CC: Mike Snitzer <snitzer@xxxxxxxxxx> >>> >>> Signed-off-by: Fengguang Wu <fengguang.wu@xxxxxxxxx> >>> >>> --- >>> >>> >>> >>> Please take the patch only if it's a positive warning. Thanks! >>> >>> >>> >>> dm-dedup-rw.c | 3 +-- >>> >>> 1 file changed, 1 insertion(+), 2 deletions(-) >>> >>> >>> >>> --- a/drivers/md/dm-dedup-rw.c >>> >>> +++ b/drivers/md/dm-dedup-rw.c >>> >>> @@ -215,8 +215,7 @@ static struct bio *prepare_bio_without_p >>> >>> my_zero_fill_bio(clone); >>> >>> >>> >>> r = merge_data(dc, clone->bi_io_vec->bv_page, bio); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> out: >>> >>> return clone; >>> >>> } >>> >> >>> From: Fengguang Wu <fengguang.wu@xxxxxxxxx> >>> >>> Subject: [PATCH] dm-dedup: fix coccinelle warnings >>> >>> TO: Mike Snitzer <snitzer@xxxxxxxxxx> >>> >>> CC: Vasily Tarasov <tarasov@xxxxxxxxxxx> >>> >>> CC: linux-raid@xxxxxxxxxxxxxxx (open list:SOFTWARE RAID >>> >>> CC: linux-kernel@xxxxxxxxxxxxxxx >>> >>> >>> >>> drivers/md/dm-dedup-target.c:784:3-6: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:788:3-6: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:652:2-5: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:658:3-6: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:724:3-6: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:729:2-5: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:292:2-5: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:161:3-6: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:165:3-6: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:169:2-5: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:180:2-5: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:190:2-5: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:194:2-5: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:220:3-6: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:228:3-6: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:234:2-5: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:242:3-6: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:250:3-6: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:254:3-6: WARNING: Use BUG_ON >>> >>> drivers/md/dm-dedup-target.c:130:2-5: WARNING: Use BUG_ON >>> >>> >>> >>> Use BUG_ON instead of a if condition followed by BUG. >>> >>> >>> >>> Semantic patch information: >>> >>> This makes an effort to find cases where BUG() follows an if >>> >>> condition on an expression and replaces the if condition and BUG() >>> >>> with a BUG_ON having the conditional expression of the if statement >>> >>> as argument. >>> >>> >>> >>> Generated by: scripts/coccinelle/misc/bugon.cocci >>> >>> >>> >>> CC: Vasily Tarasov <tarasov@xxxxxxxxxxx> >>> >>> CC: Mike Snitzer <snitzer@xxxxxxxxxx> >>> >>> Signed-off-by: Fengguang Wu <fengguang.wu@xxxxxxxxx> >>> >>> --- >>> >>> >>> >>> Please take the patch only if it's a positive warning. Thanks! >>> >>> >>> >>> dm-dedup-target.c | 60 ++++++++++++++++++------------------------------------ >>> >>> 1 file changed, 20 insertions(+), 40 deletions(-) >>> >>> >>> >>> --- a/drivers/md/dm-dedup-target.c >>> >>> +++ b/drivers/md/dm-dedup-target.c >>> >>> @@ -126,8 +126,7 @@ static int write_to_new_block(struct ded >>> >>> >>> >>> r = dc->kvs_lbn_pbn->kvs_insert(dc->kvs_lbn_pbn, (void *)&lbn, >>> >>> sizeof(lbn), (void *)&lbnpbn_value, sizeof(lbnpbn_value)); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> >>> >>> return r; >>> >>> } >>> >>> @@ -157,16 +156,13 @@ static int handle_write_no_hash(struct d >>> >>> r = dc->kvs_hash_pbn->kvs_insert(dc->kvs_hash_pbn, (void *)hash, >>> >>> dc->crypto_key_size, (void *)&hashpbn_value, >>> >>> sizeof(hashpbn_value)); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> >>> >>> r = dc->mdops->inc_refcount(dc->bmd, pbn_new); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> >>> >>> goto out; >>> >>> - } else if (r < 0) >>> >>> - BUG(); >>> >>> + } else BUG_ON(r < 0); >>> >>> >>> >>> /* LBN->PBN mappings exist */ >>> >>> dc->overwrites++; >>> >>> @@ -176,8 +172,7 @@ static int handle_write_no_hash(struct d >>> >>> >>> >>> pbn_old = lbnpbn_value.pbn; >>> >>> r = dc->mdops->dec_refcount(dc->bmd, pbn_old); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> >>> >>> dc->logical_block_counter--; >>> >>> >>> >>> @@ -186,12 +181,10 @@ static int handle_write_no_hash(struct d >>> >>> r = dc->kvs_hash_pbn->kvs_insert(dc->kvs_hash_pbn, (void *)hash, >>> >>> dc->crypto_key_size, (void *)&hashpbn_value, >>> >>> sizeof(hashpbn_value)); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> >>> >>> r = dc->mdops->inc_refcount(dc->bmd, pbn_new); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> out: >>> >>> return r; >>> >>> } >>> >>> @@ -216,42 +209,36 @@ static int handle_write_with_hash(struct >>> >>> dc->newwrites++; >>> >>> >>> >>> r = dc->mdops->inc_refcount(dc->bmd, pbn_new); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> >>> >>> lbnpbn_value.pbn = pbn_new; >>> >>> >>> >>> r = dc->kvs_lbn_pbn->kvs_insert(dc->kvs_lbn_pbn, (void *)&lbn, >>> >>> sizeof(lbn), (void *)&lbnpbn_value, >>> >>> sizeof(lbnpbn_value)); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> >>> >>> dc->logical_block_counter++; >>> >>> >>> >>> goto out; >>> >>> - } else if (r < 0) >>> >>> - BUG(); >>> >>> + } else BUG_ON(r < 0); >>> >>> >>> >>> /* LBN->PBN mapping entry exists */ >>> >>> dc->overwrites++; >>> >>> pbn_old = lbnpbn_value.pbn; >>> >>> if (pbn_new != pbn_old) { >>> >>> r = dc->mdops->inc_refcount(dc->bmd, pbn_new); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> >>> >>> new_lbnpbn_value.pbn = pbn_new; >>> >>> >>> >>> r = dc->kvs_lbn_pbn->kvs_insert(dc->kvs_lbn_pbn, (void *)&lbn, >>> >>> sizeof(lbn), (void *)&new_lbnpbn_value, >>> >>> sizeof(new_lbnpbn_value)); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> >>> >>> r = dc->mdops->dec_refcount(dc->bmd, pbn_old); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> >>> >>> goto out; >>> >>> } >>> >>> @@ -288,8 +275,7 @@ static void handle_write(struct dedup_co >>> >>> lbn = bio_lbn(dc, bio); >>> >>> >>> >>> r = compute_hash_bio(dc->desc_table, bio, hash); >>> >>> - if (r) >>> >>> - BUG(); >>> >>> + BUG_ON(r); >>> >>> >>> >>> r = dc->kvs_hash_pbn->kvs_lookup(dc->kvs_hash_pbn, hash, >>> >>> dc->crypto_key_size, &hashpbn_value, &vsize); >>> >>> @@ -648,14 +634,12 @@ static int dm_dedup_ctr_fn(struct dm_tar >>> >>> } >>> >>> >>> >>> r = dc->mdops->flush_meta(md); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> >>> >>> if (!unformatted && dc->mdops->get_private_data) { >>> >>> r = dc->mdops->get_private_data(md, (void **)&data, >>> >>> sizeof(struct on_disk_stats)); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> >>> >>> logical_block_counter = data->logical_block_counter; >>> >>> physical_block_counter = data->physical_block_counter; >>> >>> @@ -720,13 +704,11 @@ static void dm_dedup_dtr_fn(struct dm_ta >>> >>> >>> >>> ret = dc->mdops->set_private_data(dc->bmd, &data, >>> >>> sizeof(struct on_disk_stats)); >>> >>> - if (ret < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(ret < 0); >>> >>> } >>> >>> >>> >>> ret = dc->mdops->flush_meta(dc->bmd); >>> >>> - if (ret < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(ret < 0); >>> >>> >>> >>> flush_workqueue(dc->workqueue); >>> >>> destroy_workqueue(dc->workqueue); >>> >>> @@ -780,12 +762,10 @@ static int cleanup_hash_pbn(void *key, i >>> >>> if (test_bit(pbn_val, ms_data->bitmap) == 0) { >>> >>> ret = dc->kvs_hash_pbn->kvs_delete(dc->kvs_hash_pbn, >>> >>> key, ksize); >>> >>> - if (ret < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(ret < 0); >>> >>> >>> >>> r = dc->mdops->dec_refcount(ms_data->dc->bmd, pbn_val); >>> >>> - if (r < 0) >>> >>> - BUG(); >>> >>> + BUG_ON(r < 0); >>> >>> >>> >>> ms_data->cleanup_count++; >>> >>> } >>> >> >>> From: Fengguang Wu <fengguang.wu@xxxxxxxxx> >>> >>> Subject: [PATCH] dm-dedup: fix coccinelle warnings >>> >>> TO: Mike Snitzer <snitzer@xxxxxxxxxx> >>> >>> CC: Vasily Tarasov <tarasov@xxxxxxxxxxx> >>> >>> CC: linux-raid@xxxxxxxxxxxxxxx (open list:SOFTWARE RAID >>> >>> CC: linux-kernel@xxxxxxxxxxxxxxx >>> >>> >>> >>> drivers/md/dm-dedup-rw.c:168:14-20: ERROR: application of sizeof to pointer >>> >>> >>> >>> sizeof when applied to a pointer typed expression gives the size of >>> >>> the pointer >>> >>> >>> >>> Generated by: scripts/coccinelle/misc/noderef.cocci >>> >>> >>> >>> CC: Vasily Tarasov <tarasov@xxxxxxxxxxx> >>> >>> CC: Mike Snitzer <snitzer@xxxxxxxxxx> >>> >>> Signed-off-by: Fengguang Wu <fengguang.wu@xxxxxxxxx> >>> >>> --- >>> >>> >>> >>> dm-dedup-rw.c | 2 +- >>> >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> >>> >>> --- a/drivers/md/dm-dedup-rw.c >>> >>> +++ b/drivers/md/dm-dedup-rw.c >>> >>> @@ -165,7 +165,7 @@ static struct bio *prepare_bio_with_pbn( >>> >>> struct page_list *pl; >>> >>> struct bio *clone = NULL; >>> >>> >>> >>> - pl = kmalloc(sizeof(pl), GFP_NOIO); >>> >>> + pl = kmalloc(sizeof(*pl), GFP_NOIO); >>> >>> if (!pl) >>> >>> goto out; >>> >>> >>> >> >>> From: Fengguang Wu <fengguang.wu@xxxxxxxxx> >>> >>> Subject: [PATCH] dm-dedup: fix coccinelle warnings >>> >>> TO: Mike Snitzer <snitzer@xxxxxxxxxx> >>> >>> CC: Vasily Tarasov <tarasov@xxxxxxxxxxx> >>> >>> CC: linux-raid@xxxxxxxxxxxxxxx (open list:SOFTWARE RAID >>> >>> CC: linux-kernel@xxxxxxxxxxxxxxx >>> >>> >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/xmmintrin.h:109:9-12: Unneeded variable: "__Y". Return "__Y" on line 110 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h:1180:9-12: Unneeded variable: "__Y". Return "__Y" on line 1181 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h:1173:10-13: Unneeded variable: "__Y". Return "__Y" on line 1174 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h:1187:10-13: Unneeded variable: "__Y". Return "__Y" on line 1188 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/emmintrin.h:93:10-13: Unneeded variable: "__Y". Return "__Y" on line 94 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/emmintrin.h:743:10-13: Unneeded variable: "__Y". Return "__Y" on line 744 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h:115:9-12: Unneeded variable: "__Y". Return "__Y" on line 116 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h:123:10-13: Unneeded variable: "__Y". Return "__Y" on line 124 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h:131:10-13: Unneeded variable: "__Y". Return "__Y" on line 132 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/xmmintrin.h:109:9-12: Unneeded variable: "__Y". Return "__Y" on line 110 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h:1180:9-12: Unneeded variable: "__Y". Return "__Y" on line 1181 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h:1173:10-13: Unneeded variable: "__Y". Return "__Y" on line 1174 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h:1187:10-13: Unneeded variable: "__Y". Return "__Y" on line 1188 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/emmintrin.h:93:10-13: Unneeded variable: "__Y". Return "__Y" on line 94 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/emmintrin.h:743:10-13: Unneeded variable: "__Y". Return "__Y" on line 744 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h:115:9-12: Unneeded variable: "__Y". Return "__Y" on line 116 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h:123:10-13: Unneeded variable: "__Y". Return "__Y" on line 124 >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h:131:10-13: Unneeded variable: "__Y". Return "__Y" on line 132 >>> >>> drivers/md/dm-dedup-target.c:750:5-8: Unneeded variable: "ret". Return "0" on line 761 >>> >>> >>> >>> >>> >>> Removes unneeded variable used to store return value. >>> >>> >>> >>> Generated by: scripts/coccinelle/misc/returnvar.cocci >>> >>> >>> >>> CC: Vasily Tarasov <tarasov@xxxxxxxxxxx> >>> >>> CC: Mike Snitzer <snitzer@xxxxxxxxxx> >>> >>> Signed-off-by: Fengguang Wu <fengguang.wu@xxxxxxxxx> >>> >>> --- >>> >>> >>> >>> Please take the patch only if it's a positive warning. Thanks! >>> >>> >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h | 6 ------ >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h | 6 ------ >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/emmintrin.h | 4 ---- >>> >>> /usr/lib/gcc/x86_64-linux-gnu/4.9/include/xmmintrin.h | 2 -- >>> >>> drivers/md/dm-dedup-target.c | 3 +-- >>> >>> 5 files changed, 1 insertion(+), 20 deletions(-) >>> >>> >>> >>> --- /usr/lib/gcc/x86_64-linux-gnu/4.9/include/xmmintrin.h >>> >>> @@ -106,7 +106,6 @@ typedef float __v4sf __attribute__ ((__v >>> >>> extern __inline __m128 __attribute__((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm_undefined_ps (void) >>> >>> { >>> >>> - __m128 __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> --- /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h >>> >>> @@ -1170,21 +1170,18 @@ _mm256_movemask_ps (__m256 __A) >>> >>> extern __inline __m256d __attribute__((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm256_undefined_pd (void) >>> >>> { >>> >>> - __m256d __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> extern __inline __m256 __attribute__((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm256_undefined_ps (void) >>> >>> { >>> >>> - __m256 __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> extern __inline __m256i __attribute__((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm256_undefined_si256 (void) >>> >>> { >>> >>> - __m256i __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> --- /usr/lib/gcc/x86_64-linux-gnu/4.9/include/emmintrin.h >>> >>> @@ -90,7 +90,6 @@ _mm_setr_pd (double __W, double __X) >>> >>> extern __inline __m128d __attribute__((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm_undefined_pd (void) >>> >>> { >>> >>> - __m128d __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> @@ -740,7 +739,6 @@ _mm_move_epi64 (__m128i __A) >>> >>> extern __inline __m128i __attribute__((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm_undefined_si128 (void) >>> >>> { >>> >>> - __m128i __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> --- /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h >>> >>> @@ -112,7 +112,6 @@ extern __inline __m512 >>> >>> __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm512_undefined_ps (void) >>> >>> { >>> >>> - __m512 __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> @@ -120,7 +119,6 @@ extern __inline __m512d >>> >>> __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm512_undefined_pd (void) >>> >>> { >>> >>> - __m512d __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> @@ -128,7 +126,6 @@ extern __inline __m512i >>> >>> __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm512_undefined_si512 (void) >>> >>> { >>> >>> - __m512i __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> --- /usr/lib/gcc/x86_64-linux-gnu/4.9/include/xmmintrin.h >>> >>> @@ -106,7 +106,6 @@ typedef float __v4sf __attribute__ ((__v >>> >>> extern __inline __m128 __attribute__((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm_undefined_ps (void) >>> >>> { >>> >>> - __m128 __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> --- /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avxintrin.h >>> >>> @@ -1170,21 +1170,18 @@ _mm256_movemask_ps (__m256 __A) >>> >>> extern __inline __m256d __attribute__((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm256_undefined_pd (void) >>> >>> { >>> >>> - __m256d __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> extern __inline __m256 __attribute__((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm256_undefined_ps (void) >>> >>> { >>> >>> - __m256 __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> extern __inline __m256i __attribute__((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm256_undefined_si256 (void) >>> >>> { >>> >>> - __m256i __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> --- /usr/lib/gcc/x86_64-linux-gnu/4.9/include/emmintrin.h >>> >>> @@ -90,7 +90,6 @@ _mm_setr_pd (double __W, double __X) >>> >>> extern __inline __m128d __attribute__((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm_undefined_pd (void) >>> >>> { >>> >>> - __m128d __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> @@ -740,7 +739,6 @@ _mm_move_epi64 (__m128i __A) >>> >>> extern __inline __m128i __attribute__((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm_undefined_si128 (void) >>> >>> { >>> >>> - __m128i __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> --- /usr/lib/gcc/x86_64-linux-gnu/4.9/include/avx512fintrin.h >>> >>> @@ -112,7 +112,6 @@ extern __inline __m512 >>> >>> __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm512_undefined_ps (void) >>> >>> { >>> >>> - __m512 __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> @@ -120,7 +119,6 @@ extern __inline __m512d >>> >>> __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm512_undefined_pd (void) >>> >>> { >>> >>> - __m512d __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> @@ -128,7 +126,6 @@ extern __inline __m512i >>> >>> __attribute__ ((__gnu_inline__, __always_inline__, __artificial__)) >>> >>> _mm512_undefined_si512 (void) >>> >>> { >>> >>> - __m512i __Y = __Y; >>> >>> return __Y; >>> >>> } >>> >>> >>> >>> --- a/drivers/md/dm-dedup-target.c >>> >>> +++ b/drivers/md/dm-dedup-target.c >>> >>> @@ -747,7 +747,6 @@ static void dm_dedup_dtr_fn(struct dm_ta >>> >>> static int mark_lbn_pbn_bitmap(void *key, int32_t ksize, >>> >>> void *value, int32_t vsize, void *data) >>> >>> { >>> >>> - int ret = 0; >>> >>> struct mark_and_sweep_data *ms_data = >>> >>> (struct mark_and_sweep_data *)data; >>> >>> uint64_t pbn_val = *((uint64_t *)value); >>> >>> @@ -758,7 +757,7 @@ static int mark_lbn_pbn_bitmap(void *key >>> >>> >>> >>> bitmap_set(ms_data->bitmap, pbn_val, 1); >>> >>> >>> >>> - return ret; >>> >>> + return 0; >>> >>> } >>> >>> >>> >>> static int cleanup_hash_pbn(void *key, int32_t ksize, void *value, >>> >> >> >> ----- End forwarded message ----- >> -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
