Re: [RFC] 2/4 Migration Cache - add mm checks

Lee Schermerhorn
2006-02-17T17:09:48Z

On Fri, 2006-02-17 at 08:32 -0800, Christoph Lameter wrote:
> On Fri, 17 Feb 2006, Lee Schermerhorn wrote:
>
> > Index: linux-2.6.16-rc3-mm1/mm/vmscan.c
> > ===================================================================
> > --- linux-2.6.16-rc3-mm1.orig/mm/vmscan.c 2006-02-15 10:50:43.000000000 -0500
> > +++ linux-2.6.16-rc3-mm1/mm/vmscan.c 2006-02-15 10:50:53.000000000 -0500
> > @@ -457,11 +457,19 @@ static unsigned long shrink_page_list(st
> > * Anonymous process memory has backing store?
> > * Try to allocate it some swap space here.
> > */
> > - if (PageAnon(page) && !PageSwapCache(page)) {
> > - if (!sc->may_swap)
> > + if (PageAnon(page)) {
> > + if (!PageSwapCache(page)) {
> > + if (!sc->may_swap)
> > + goto keep_locked;
> > + if (!add_to_swap(page, GFP_ATOMIC))
> > + goto activate_locked;
> > + } else if (page_is_migration(page)) {
> > + /*
> > + * For now, skip migration cache pages.
> > + * TODO: move to swap cache [difficult?]
> > + */
> > goto keep_locked;
> > - if (!add_to_swap(page, GFP_ATOMIC))
> > - goto activate_locked;
> > + }
> > }
> > #endif /* CONFIG_SWAP */
>
>
> Would it not be simpler to modify add_to_swap to switch from migration
> pte to a real swap pte or simply fail? Then you wont have to touch
> shrink_page().

I can look into that. This is new, untested code for comment, so
thanks.

>
> > Index: linux-2.6.16-rc3-mm1/mm/rmap.c
> > ===================================================================
> > --- linux-2.6.16-rc3-mm1.orig/mm/rmap.c 2006-02-15 10:50:43.000000000 -0500
> > +++ linux-2.6.16-rc3-mm1/mm/rmap.c 2006-02-15 10:50:53.000000000 -0500
> > @@ -232,7 +232,13 @@ void remove_from_swap(struct page *page)
> >
> > spin_unlock(&anon_vma->lock);
> >
> > - delete_from_swap_cache(page);
> > + if (PageSwapCache(page))
> > + delete_from_swap_cache(page);
> > + /*
> > + * if page was in migration cache, it will have been
> > + * removed when the last swap pte referencing the entry
> > + * was removed by the loop above.
> > + */
> > }
> > EXPORT_SYMBOL(remove_from_swap);
> > #endif
>
> Hmmm. That points to inconsistent behavior of the swap functions in case
> these are working on the migration cache. Could you keep PageSwapCache
> until delete_from_swap_cache is called?

Well, one could redesign the migration cache such that the cache held a
ref
on the entry, like the swap cache. Then we'd need an explicit call
later to
destroy the entry. I guess this would happen when we free the page.
Pages
would just tend to hang around in the migration cache for no reason
after
all ptes references had been removed. Short of that, I don't think we
can
keep the PageSwapCache() set when the page really isn't in either the
swap
nor migration cache.

A simpler, but perhaps riskier?, approach would be to have
delete_from_swap_cache() silently ingnore pages that are no longer in
the swap cache. Currently, the lower level __delete_from_swap_cache()
will BUG_ON this case. Making delete_from_swap_cache() forgiving in
this case would handle the call from remove_from_swap() above.