Re: [f2fs-dev] [PATCH] f2fs: add unlikely() macro for compiler more aggressively

2013-12-06 Thread Chao Yu 
 -Original Message-
 From: Jaegeuk Kim [mailto:jaegeuk@samsung.com]
 Sent: Friday, December 06, 2013 2:15 PM
 Cc: Jaegeuk Kim; linux-fsde...@vger.kernel.org; linux-ker...@vger.kernel.org; 
 linux-f2fs-devel@lists.sourceforge.net; Chao Yu
 Subject: [PATCH] f2fs: add unlikely() macro for compiler more aggressively
 
 This patch adds unlikely() macro into the most of codes.
 The basic rule is to add that when:
 - checking unusual errors,
 - checking page mappings,
 - and the other unlikely conditions.

Looks nice to me!

 
 Cc: Chao Yu chao2...@samsung.com
 Signed-off-by: Jaegeuk Kim jaegeuk@samsung.com

Reviewed-by: Chao Yu chao2...@samsung.com

 ---
  fs/f2fs/checkpoint.c |  22 ++
  fs/f2fs/data.c   |  63 +--
  fs/f2fs/dir.c|  10 ++---
  fs/f2fs/file.c   |  23 +-
  fs/f2fs/gc.c |  19 -
  fs/f2fs/inode.c  |  12 +++---
  fs/f2fs/namei.c  |  24 ++-
  fs/f2fs/node.c   | 117 
 +++
  fs/f2fs/recovery.c   |  10 ++---
  fs/f2fs/segment.c|  42 +-
  fs/f2fs/super.c  |  83 ++--
  fs/f2fs/xattr.c  |  28 ++--
  12 files changed, 234 insertions(+), 219 deletions(-)

[snip]

 @@ -808,11 +808,11 @@ retry:
   brelse(*raw_super_buf);
   f2fs_msg(sb, KERN_ERR, Can't find a valid F2FS filesystem 
   in %dth superblock, block + 1);
 - if(block == 0) {
 + if(unlikely(block != 0)) {

Original code has style problem:
Space is required before the open parenthesis '('.
Thanks.



--
Sponsored by Intel(R) XDK 
Develop, test and display web and hybrid apps with a single code base.
Download it for free now!
http://pubads.g.doubleclick.net/gampad/clk?id=111408631iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: add unlikely() macro for compiler more aggressively

2013-12-06 Thread Andi Kleen 
Jaegeuk Kim jaegeuk@samsung.com writes:
   struct page *page;
  repeat:
   page = grab_cache_page(mapping, index);
 - if (!page) {
 + if (unlikely(!page)) {

This is completely pointless, gcc already considers any test for NULL
unlikely.

In general i would advise against splattering unlikely all over your
code, the benefits are very minimal and programers often get it wrong.

-Andi

-- 
a...@linux.intel.com -- Speaking for myself only

--
Sponsored by Intel(R) XDK 
Develop, test and display web and hybrid apps with a single code base.
Download it for free now!
http://pubads.g.doubleclick.net/gampad/clk?id=111408631iu=/4140/ostg.clktrk
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel