Re: [RESEND][PATCH V1] mm/vsmcan: check shrink_active_list() sc->isolate_pages() return value.

From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
To: Wu Fengguang <fengguang.wu@intel.com>
Cc: kosaki.motohiro@jp.fujitsu.com,
	Minchan Kim <minchan.kim@gmail.com>, Vincent Li <root@brc.ubc.ca>,
	Vincent Li <macli@brc.ubc.ca>, Mel Gorman <mel@csn.ul.ie>,
	Andrew Morton <akpm@linux-foundation.org>,
	"riel@redhat.com" <riel@redhat.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>
Subject: Re: [RESEND][PATCH V1] mm/vsmcan: check shrink_active_list() sc->isolate_pages() return value.
Date: Fri, 16 Oct 2009 12:05:58 +0900 (JST)	[thread overview]
Message-ID: <20091016120242.AF31.A69D9226@jp.fujitsu.com> (raw)
In-Reply-To: <20091016022011.GA22706@localhost>

> On Fri, Oct 16, 2009 at 10:10:41AM +0800, Minchan Kim wrote:
> > Hi, Vicent. 
> > First of all, Thanks for your effort. :)
>  
> That's pretty serious efforts ;)
> 
> > But as your data said, on usual case, nr_taken_zero count is much less 
> > than non_zero. so we could lost benefit in normal case due to compare
> > insturction although it's trivial. 
> > 
> > I have no objection in this patch since overhead is not so big.
> > But I am not sure what other guys think about it. 
> > 
> > How about adding unlikely following as ?
> > 
> > +
> > +       if (unlikely(nr_taken == 0))
> > +               goto done;
> 
> I would prefer to just remove it - to make the code simple :)

+1 me.

Thank you, Vincent. Your effort was pretty clear and good.
but your mesurement data didn't persuade us.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>