Re: [RFC PATCH 3/3] coccinelle: api: add offset_in_page.cocci

2018-12-06 Thread Julia Lawall 



On Thu, 6 Dec 2018, Johannes Thumshirn wrote:

> On 05/12/2018 15:46, Julia Lawall wrote:
> [...]
> >> +@r_patch depends on !context && patch && !org && !report@
> >> +expression E;
> >> +type T;
> >> +@@
> >> +
> >> +(
> >> +- E & ~PAGE_MASK
> >> ++ offset_in_page(E)
> >> +|
> >> +- E & (PAGE_SIZE - 1)
> >> ++ offset_in_page(E)
> >
> > The two lines above should be subsumed by the two lines below.  When there
> > is a type metavariable that has no other dependencies, an isomorphism will
> > consider that it is either present or absent.
>
> Oh OK, I'm sorry I'm not really into cocinelle so I guessed it might
> take some iterations.
>
> Do you have an example for this?

Expanation 1:

Coccinelle as a file standard.iso that shows the isomorphisms (rewrite
rules) that may be applied to semantic patches.  One of the rules is:

Expression
@ not_ptr1 @
expression *X;
@@
 !X => X == NULL

So if you have a pointer typed expression X and you write a transformation
on !X, it will also apply to occurrences of X == NULL in the source code.
In this way, you don't have to write so many variants.

Likewise there is an isomorphism:

Expression
@ drop_cast @
expression E;
pure type T;
@@

 (T)E => E

That is, if you have a semantic patch with (T)X, then it will also apply
to code that matches just X, without the cast.  The word pure means that
this isomorphism metavariable has to match a semantic patch term that is a
metavariable and this metavariable can't be used elsewhere.  If you wrote

- (char)x

Then you would probably not want that to apply without the (char) cast.
But if you have just

- (T)x

for some randome unbound metavariable T, then perhaps you don't case about
the cast to T.  If you actually do, then you can put disable drop_cast in
the header of your rule.



Explanation 2:

To see what your semantic patch is really doing, you can run

spatch --parse-cocci sp.cocci

Here is what I get for your patch rule, with some annotations added:

@@
expression E;
type T;
@@


(
-E
  >>> offset_in_page(E)
 -& -~-PAGE_MASK
|
-~
  >>> offset_in_page(E)
-PAGE_MASK -& -E
|

// the following come from
// - E & (PAGE_SIZE - 1)
// + offset_in_page(E)

-E   // 1
  >>> offset_in_page(E)
 -& -(-PAGE_SIZE -- -1-)
|
-E   // 2
  >>> offset_in_page(E)
 -& -PAGE_SIZE -- -1
|
-(   // 3
>>> offset_in_page(E)
  -PAGE_SIZE -- -1-) -& -E
|
-PAGE_SIZE   // 4
  >>> offset_in_page(E)
 -- -1 -& -E
|

// the following come from:
// - E & ((T)PAGE_SIZE - 1)
// + offset_in_page(E)

-E
  >>> offset_in_page(E)
 -& -(-(-T -)-PAGE_SIZE -- -1-)
|
-E   // same as 1
  >>> offset_in_page(E)
 -& -(-PAGE_SIZE -- -1-)
|
-E
  >>> offset_in_page(E)
 -& -(-T -)-PAGE_SIZE -- -1
|
-E   // same as 2
  >>> offset_in_page(E)
 -& -PAGE_SIZE -- -1
|
-(
>>> offset_in_page(E)
  -(-T -)-PAGE_SIZE -- -1-) -& -E
|
-(   // same as 3
>>> offset_in_page(E)
  -PAGE_SIZE -- -1-) -& -E
|
-(
>>> offset_in_page(E)
  -T -)-PAGE_SIZE -- -1 -& -E
|
-PAGE_SIZE   // same as 4
  >>> offset_in_page(E)
 -- -1 -& -E
)

So all the transformation generated by

- E & (PAGE_SIZE - 1)
+ offset_in_page(E)

are also generated by

- E & ((T)PAGE_SIZE - 1)
+ offset_in_page(E)

I hope that is helpful.

julia

Re: [RFC PATCH 3/3] coccinelle: api: add offset_in_page.cocci

2018-12-05 Thread Julia Lawall 



On Wed, 5 Dec 2018, Johannes Thumshirn wrote:

> Constructs like 'var & (PAGE_SIZE - 1)' or 'var & ~PAGE_MASK' can be
> replaced by the offset_in_page() macro instead of open-coding it.
>
> Add a coccinelle semantic patch to ease detection and conversion of these.
>
> This unfortunately doesn't account for the case when we want PAGE_ALIGNED()
> instead of offset_in_page() yet.
>
> Cc: Julia Lawall 
> Signed-off-by: Johannes Thumshirn 
> ---
>  scripts/coccinelle/api/offset_in_page.cocci | 81 
> +
>  1 file changed, 81 insertions(+)
>  create mode 100644 scripts/coccinelle/api/offset_in_page.cocci
>
> diff --git a/scripts/coccinelle/api/offset_in_page.cocci 
> b/scripts/coccinelle/api/offset_in_page.cocci
> new file mode 100644
> index ..ea5b3a8e0390
> --- /dev/null
> +++ b/scripts/coccinelle/api/offset_in_page.cocci
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0
> +///
> +/// Use offset_in_page macro on address instead of explicit computation.
> +///
> +//  Confidence: High
> +//  Keywords: offset_in_page
> +//  Comment: Based on vma_pages.cocci
> +
> +virtual context
> +virtual patch
> +virtual org
> +virtual report
> +
> +
> +//--
> +//  For context mode
> +//--
> +
> +@r_context depends on context && !patch && !org && !report@
> +expression E;
> +@@
> +
> +(
> +* E & ~PAGE_MASK
> +|
> +* E & (PAGE_SIZE - 1)
> +)
> +
> +
> +//--
> +//  For patch mode
> +//--
> +
> +@r_patch depends on !context && patch && !org && !report@
> +expression E;
> +type T;
> +@@
> +
> +(
> +- E & ~PAGE_MASK
> ++ offset_in_page(E)
> +|
> +- E & (PAGE_SIZE - 1)
> ++ offset_in_page(E)

The two lines above should be subsumed by the two lines below.  When there
is a type metavariable that has no other dependencies, an isomorphism will
consider that it is either present or absent.

Why not include the cast case for the context and org cases?

Masahiro will ultimately commit this.  I have added him to CC.

Thanks for the contribution.

julia


> +|
> +- E & ((T)PAGE_SIZE - 1)
> ++ offset_in_page(E)
> +)
> +
> +//--
> +//  For org mode
> +//--
> +
> +@r_org depends on !context && !patch && (org || report)@
> +expression E;
> +position p;
> +@@
> +
> +  (
> +  * E@p & ~PAGE_MASK
> +  |
> +  * E@p & (PAGE_SIZE - 1)
> +  )
> +
> +@script:python depends on report@
> +p << r_org.p;
> +x << r_org.E;
> +@@
> +
> +msg="WARNING: Consider using offset_in_page helper on %s" % (x)
> +coccilib.report.print_report(p[0], msg)
> +
> +@script:python depends on org@
> +p << r_org.p;
> +x << r_org.E;
> +@@
> +
> +msg="WARNING: Consider using offset_in_page helper on %s" % (x)
> +msg_safe=msg.replace("[","@(").replace("]",")")
> +coccilib.org.print_todo(p[0], msg_safe)
> +
> --
> 2.16.4
>
>

[josef-btrfs:btrfs-readdir 4/5] fs/btrfs/ref-verify.c:322:12-14: ERROR: reference preceded by free on line 321 (fwd)

2017-08-31 Thread Julia Lawall 
It does not look correct to access >node on line 322 after freeing be
on line 321.

julia

-- Forwarded message --
Date: Fri, 1 Sep 2017 07:41:26 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: [josef-btrfs:btrfs-readdir 4/5] fs/btrfs/ref-verify.c:322:12-14: ERROR:
 reference preceded by free on line 321

CC: kbuild-...@01.org
CC: linux-btrfs@vger.kernel.org
TO: Josef Bacik <jba...@fb.com>

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/josef/btrfs-next.git 
btrfs-readdir
head:   707c82dec33ba199538c4b20885873c32c0c4259
commit: b5bfaee4fb7028e8ac44e1feab4d99b917b876d7 [4/5] Btrfs: add a extent ref 
verify tool
:: branch date: 5 hours ago
:: commit date: 8 hours ago

>> fs/btrfs/ref-verify.c:322:12-14: ERROR: reference preceded by free on line 
>> 321

# 
https://git.kernel.org/pub/scm/linux/kernel/git/josef/btrfs-next.git/commit/?id=b5bfaee4fb7028e8ac44e1feab4d99b917b876d7
git remote add josef-btrfs 
https://git.kernel.org/pub/scm/linux/kernel/git/josef/btrfs-next.git
git remote update josef-btrfs
git checkout b5bfaee4fb7028e8ac44e1feab4d99b917b876d7
vim +322 fs/btrfs/ref-verify.c

b5bfaee4 Josef Bacik 2014-05-09  284
b5bfaee4 Josef Bacik 2014-05-09  285  static struct block_entry 
*add_block_entry(struct btrfs_root *root, u64 bytenr,
b5bfaee4 Josef Bacik 2014-05-09  286   u64 
len, u64 root_objectid)
b5bfaee4 Josef Bacik 2014-05-09  287  {
b5bfaee4 Josef Bacik 2014-05-09  288struct btrfs_fs_info *fs_info = 
root->fs_info;
b5bfaee4 Josef Bacik 2014-05-09  289struct block_entry *be = NULL, *exist;
b5bfaee4 Josef Bacik 2014-05-09  290struct root_entry *re = NULL;
b5bfaee4 Josef Bacik 2014-05-09  291
b5bfaee4 Josef Bacik 2014-05-09  292re = kmalloc(sizeof(struct root_entry), 
GFP_NOFS);
b5bfaee4 Josef Bacik 2014-05-09  293be = kmalloc(sizeof(struct 
block_entry), GFP_NOFS);
b5bfaee4 Josef Bacik 2014-05-09  294if (!be || !re) {
b5bfaee4 Josef Bacik 2014-05-09  295kfree(re);
b5bfaee4 Josef Bacik 2014-05-09  296kfree(be);
b5bfaee4 Josef Bacik 2014-05-09  297return ERR_PTR(-ENOMEM);
b5bfaee4 Josef Bacik 2014-05-09  298}
b5bfaee4 Josef Bacik 2014-05-09  299be->bytenr = bytenr;
b5bfaee4 Josef Bacik 2014-05-09  300be->len = len;
b5bfaee4 Josef Bacik 2014-05-09  301
b5bfaee4 Josef Bacik 2014-05-09  302re->root_objectid = root_objectid;
b5bfaee4 Josef Bacik 2014-05-09  303re->num_refs = 0;
b5bfaee4 Josef Bacik 2014-05-09  304
b5bfaee4 Josef Bacik 2014-05-09  305spin_lock(_info->ref_verify_lock);
b5bfaee4 Josef Bacik 2014-05-09  306exist = 
insert_block_entry(_info->block_tree, be);
b5bfaee4 Josef Bacik 2014-05-09  307if (exist) {
b5bfaee4 Josef Bacik 2014-05-09  308update_block_entry(root, exist, 
re);
b5bfaee4 Josef Bacik 2014-05-09  309kfree(be);
b5bfaee4 Josef Bacik 2014-05-09  310be = exist;
b5bfaee4 Josef Bacik 2014-05-09  311goto out;
b5bfaee4 Josef Bacik 2014-05-09  312}
b5bfaee4 Josef Bacik 2014-05-09  313
b5bfaee4 Josef Bacik 2014-05-09  314be->num_refs = 1;
b5bfaee4 Josef Bacik 2014-05-09  315be->metadata = 0;
b5bfaee4 Josef Bacik 2014-05-09  316be->roots = RB_ROOT;
b5bfaee4 Josef Bacik 2014-05-09  317be->refs = RB_ROOT;
b5bfaee4 Josef Bacik 2014-05-09  318INIT_LIST_HEAD(>actions);
b5bfaee4 Josef Bacik 2014-05-09  319if (insert_root_entry(>roots, re)) {
b5bfaee4 Josef Bacik 2014-05-09  320kfree(re);
b5bfaee4 Josef Bacik 2014-05-09 @321kfree(be);
b5bfaee4 Josef Bacik 2014-05-09 @322rb_erase(>node, 
_info->block_tree);
b5bfaee4 Josef Bacik 2014-05-09  323be = ERR_PTR(-EINVAL);
b5bfaee4 Josef Bacik 2014-05-09  324ASSERT(0);
b5bfaee4 Josef Bacik 2014-05-09  325}
b5bfaee4 Josef Bacik 2014-05-09  326  out:
b5bfaee4 Josef Bacik 2014-05-09  327spin_unlock(_info->ref_verify_lock);
b5bfaee4 Josef Bacik 2014-05-09  328return be;
b5bfaee4 Josef Bacik 2014-05-09  329  }
b5bfaee4 Josef Bacik 2014-05-09  330

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] btrfs: qgroup: Fix qgroup corruption caused by inode_cache mount option (fwd)

2016-11-28 Thread Julia Lawall 
It looks like the tree_log_mutex needs to be unlocked at lines 2153 and
2159.

julia

-- Forwarded message --
Date: Tue, 29 Nov 2016 14:16:30 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH 1/3] btrfs: qgroup: Fix qgroup corruption caused by
inode_cache mount option

Hi Qu,

[auto build test WARNING on next-20161128]
[also build test WARNING on v4.9-rc7]
[cannot apply to btrfs/next v4.9-rc7 v4.9-rc6 v4.9-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-qgroup-Fix-qgroup-corruption-caused-by-inode_cache-mount-option/20161129-133729
:: branch date: 39 minutes ago
:: commit date: 39 minutes ago

>> fs/btrfs/transaction.c:2307:1-7: preceding lock on line 2126

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout a10797b831381f092c853c616f2eddd856ace99d
vim +2307 fs/btrfs/transaction.c

e02119d5a Chris Mason2008-09-05  2120* At this point in the commit, 
there can't be any tree-log
e02119d5a Chris Mason2008-09-05  2121* writers, but a little lower 
down we drop the trans mutex
e02119d5a Chris Mason2008-09-05  2122* and let new people in.  By 
holding the tree_log_mutex
e02119d5a Chris Mason2008-09-05  2123* from now until after the 
super is written, we avoid races
e02119d5a Chris Mason2008-09-05  2124* with the tree-log code.
e02119d5a Chris Mason2008-09-05  2125*/
e02119d5a Chris Mason2008-09-05 @2126   
mutex_lock(>fs_info->tree_log_mutex);
1a40e23b9 Zheng Yan  2008-09-26  2127
5d4f98a28 Yan Zheng  2009-06-10  2128   ret = commit_fs_roots(trans, 
root);
49b25e054 Jeff Mahoney   2012-03-01  2129   if (ret) {
49b25e054 Jeff Mahoney   2012-03-01  2130   
mutex_unlock(>fs_info->tree_log_mutex);
871383be5 David Sterba   2012-04-02  2131   
mutex_unlock(>fs_info->reloc_mutex);
6cf7f77e6 Wang Shilong   2014-02-19  2132   goto scrub_continue;
49b25e054 Jeff Mahoney   2012-03-01  2133   }
54aa1f4df Chris Mason2007-06-22  2134
3818aea27 Qu Wenruo  2014-01-13  2135   /*
7e1876aca David Sterba   2014-02-05  2136* Since the transaction is 
done, we can apply the pending changes
7e1876aca David Sterba   2014-02-05  2137* before the next transaction.
3818aea27 Qu Wenruo  2014-01-13  2138*/
572d9ab78 David Sterba   2014-02-05  2139   
btrfs_apply_pending_changes(root->fs_info);
3818aea27 Qu Wenruo  2014-01-13  2140
5d4f98a28 Yan Zheng  2009-06-10  2141   /* commit_fs_roots gets rid of 
all the tree log roots, it is now
e02119d5a Chris Mason2008-09-05  2142* safe to free the root of 
tree log roots
e02119d5a Chris Mason2008-09-05  2143*/
e02119d5a Chris Mason2008-09-05  2144   btrfs_free_log_root_tree(trans, 
root->fs_info);
e02119d5a Chris Mason2008-09-05  2145
0ed4792af Qu Wenruo  2015-04-16  2146   /*
a10797b83 Qu Wenruo  2016-11-29  2147* commit_fs_roots() can call 
btrfs_save_ino_cache(), which generates
a10797b83 Qu Wenruo  2016-11-29  2148* new delayed refs. Must 
handle them or qgroup can be wrong.
a10797b83 Qu Wenruo  2016-11-29  2149*/
a10797b83 Qu Wenruo  2016-11-29  2150   ret = 
btrfs_run_delayed_refs(trans, root, (unsigned long)-1);
a10797b83 Qu Wenruo  2016-11-29  2151   if (ret) {
a10797b83 Qu Wenruo  2016-11-29  2152   
mutex_unlock(>fs_info->reloc_mutex);
a10797b83 Qu Wenruo  2016-11-29  2153   goto scrub_continue;
a10797b83 Qu Wenruo  2016-11-29  2154   }
a10797b83 Qu Wenruo  2016-11-29  2155
a10797b83 Qu Wenruo  2016-11-29  2156   ret = 
btrfs_qgroup_prepare_account_extents(trans, root->fs_info);
a10797b83 Qu Wenruo  2016-11-29  2157   if (ret) {
a10797b83 Qu Wenruo  2016-11-29  2158   
mutex_unlock(>fs_info->reloc_mutex);
a10797b83 Qu Wenruo  2016-11-29  2159   goto scrub_continue;
a10797b83 Qu Wenruo  2016-11-29  2160   }
a10797b83 Qu Wenruo  2016-11-29  2161
a10797b83 Qu Wenruo  2016-11-29  2162   /*
0ed4792af Qu Wenruo  2015-04-16  2163* Since fs roots are all 
committed, we can get a quite accurate
0ed4792af Qu Wenruo  2015-04-16  2164* new_roots. So let's do quota 
accounting.
0ed4792af Qu Wenruo  2015-04-16  2165*/
0ed4792af Qu Wenruo  2015-04-16  2166   ret = 
btrfs_qgroup_account_extents(trans, root->fs_info);
0ed4792af Qu Wenruo  2015-04-16  2167   if (ret < 0) {
0ed4792af Qu Wenruo  2015-04-16  2168   
mutex_unlock(>fs_info->tree_log_mutex);
0ed4792af Qu Wenruo  2015-04-16

[PATCH 00/39] drop null test before destroy functions

2015-09-13 Thread Julia Lawall 
Recent commits to kernel/git/torvalds/linux.git have made the following
functions able to tolerate NULL arguments:

kmem_cache_destroy (commit 3942d29918522)
mempool_destroy (commit 4e3ca3e033d1)
dma_pool_destroy (commit 44d7175da6ea)

These patches remove the associated NULL tests for the files that I found
easy to compile test.  If these changes are OK, I will address the
remainder later.

---

 arch/x86/kvm/mmu.c |6 --
 block/bio-integrity.c  |7 --
 block/bio.c|7 --
 block/blk-core.c   |3 -
 block/elevator.c   |3 -
 drivers/atm/he.c   |7 --
 drivers/block/aoe/aoedev.c |3 -
 drivers/block/drbd/drbd_main.c |   21 ++-
 drivers/block/pktcdvd.c|3 -
 drivers/block/rbd.c|6 --
 drivers/dma/dmaengine.c|6 --
 drivers/firmware/google/gsmi.c |3 -
 drivers/gpu/drm/i915/i915_dma.c|   19 ++
 drivers/iommu/amd_iommu_init.c |7 --
 drivers/md/bcache/bset.c   |3 -
 drivers/md/bcache/request.c|3 -
 drivers/md/bcache/super.c  |9 +--
 drivers/md/dm-bufio.c  |3 -
 drivers/md/dm-cache-target.c   |3 -
 drivers/md/dm-crypt.c  |6 --
 drivers/md/dm-io.c |3 -
 drivers/md/dm-log-userspace-base.c |3 -
 drivers/md/dm-region-hash.c|4 -
 drivers/md/dm.c|   13 +---
 drivers/md/multipath.c |3 -
 drivers/md/raid1.c |6 --
 drivers/md/raid10.c|9 +--
 drivers/md/raid5.c |3 -
 drivers/mtd/nand/nandsim.c |3 -
 drivers/mtd/ubi/attach.c   |4 -
 drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c  |3 -
 drivers/staging/lustre/lustre/llite/super25.c  |   16 +
 drivers/staging/lustre/lustre/obdclass/genops.c|   24 ++--
 drivers/staging/lustre/lustre/obdclass/lu_object.c |6 --
 drivers/staging/rdma/hfi1/user_sdma.c  |3 -
 drivers/thunderbolt/ctl.c  |3 -
 drivers/usb/gadget/udc/bdc/bdc_core.c  |3 -
 drivers/usb/gadget/udc/gr_udc.c|3 -
 drivers/usb/gadget/udc/mv_u3d_core.c   |3 -
 drivers/usb/gadget/udc/mv_udc_core.c   |3 -
 drivers/usb/host/fotg210-hcd.c |   12 +---
 drivers/usb/host/fusbh200-hcd.c|   12 +---
 drivers/usb/host/whci/init.c   |3 -
 drivers/usb/host/xhci-mem.c|   12 +---
 fs/btrfs/backref.c |3 -
 fs/btrfs/delayed-inode.c   |3 -
 fs/btrfs/delayed-ref.c |   12 +---
 fs/btrfs/disk-io.c |3 -
 fs/btrfs/extent_io.c   |6 --
 fs/btrfs/extent_map.c  |3 -
 fs/btrfs/file.c|3 -
 fs/btrfs/inode.c   |   18 ++
 fs/btrfs/ordered-data.c|3 -
 fs/dlm/memory.c|6 --
 fs/ecryptfs/main.c |3 -
 fs/ext4/crypto.c   |9 +--
 fs/ext4/extents_status.c   |3 -
 fs/ext4/mballoc.c  |3 -
 fs/f2fs/crypto.c   |9 +--
 fs/gfs2/main.c |   29 ++
 fs/jbd2/journal.c  |   15 +
 fs/jbd2/revoke.c   |   12 +---
 fs/jbd2/transaction.c  |6 --
 fs/jffs2/malloc.c  |   27 +++--
 fs/nfsd/nfscache.c |6 --
 fs/nilfs2/super.c  |   12 +---
 fs/ocfs2/dlm/dlmlock.c |3 -
 fs/ocfs2/dlm/dlmmaster.c   |   16 +
 fs/ocfs2/super.c   |   18 ++
 fs/ocfs2/uptodate.c|3 -
 lib/debugobjects.c |3 -
 net/core/sock.c|   12 +---
 net/dccp/ackvec.c  |   12 +---
 net/dccp/ccid.c

[PATCH 06/39] Btrfs: drop null test before destroy functions

2015-09-13 Thread Julia Lawall 
Remove unneeded NULL test.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
@@ expression x; @@
-if (x != NULL)
  \(kmem_cache_destroy\|mempool_destroy\|dma_pool_destroy\)(x);
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 fs/btrfs/backref.c   |3 +--
 fs/btrfs/delayed-inode.c |3 +--
 fs/btrfs/delayed-ref.c   |   12 
 fs/btrfs/disk-io.c   |3 +--
 fs/btrfs/extent_io.c |6 ++
 fs/btrfs/extent_map.c|3 +--
 fs/btrfs/file.c  |3 +--
 fs/btrfs/inode.c |   18 ++
 fs/btrfs/ordered-data.c  |3 +--
 9 files changed, 18 insertions(+), 36 deletions(-)

diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index 6a98bdd..6416c11 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -20,8 +20,7 @@ int __init extent_map_init(void)
 
 void extent_map_exit(void)
 {
-   if (extent_map_cache)
-   kmem_cache_destroy(extent_map_cache);
+   kmem_cache_destroy(extent_map_cache);
 }
 
 /**
diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index ac3e81d..54ae98d 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -787,14 +787,10 @@ btrfs_find_delayed_ref_head(struct btrfs_trans_handle 
*trans, u64 bytenr)
 
 void btrfs_delayed_ref_exit(void)
 {
-   if (btrfs_delayed_ref_head_cachep)
-   kmem_cache_destroy(btrfs_delayed_ref_head_cachep);
-   if (btrfs_delayed_tree_ref_cachep)
-   kmem_cache_destroy(btrfs_delayed_tree_ref_cachep);
-   if (btrfs_delayed_data_ref_cachep)
-   kmem_cache_destroy(btrfs_delayed_data_ref_cachep);
-   if (btrfs_delayed_extent_op_cachep)
-   kmem_cache_destroy(btrfs_delayed_extent_op_cachep);
+   kmem_cache_destroy(btrfs_delayed_ref_head_cachep);
+   kmem_cache_destroy(btrfs_delayed_tree_ref_cachep);
+   kmem_cache_destroy(btrfs_delayed_data_ref_cachep);
+   kmem_cache_destroy(btrfs_delayed_extent_op_cachep);
 }
 
 int btrfs_delayed_ref_init(void)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 0d98aee..6cc044d 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -103,8 +103,7 @@ int __init btrfs_end_io_wq_init(void)
 
 void btrfs_end_io_wq_exit(void)
 {
-   if (btrfs_end_io_wq_cache)
-   kmem_cache_destroy(btrfs_end_io_wq_cache);
+   kmem_cache_destroy(btrfs_end_io_wq_cache);
 }
 
 /*
diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index a2ae427..1fb6ce3 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -43,8 +43,7 @@ int __init btrfs_delayed_inode_init(void)
 
 void btrfs_delayed_inode_exit(void)
 {
-   if (delayed_node_cache)
-   kmem_cache_destroy(delayed_node_cache);
+   kmem_cache_destroy(delayed_node_cache);
 }
 
 static inline void btrfs_init_delayed_node(
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index b823fac..c8d0210 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -2820,8 +2820,7 @@ const struct file_operations btrfs_file_operations = {
 
 void btrfs_auto_defrag_exit(void)
 {
-   if (btrfs_inode_defrag_cachep)
-   kmem_cache_destroy(btrfs_inode_defrag_cachep);
+   kmem_cache_destroy(btrfs_inode_defrag_cachep);
 }
 
 int btrfs_auto_defrag_init(void)
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 52170cf..2e5a1e1 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -1072,6 +1072,5 @@ int __init ordered_data_init(void)
 
 void ordered_data_exit(void)
 {
-   if (btrfs_ordered_extent_cache)
-   kmem_cache_destroy(btrfs_ordered_extent_cache);
+   kmem_cache_destroy(btrfs_ordered_extent_cache);
 }
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index a0fa725..c7904d3 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9031,18 +9031,12 @@ void btrfs_destroy_cachep(void)
 * destroy cache.
 */
rcu_barrier();
-   if (btrfs_inode_cachep)
-   kmem_cache_destroy(btrfs_inode_cachep);
-   if (btrfs_trans_handle_cachep)
-   kmem_cache_destroy(btrfs_trans_handle_cachep);
-   if (btrfs_transaction_cachep)
-   kmem_cache_destroy(btrfs_transaction_cachep);
-   if (btrfs_path_cachep)
-   kmem_cache_destroy(btrfs_path_cachep);
-   if (btrfs_free_space_cachep)
-   kmem_cache_destroy(btrfs_free_space_cachep);
-   if (btrfs_delalloc_work_cachep)
-   kmem_cache_destroy(btrfs_delalloc_work_cachep);
+   kmem_cache_destroy(btrfs_inode_cachep);
+   kmem_cache_destroy(btrfs_trans_handle_cachep);
+   kmem_cache_destroy(btrfs_transaction_cachep);
+   kmem_cache_destroy(btrfs_path_cachep);
+   kmem_cache_destroy(btrfs_free_space_cachep);
+   kmem_cache_destroy(btrfs_delalloc_work_cachep);
 }
 
 int btrfs_init_cachep(void)
diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index ecbc63d..6763b1f 100644
--

Re: [btrfs:integration-4.2 29/57] fs/btrfs/disk-io.c:2325:17-30: ERROR: reference preceded by free on line 2324

2015-06-11 Thread Julia Lawall 
This does not look correct.  Please check.

julia

On Thu, 11 Jun 2015, kbuild test robot wrote:

 TO: Liu Bo bo.li@oracle.com
 CC: Chris Mason chris.ma...@fusionio.com Chris Mason c...@fb.com
 CC: David Sterba dste...@suse.cz
 CC: linux-btrfs@vger.kernel.org
 
 tree:   git://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs.git 
 integration-4.2
 head:   9a4e7276d39071576d369e607d7accb84b41d0b4
 commit: 64c043de466d5746e7ca306dab9d418cd871cefc [29/57] Btrfs: fix up 
 read_tree_block to return proper error
 :: branch date: 7 hours ago
 :: commit date: 8 days ago
 
  fs/btrfs/disk-io.c:2325:17-30: ERROR: reference preceded by free on line 
  2324
 
 git remote add btrfs 
 git://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs.git
 git remote update btrfs
 git checkout 64c043de466d5746e7ca306dab9d418cd871cefc
 vim +2325 fs/btrfs/disk-io.c
 
 63443bf5 Eric Sandeen 2014-08-01  2318
 BTRFS_TREE_LOG_OBJECTID);
 63443bf5 Eric Sandeen 2014-08-01  2319  
 63443bf5 Eric Sandeen 2014-08-01  2320log_tree_root-node = 
 read_tree_block(tree_root, bytenr,
 63443bf5 Eric Sandeen 2014-08-01  2321
 fs_info-generation + 1);
 64c043de Liu Bo   2015-05-25  2322if 
 (IS_ERR(log_tree_root-node)) {
 64c043de Liu Bo   2015-05-25  2323printk(KERN_ERR BTRFS: 
 failed to read log tree\n);
 64c043de Liu Bo   2015-05-25 @2324kfree(log_tree_root);
 64c043de Liu Bo   2015-05-25 @2325return 
 PTR_ERR(log_tree_root-node);
 64c043de Liu Bo   2015-05-25  2326} else if 
 (!extent_buffer_uptodate(log_tree_root-node)) {
 63443bf5 Eric Sandeen 2014-08-01  2327printk(KERN_ERR BTRFS: 
 failed to read log tree\n);
 63443bf5 Eric Sandeen 2014-08-01  2328
 free_extent_buffer(log_tree_root-node);
 
 ---
 0-DAY kernel test infrastructureOpen Source Technology Center
 http://lists.01.org/mailman/listinfo/kbuild Intel Corporation
 
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] fs: btrfs: free-space-cache.c: remove two unnecessary checks before calling kfree()

