Re: [PATCH 1/2] Page migration via Swap V2: Page Eviction

[Magnus Damm <magnus.damm@gmail.com>]

On 10/18/05, Christoph Lameter <clameter@sgi.com> wrote:
> +/*
> + * Isolate one page from the LRU lists and put it on the
> + * indicated list.
> + *
> + * Result:
> + *  0 = page not on LRU list
> + *  1 = page removed from LRU list and added to the specified list.
> + * -1 = page is being freed elsewhere.
> + */
> +int isolate_lru_page(struct page *page, struct list_head *l)
> +{
> +       int rc = 0;
> +       struct zone *zone = page_zone(page);
> +
> +redo:
> +       spin_lock_irq(&zone->lru_lock);
> +       if (TestClearPageLRU(page)) {
> +               list_del(&page->lru);
> +               if (get_page_testone(page)) {
> +                       /*
> +                        * It is being freed elsewhere
> +                        */
> +                       __put_page(page);
> +                       SetPageLRU(page);
> +                       if (PageActive(page))
> +                               list_add(&page->lru, &zone->active_list);
> +                       else
> +                               list_add(&page->lru, &zone->inactive_list);
> +                       rc = -1;
> +               } else {
> +                       list_add(&page->lru, l);
> +                       if (PageActive(page))
> +                               zone->nr_active--;
> +                       else
> +                               zone->nr_inactive--;
> +                       rc = 1;
> +               }
> +       }
> +       spin_unlock_irq(&zone->lru_lock);
> +       if (rc == 0) {
> +               /*
> +                * Maybe this page is still waiting for a cpu to drain it
> +                * from one of the lru lists?
> +                */
> +               smp_call_function(&lru_add_drain_per_cpu, NULL, 0, 1);
> +               lru_add_drain();
> +               if (PageLRU(page))
> +                       goto redo;
> +       }
> +       return rc;
> +}

This function is very similar to isolate_lru_pages(), except that it
operates on one page at a time and drains the lru if needed. Maybe
isolate_lru_pages() could use this function (inline) if the spinlock
and drain code was moved out?

I'm also curios why you choose to always use list_del() and move back
the page if freed elsewhere, instead of using
del_page_from_[in]active_list(). I guess because of performance. But
if that is the case, wouldn't it make sense to do as little as
possible with the spinlock held, ie move list_add() (when rc == 1) out
of the function?

I'd love to see those patches included somewhere, it would help me a
lot when I build code for separated mapped and unmapped LRU:s.

/ magnus

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