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

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

On 10/19/05, Christoph Lameter <clameter@engr.sgi.com> wrote:
> On Tue, 18 Oct 2005, Magnus Damm wrote:
>
> > 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?
>
> isolate_lru_pages operates on batches of pages from the same zone and is
> very efficient by only taking a single lock. It also does not drain other
> processors LRUs.

Ah, I see. You have a mix of pages from different zones on your list.
Maybe it is possible to use the same kind of zone locking style as
release_pages() to avoid duplicating code...

> > 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 tried to follow isolate_lru_pages as closely as possible. list_add() is
> a simple operation and so I left it inside following some earlier code
> from the hotplug project.

Yep, it probably won't matter.

I'm trying to figure out if this code works in all cases:

+               spin_lock_irq(&zone->lru_lock);
+               list_del(&page->lru);
+               if (!TestSetPageLRU(page)) {
+                       if (PageActive(page))
+                               add_page_to_active_list(zone, page);
+                       else
+                               add_page_to_inactive_list(zone, page);
+                       count++;
+               }
+               spin_unlock_irq(&zone->lru_lock);

Why not use if (TestSetPageLRU(page)) BUG()?

Or is it possible that someone sets the LRU bit while we are keeping
the pages on our non-lru list? If so, who is stealing and will your
put_page() patch work correctly if the page is stolen from us?

Thanks,

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