2015-02-11 Thread Julia Lawall 


On Wed, 11 Feb 2015, Bas Peters wrote:

 kfree checks whether the pointer it is passed is NULL. The two foregoing
 checks are therefore unnecessary.

 This issue was detected using Coccinelle.

 Signed-off-by: Bas Peters baspeter...@gmail.com
 ---
  fs/btrfs/free-space-cache.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

 diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
 index d6c03f7..7d2d817 100644
 --- a/fs/btrfs/free-space-cache.c
 +++ b/fs/btrfs/free-space-cache.c
 @@ -1976,8 +1976,7 @@ new_bitmap:

  out:
   if (info) {
 - if (info-bitmap)
 - kfree(info-bitmap);
 + kfree(info-bitmap);
   kmem_cache_free(btrfs_free_space_cachep, info);
   }

 @@ -3427,8 +3426,7 @@ again:

   if (info)
   kmem_cache_free(btrfs_free_space_cachep, info);
 - if (map)
 - kfree(map);
 + kfree(map);

A certain lack of parallelism arises in the latter case.

julia

   return 0;
  }

 --
 2.1.0

 --
 To unsubscribe from this list: send the line unsubscribe kernel-janitors in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Cocci] [PATCH 1/1] btrfs: Deletion of unnecessary checks before six function calls

2014-10-31 Thread Julia Lawall 


On Fri, 31 Oct 2014, SF Markus Elfring wrote:

 The following functions test whether their argument is NULL and then
 return immediately.
 * btrfs_free_path()
 * free_extent_buffer()
 * free_extent_map()
 * free_extent_state()
 * iput()
 * kfree()
 
 Thus the test around the call is not needed.
 
 This issue was detected by using the Coccinelle software.
 
 Signed-off-by: Markus Elfring elfr...@users.sourceforge.net
 ---
  fs/btrfs/dev-replace.c   |  3 +--
  fs/btrfs/extent_io.c | 12 
  fs/btrfs/file.c  |  6 ++
  fs/btrfs/free-space-cache.c  |  7 +++
  fs/btrfs/inode.c |  6 ++
  fs/btrfs/reada.c |  3 +--
  fs/btrfs/relocation.c|  3 +--
  fs/btrfs/tests/btrfs-tests.c |  3 +--
  fs/btrfs/tree-defrag.c   |  3 +--
  fs/btrfs/tree-log.c  |  6 ++
  10 files changed, 18 insertions(+), 34 deletions(-)
 
 diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
 index 6f662b3..3465029 100644
 --- a/fs/btrfs/dev-replace.c
 +++ b/fs/btrfs/dev-replace.c
 @@ -183,8 +183,7 @@ no_valid_dev_replace_entry_found:
   }
 
  out:
 - if (path)
 - btrfs_free_path(path);
 + btrfs_free_path(path);

It appears to be statically apparent whether btrfs_free_path is needed or 
not.  The code could be changed both not to have the test and not to have 
the jump and call to btrfs_free_path.

This is probably the case for the other occurrences next to labels.

julia

   return ret;
  }
 
 diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
 index bf3f424..cfbf00a 100644
 --- a/fs/btrfs/extent_io.c
 +++ b/fs/btrfs/extent_io.c
 @@ -704,8 +704,7 @@ next:
 
  out:
   spin_unlock(tree-lock);
 - if (prealloc)
 - free_extent_state(prealloc);
 + free_extent_state(prealloc);
 
   return 0;
 
 @@ -1006,8 +1005,7 @@ hit_next:
 
  out:
   spin_unlock(tree-lock);
 - if (prealloc)
 - free_extent_state(prealloc);
 + free_extent_state(prealloc);
 
   return err;
 
 @@ -1223,8 +1221,7 @@ hit_next:
 
  out:
   spin_unlock(tree-lock);
 - if (prealloc)
 - free_extent_state(prealloc);
 + free_extent_state(prealloc);
 
   return err;
 
 @@ -4146,8 +4143,7 @@ int extent_readpages(struct extent_io_tree *tree,
   __extent_readpages(tree, pagepool, nr, get_extent, em_cached,
  bio, 0, bio_flags, READ);
 
 - if (em_cached)
 - free_extent_map(em_cached);
 + free_extent_map(em_cached);
 
   BUG_ON(!list_empty(pages));
   if (bio)
 diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
 index a18ceab..add07ce8 100644
 --- a/fs/btrfs/file.c
 +++ b/fs/btrfs/file.c
 @@ -677,10 +677,8 @@ next:
   /* once for the tree*/
   free_extent_map(em);
   }
 - if (split)
 - free_extent_map(split);
 - if (split2)
 - free_extent_map(split2);
 + free_extent_map(split);
 + free_extent_map(split2);
  }
 
  /*
 diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
 index 3384819..11883e2 100644
 --- a/fs/btrfs/free-space-cache.c
 +++ b/fs/btrfs/free-space-cache.c
 @@ -1943,8 +1943,7 @@ new_bitmap:
 
  out:
   if (info) {
 - if (info-bitmap)
 - kfree(info-bitmap);
 + kfree(info-bitmap);
   kmem_cache_free(btrfs_free_space_cachep, info);
   }
 
 @@ -3322,8 +3321,8 @@ again:
 
   if (info)
   kmem_cache_free(btrfs_free_space_cachep, info);
 - if (map)
 - kfree(map);
 +
 + kfree(map);
   return 0;
  }
 
 diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
 index d23362f..7301b99 100644
 --- a/fs/btrfs/inode.c
 +++ b/fs/btrfs/inode.c
 @@ -857,8 +857,7 @@ static u64 get_extent_allocation_hint(struct inode *inode,
 u64 start,
   em = search_extent_mapping(em_tree, 0, 0);
   if (em  em-block_start  EXTENT_MAP_LAST_BYTE)
   alloc_hint = em-block_start;
 - if (em)
 - free_extent_map(em);
 + free_extent_map(em);
   } else {
   alloc_hint = em-block_start;
   free_extent_map(em);
 @@ -6573,8 +6572,7 @@ out:
 
   trace_btrfs_get_extent(root, em);
 
 - if (path)
 - btrfs_free_path(path);
 + btrfs_free_path(path);
   if (trans) {
   ret = btrfs_end_transaction(trans, root);
   if (!err)
 diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
 index b63ae20..ec8eb49 100644
 --- a/fs/btrfs/reada.c
 +++ b/fs/btrfs/reada.c
 @@ -731,8 +731,7 @@ static int reada_start_machine_dev(struct btrfs_fs_info
 *fs_info,
   else if (eb)
   __readahead_hook(fs_info-extent_root, eb, eb-start, ret);
 
 - if (eb)
 - free_extent_buffer(eb);
 + free_extent_buffer(eb);
 
   return 1;
 
 diff

merging printk and WARN

2012-11-04 Thread Julia Lawall 
It looks like these patches were not a good idea, because in each case the 
printk provides an error level, and WARN then provides another one.


Sorry for the noise.

julia
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 13/16] fs/btrfs: use WARN

2012-11-03 Thread Julia Lawall 
From: Julia Lawall julia.law...@lip6.fr

Use WARN rather than printk followed by WARN_ON(1), for conciseness.

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// smpl
@@
expression list es;
@@

-printk(
+WARN(1,
  es);
-WARN_ON(1);
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
Many of these end up with the form
if (somewhat_complex_condition) WARN(1, ...)
They could also be converted to WARN(somewhat_complex_condition, ...)
if that would be preferred.

 fs/btrfs/ctree.c   |   19 +++
 fs/btrfs/disk-io.c |6 ++
 fs/btrfs/extent-tree.c |7 +++
 fs/btrfs/extent_io.c   |9 +++--
 fs/btrfs/inode.c   |3 +--
 fs/btrfs/transaction.c |   12 
 fs/btrfs/volumes.c |3 +--
 7 files changed, 21 insertions(+), 38 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index cdfb4c4..adfa929 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1361,19 +1361,16 @@ noinline int btrfs_cow_block(struct btrfs_trans_handle 
*trans,
u64 search_start;
int ret;
 
-   if (trans-transaction != root-fs_info-running_transaction) {
-   printk(KERN_CRIT trans %llu running %llu\n,
+   if (trans-transaction != root-fs_info-running_transaction)
+   WARN(1, KERN_CRIT trans %llu running %llu\n,
   (unsigned long long)trans-transid,
   (unsigned long long)
   root-fs_info-running_transaction-transid);
-   WARN_ON(1);
-   }
-   if (trans-transid != root-fs_info-generation) {
-   printk(KERN_CRIT trans %llu running %llu\n,
+
+   if (trans-transid != root-fs_info-generation)
+   WARN(1, KERN_CRIT trans %llu running %llu\n,
   (unsigned long long)trans-transid,
   (unsigned long long)root-fs_info-generation);
-   WARN_ON(1);
-   }
 
if (!should_cow_block(trans, root, buf)) {
*cow_ret = buf;
@@ -3642,11 +3639,9 @@ static noinline int __push_leaf_left(struct 
btrfs_trans_handle *trans,
btrfs_set_header_nritems(left, old_left_nritems + push_items);
 
/* fixup right node */
-   if (push_items  right_nritems) {
-   printk(KERN_CRIT push items %d nr %u\n, push_items,
+   if (push_items  right_nritems)
+   WARN(1, KERN_CRIT push items %d nr %u\n, push_items,
   right_nritems);
-   WARN_ON(1);
-   }
 
if (push_items  right_nritems) {
push_space = btrfs_item_offset_nr(right, push_items - 1) -
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 22a0439..1769e7d 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3383,14 +3383,12 @@ void btrfs_mark_buffer_dirty(struct extent_buffer *buf)
int was_dirty;
 
btrfs_assert_tree_locked(buf);
-   if (transid != root-fs_info-generation) {
-   printk(KERN_CRIT btrfs transid mismatch buffer %llu, 
+   if (transid != root-fs_info-generation)
+   WARN(1, KERN_CRIT btrfs transid mismatch buffer %llu, 
   found %llu running %llu\n,
(unsigned long long)buf-start,
(unsigned long long)transid,
(unsigned long long)root-fs_info-generation);
-   WARN_ON(1);
-   }
was_dirty = set_extent_buffer_dirty(buf);
if (!was_dirty) {
spin_lock(root-fs_info-delalloc_lock);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 3d3e2c1..37353eb 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -6292,10 +6292,9 @@ use_block_rsv(struct btrfs_trans_handle *trans,
static DEFINE_RATELIMIT_STATE(_rs,
DEFAULT_RATELIMIT_INTERVAL,
/*DEFAULT_RATELIMIT_BURST*/ 2);
-   if (__ratelimit(_rs)) {
-   printk(KERN_DEBUG btrfs: block rsv returned %d\n, 
ret);
-   WARN_ON(1);
-   }
+   if (__ratelimit(_rs))
+   WARN(1, KERN_DEBUG btrfs: block rsv returned %d\n,
+ret);
ret = reserve_metadata_bytes(root, block_rsv, blocksize, 0);
if (!ret) {
return block_rsv;
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 472873a..3c062c8 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -341,12 +341,10 @@ static int insert_state(struct extent_io_tree *tree,
 {
struct rb_node *node;
 
-   if (end  start) {
-   printk(KERN_ERR btrfs end  start %llu %llu\n,
+   if (end  start)
+   WARN(1, KERN_ERR btrfs end  start %llu %llu\n,
   (unsigned long long)end,
   (unsigned long long)start

[PATCH 1/8] fs/btrfs: drop if around WARN_ON

2012-11-03 Thread Julia Lawall 
From: Julia Lawall julia.law...@lip6.fr

Just use WARN_ON rather than an if containing only WARN_ON(1).

A simplified version of the semantic patch that makes this transformation
is as follows: (http://coccinelle.lip6.fr/)

// smpl
@@
expression e;
@@
- if (e) WARN_ON(1);
+ WARN_ON(e);
// /smpl

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 fs/btrfs/backref.c |3 +--
 fs/btrfs/ctree.c   |9 +++--
 fs/btrfs/inode.c   |3 +--
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index 208d8aa..a321952 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -890,8 +890,7 @@ again:
while (!list_empty(prefs)) {
ref = list_first_entry(prefs, struct __prelim_ref, list);
list_del(ref-list);
-   if (ref-count  0)
-   WARN_ON(1);
+   WARN_ON(ref-count  0);
if (ref-count  ref-root_id  ref-parent == 0) {
/* no parent == root of tree */
ret = ulist_add(roots, ref-root_id, 0, GFP_NOFS);
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index cdfb4c4..8ecb995 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1469,10 +1469,8 @@ int btrfs_realloc_node(struct btrfs_trans_handle *trans,
if (cache_only  parent_level != 1)
return 0;
 
-   if (trans-transaction != root-fs_info-running_transaction)
-   WARN_ON(1);
-   if (trans-transid != root-fs_info-generation)
-   WARN_ON(1);
+   WARN_ON(trans-transaction != root-fs_info-running_transaction);
+   WARN_ON(trans-transid != root-fs_info-generation);
 
parent_nritems = btrfs_header_nritems(parent);
blocksize = btrfs_level_size(root, parent_level - 1);
@@ -3403,8 +3401,7 @@ static noinline int __push_leaf_right(struct 
btrfs_trans_handle *trans,
if (push_items == 0)
goto out_unlock;
 
-   if (!empty  push_items == left_nritems)
-   WARN_ON(1);
+   WARN_ON(!empty  push_items == left_nritems);
 
/* push left to right */
right_nritems = btrfs_header_nritems(right);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 95542a1..0d169ab 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1657,8 +1657,7 @@ static noinline int add_pending_csums(struct 
btrfs_trans_handle *trans,
 int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end,
  struct extent_state **cached_state)
 {
-   if ((end  (PAGE_CACHE_SIZE - 1)) == 0)
-   WARN_ON(1);
+   WARN_ON((end  (PAGE_CACHE_SIZE - 1)) == 0);
return set_extent_delalloc(BTRFS_I(inode)-io_tree, start, end,
   cached_state, GFP_NOFS);
 }

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] fs/btrfs/volumes.c: add missing free_fs_devices

2012-04-14 Thread Julia Lawall 
From: Julia Lawall julia.law...@lip6.fr

Free fs_devices as done in the error-handling code just below.

Signed-off-by: Julia Lawall julia.law...@lip6.fr

---
 fs/btrfs/volumes.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index a872b48..5d246c3 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4334,8 +4334,10 @@ static int open_seed_devices(struct btrfs_root *root, u8 
*fsid)
 
ret = __btrfs_open_devices(fs_devices, FMODE_READ,
   root-fs_info-bdev_holder);
-   if (ret)
+   if (ret) {
+   free_fs_devices(fs_devices);
goto out;
+   }
 
if (!fs_devices-seeding) {
__btrfs_close_devices(fs_devices);

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 8/11] fs/btrfs/volumes.c: trivial: use BUG_ON

2011-08-02 Thread Julia Lawall 
From: Julia Lawall ju...@diku.dk

Use BUG_ON(x) rather than if(x) BUG();

The semantic patch that fixes this problem is as follows:
(http://coccinelle.lip6.fr/)

// smpl
@@ identifier x; @@
-if (x) BUG();
+BUG_ON(x);

@@ identifier x; @@
-if (!x) BUG();
+BUG_ON(!x);
// /smpl

Signed-off-by: Julia Lawall ju...@diku.dk

---
 fs/btrfs/volumes.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 19450bc..275be0f 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1983,8 +1983,8 @@ again:
   found_key.offset);
if (ret == -ENOSPC)
failed++;
-   else if (ret)
-   BUG();
+   else
+   BUG_ON(ret);
}
 
if (found_key.offset == 0)

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] fs/btrfs: Eliminate memory leak

2010-08-25 Thread Julia Lawall 
Hi all,

I'm really sorry to have spammed your mailbox with my send-email
experiments...

julia
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/16] fs/btrfs: Use available error codes

2010-08-16 Thread Julia Lawall 
From: Julia Lawall ju...@diku.dk

Error codes are stored in ret, but the return value is always 0.  Return
ret instead.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// smpl
@r@
local idexpression x;
constant C;
@@

if (...) { ...
  x = -C
  ... when != x
(
  return +...x...+;
|
  return NULL;
|
  return;
|
* return ...;
)
}
// /smpl

Signed-off-by: Julia Lawall ju...@diku.dk

---
This changes the semantics and has not been tested.

 fs/btrfs/ioctl.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 9254b3d..bc7e9c5 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -212,7 +212,7 @@ static int btrfs_ioctl_setflags(struct file *file, void 
__user *arg)
mnt_drop_write(file-f_path.mnt);
  out_unlock:
mutex_unlock(inode-i_mutex);
-   return 0;
+   return ret;
 }
 
 static int btrfs_ioctl_getversion(struct file *file, int __user *arg)
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/8] fs/btrfs: Use ERR_CAST

2010-05-22 Thread Julia Lawall 
From: Julia Lawall ju...@diku.dk

Use ERR_CAST(x) rather than ERR_PTR(PTR_ERR(x)).  The former makes more
clear what is the purpose of the operation, which otherwise looks like a
no-op.

The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// smpl
@@
type T;
T x;
identifier f;
@@

T f (...) { +...
- ERR_PTR(PTR_ERR(x))
+ x
 ...+ }

@@
expression x;
@@

- ERR_PTR(PTR_ERR(x))
+ ERR_CAST(x)
// /smpl

Signed-off-by: Julia Lawall ju...@diku.dk

---
 fs/btrfs/extent_map.c |4 ++--
 fs/btrfs/super.c  |2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff -u -p a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -335,7 +335,7 @@ struct extent_map *lookup_extent_mapping
goto out;
}
if (IS_ERR(rb_node)) {
-   em = ERR_PTR(PTR_ERR(rb_node));
+   em = ERR_CAST(rb_node);
goto out;
}
em = rb_entry(rb_node, struct extent_map, rb_node);
@@ -384,7 +384,7 @@ struct extent_map *search_extent_mapping
goto out;
}
if (IS_ERR(rb_node)) {
-   em = ERR_PTR(PTR_ERR(rb_node));
+   em = ERR_CAST(rb_node);
goto out;
}
em = rb_entry(rb_node, struct extent_map, rb_node);
diff -u -p a/fs/btrfs/super.c b/fs/btrfs/super.c
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -378,7 +378,7 @@ static struct dentry *get_default_root(s
 find_root:
new_root = btrfs_read_fs_root_no_name(root-fs_info, location);
if (IS_ERR(new_root))
-   return ERR_PTR(PTR_ERR(new_root));
+   return ERR_CAST(new_root);
 
if (btrfs_root_refs(new_root-root_item) == 0)
return ERR_PTR(-ENOENT);
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] fs/btrfs: Correct use after free

2010-03-28 Thread Julia Lawall 
From: Julia Lawall ju...@diku.dk

If the kfree is executed, the dereference of range afterwards will
represent a use after free.  Added goto out, as done in the other nearby
error handling code.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// smpl
@@
expression x,e;
identifier f;
iterator I;
statement S;
@@

*kfree(x);
... when != x
when != x = e
when != I(x,...) S
*x-f
// /smpl

Signed-off-by: Julia Lawall ju...@diku.dk

---
 fs/btrfs/ioctl.c|1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 2845c6c..dacbb52 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1375,6 +1375,7 @@ static int btrfs_ioctl_defrag(struct file *file, void 
__user *argp)
   sizeof(*range))) {
ret = -EFAULT;
kfree(range);
+   goto out;
}
/* compression requires us to start the IO */
if ((range-flags  BTRFS_DEFRAG_RANGE_COMPRESS)) {
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/8] fs/btrfs: introduce missing kfree

2009-09-11 Thread Julia Lawall 
From: Julia Lawall ju...@diku.dk

Error handling code following a kzalloc should free the allocated data.

The semantic match that finds the problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// smpl
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@

x...@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
... when != x
 when != if (...) { +...x...+ }
(
x-f1 = E
|
 (x-f1 == NULL || ...)
|
 f(...,x-f1,...)
)
...
(
 return \(0\|+...x...+\|ptr\);
|
 ret...@p2 ...;
)

@script:python@
p1  r.p1;
p2  r.p2;
@@

print * file: %s kmalloc %s return %s % (p1[0].file,p1[0].line,p2[0].line)
// /smpl

Signed-off-by: Julia Lawall ju...@diku.dk
---
 fs/btrfs/volumes.c  |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 5cf405b..4ced7c3 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -446,8 +446,10 @@ static struct btrfs_fs_devices *clone_fs_devices(struct 
btrfs_fs_devices *orig)
goto error;
 
device-name = kstrdup(orig_dev-name, GFP_NOFS);
-   if (!device-name)
+   if (!device-name) {
+   kfree(device);
goto error;
+   }
 
device-devid = orig_dev-devid;
device-work.func = pending_bios_fn;
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/6] fs/btrfs: convert nested spin_lock_irqsave to spin_lock

2009-07-18 Thread Julia Lawall 
From: Julia Lawall ju...@diku.dk

If spin_lock_irqsave is called twice in a row with the same second
argument, the interrupt state at the point of the second call overwrites
the value saved by the first call.  Indeed, the second call does not need
to save the interrupt state, so it is changed to a simple spin_lock.

The semantic match that finds this problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// smpl
@@
expression lock1,lock2;
expression flags;
@@

*spin_lock_irqsave(lock1,flags)
... when != flags
*spin_lock_irqsave(lock2,flags)
// /smpl

Signed-off-by: Julia Lawall ju...@diku.dk

---
 fs/btrfs/async-thread.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/async-thread.c b/fs/btrfs/async-thread.c
index 6e4f6c5..019e8af 100644
--- a/fs/btrfs/async-thread.c
+++ b/fs/btrfs/async-thread.c
@@ -424,11 +424,11 @@ int btrfs_requeue_work(struct btrfs_work *work)
 * list
 */
if (worker-idle) {
-   spin_lock_irqsave(worker-workers-lock, flags);
+   spin_lock(worker-workers-lock);
worker-idle = 0;
list_move_tail(worker-worker_list,
   worker-workers-worker_list);
-   spin_unlock_irqrestore(worker-workers-lock, flags);
+   spin_unlock(worker-workers-lock);
}
if (!worker-working) {
wake = 1;
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] fs/btrfs: adjust NULL test

2009-07-13 Thread Julia Lawall 
From: Julia Lawall ju...@diku.dk

Move the call to BUG_ON to before the dereference of the tested value.

A simplified version of the semantic match that finds this problem is as
follows: (http://www.emn.fr/x-info/coccinelle/)

// smpl
@r@
expression x,E,E1;
identifier f,l;
position p1,p2;
@@

*...@p1-f = E1;
... when != x = E
when != goto l;
(
*...@p2 == NULL
|
*...@p2 != NULL
)
// /smpl

Signed-off-by: Julia Lawall ju...@diku.dk

---
 fs/btrfs/inode.c|2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 7ffa3d3..8cc6025 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2604,8 +2604,8 @@ noinline int btrfs_truncate_inode_items(struct 
btrfs_trans_handle *trans,
if (root-ref_cows)
btrfs_drop_extent_cache(inode, new_size  (~mask), (u64)-1, 0);
path = btrfs_alloc_path();
-   path-reada = -1;
BUG_ON(!path);
+   path-reada = -1;
 
/* FIXME, add redo link to tree so we don't leak on crash */
key.objectid = inode-i_ino;
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html