Hi Dan, On Fri, Aug 30, 2019 at 02:26:12PM +0300, Dan Carpenter wrote: > On Fri, Aug 30, 2019 at 04:43:33PM +0800, Gao Xiang wrote: > > Hi Dan, > > > > On Fri, Aug 30, 2019 at 11:34:45AM +0300, Dan Carpenter wrote: > > > On Fri, Aug 30, 2019 at 12:04:41AM +0800, Gao Xiang wrote: > > > > Anyway, I'm fine to delete them all if you like, but I think majority of these > > > > are meaningful. > > > > > > > > data.c- /* page is already locked */ > > > > data.c- DBG_BUGON(PageUptodate(page)); > > > > data.c- > > > > data.c: if (unlikely(err)) > > > > data.c- SetPageError(page); > > > > data.c- else > > > > data.c- SetPageUptodate(page); > > > > > > If we cared about speed here then we would delete the DBG_BUGON() check > > > because that's going to be expensive. The likely/unlikely annotations > > > should be used in places a reasonable person thinks it will make a > > > difference to benchmarks. > > > > DBG_BUGON will be a no-op ((void)x) in non-debugging mode, > > It expands to: > > ((void)PageUptodate(page)); > > Calling PageUptodate() doesn't do anything, but it isn't free. The > time it takes to do that function call completely negates any speed up > from using likely/unlikely. > > I'm really not trying to be a jerk... You are right, I recalled that PageUptodate is not as simple as it implys. Yes, those are all removed now... I am ok with that, thanks for your suggestion :) Thanks, Gao Xiang > > regards, > dan carpenter >
