Re: [PATCH 08/11] f2fs: fix wrong condition for unlikely

[Jaegeuk Kim <jaegeuk@kernel.org>]

On Wed, Jul 30, 2014 at 09:44:43AM +0800, Chao Yu wrote:
> Hi Jaegeuk,
> 
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> > Sent: Saturday, July 26, 2014 6:47 AM
> > To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org;
> > linux-f2fs-devel@lists.sourceforge.net
> > Cc: Jaegeuk Kim
> > Subject: [f2fs-dev] [PATCH 08/11] f2fs: fix wrong condition for unlikely
> > 
> > This patch fixes the wrongly used unlikely condition.
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/checkpoint.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > index 42a16c1..36b0d47 100644
> > --- a/fs/f2fs/checkpoint.c
> > +++ b/fs/f2fs/checkpoint.c
> > @@ -932,7 +932,7 @@ static void do_checkpoint(struct f2fs_sb_info *sbi, bool is_umount)
> >  	/* Here, we only have one bio having CP pack */
> >  	sync_meta_pages(sbi, META_FLUSH, LONG_MAX);
> > 
> > -	if (unlikely(!is_set_ckpt_flags(ckpt, CP_ERROR_FLAG))) {
> > +	if (!is_set_ckpt_flags(ckpt, CP_ERROR_FLAG)) {
> 
> Maybe use likely(!is_set_ckpt_flags(ckpt, CP_ERROR_FLAG)) or
> 
> if (unlikely(is_set_ckpt_flags(ckpt, CP_ERROR_FLAG)))
> 	return;
> 
> is more appropriate. How do you think?

Currently I'd like to put this without any likely or unlikely.
Best thing is to measure some performance how this would make effect on.
Until then, it'd be better to do without it, since apparently this should
not be unlikely.

How about you?
Can we compare both of them explicitly?

Thanks,

> 
> >  		clear_prefree_segments(sbi);
> >  		release_dirty_inode(sbi);
> >  		F2FS_RESET_SB_DIRT(sbi);
> > --
> > 1.8.5.2 (Apple Git-48)
> > 
> > 
> > ------------------------------------------------------------------------------
> > Want fast and easy access to all the code in your enterprise? Index and
> > search up to 200,000 lines of code with a free copy of Black Duck
> > Code Sight - the same software that powers the world's largest code
> > search on Ohloh, the Black Duck Open Hub! Try it now.
> > http://p.sf.net/sfu/bds
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

------------------------------------------------------------------------------
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls. 
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk