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

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

 



On 10/21/05, Dave Hansen <[email protected]> wrote:
> On Thu, 2005-10-20 at 15:59 -0700, Christoph Lameter wrote:
>
> > +/*
> > + * Isolate one page from the LRU lists.
> > + *
> > + * - zone->lru_lock must be held
> > + *
> > + * Result:
> > + *  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. Any
suggestions?

> How about
>
> +static inline int
> > +__isolate_lru_page(struct zone *zone, struct page *page)
> > +{
>         int ret = 0;
>
>         if (!TestClearPageLRU(page))
>                 return ret;
>
> Then, the rest of the thing doesn't need to be indented.

Good idea.

> > +static inline void
> > +__putback_lru_page(struct zone *zone, struct page *page)
> > +{
>
> __put_back_lru_page?
>
> 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?

/ magnus
-
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