Re: [PATCH 1/4] Swap migration V3: LRU operations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2005-10-21 at 15:27 +0900, Magnus Damm wrote:
> On 10/21/05, Dave Hansen <[email protected]> wrote:
> > On Thu, 2005-10-20 at 15:59 -0700, Christoph Lameter wrote:
> > > + *  0 = page not on LRU list
> > > + *  1 = page removed from LRU list
> > > + * -1 = page is being freed elsewhere.
> > > + */
> >
> > Can these return values please get some real names?  I just hate when
> > things have more than just fail and success as return codes.
> >
> > It makes much more sense to have something like:
> >
> >         if (ret == ISOLATION_IMPOSSIBLE) {
> 
> Absolutely. But this involves figuring out nice names that everyone
> likes and that does not pollute the name space too much.

So, your excuse for bad code is that you want to avoid a discussion?
Are you new here? ;)

> Any suggestions?

I'd start with the comment, and work from there.  

ISOLATE_PAGE_NOT_LRU
ISOLATE_PAGE_REMOVED_FROM_LRU
ISOLATE_PAGE_FREEING_ELSEWHERE

Not my best names in history, but probably a place to start.  It keeps
the author from having to add bad comments explaining what the code
does.

> > BTW, it would probably be nice to say where these patches came from
> > before Magnus. :)
> 
> Uh? Yesterday I broke out code from isolate_lru_pages() and
> shrink_cache() and emailed Christoph privately. Do you have similar
> code in your tree?

Hirokazu's page migration patches have some functions called the exact
same things: __putback_page_to_lru, etc... although they are simpler.
Not my code, but it would be nice to acknowledge if ideas were coming
from there.

-- Dave

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux