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) {
list_del(&page->lru);
list_add(&page->lru, src);
}
than
+ if (rc == -1) { /* Not possible to isolate */
+ list_del(&page->lru);
+ list_add(&page->lru, src);
+ } if
The comment just makes the code harder to read.
> +static inline int
> +__isolate_lru_page(struct zone *zone, struct page *page)
> +{
> + if (TestClearPageLRU(page)) {
> + if (get_page_testone(page)) {
> + /*
> + * It is being freed elsewhere
> + */
> + __put_page(page);
> + SetPageLRU(page);
> + return -1;
> + } else {
> + if (PageActive(page))
> + del_page_from_active_list(zone, page);
> + else
> + del_page_from_inactive_list(zone, page);
> + return 1;
> + }
> + }
> +
> + return 0;
> +}
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.
> +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. :)
-- 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]
