Re: [PATCH] fix unused/stale swap cache handling on memcg v3

[Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>]

Sorry for late reply.

On Fri, 17 Apr 2009 17:13:43 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> On Fri, 17 Apr 2009 17:12:01 +0900
> Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> 
> > On Fri, 17 Apr 2009 16:58:06 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > > On Fri, 17 Apr 2009 16:50:36 +0900
> > > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > > 
> > > > On Fri, 17 Apr 2009 15:54:11 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > > > > On Fri, 17 Apr 2009 15:34:55 +0900
> > > > > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > > > > > I made a patch for reclaiming SwapCache from orphan LRU based on your patch,
> > > > > > and have been testing it these days.
> > > > > > 
> > > > > Good trial! 
> > > > > Honestly, I've written a patch to fix this problem in these days but seems to
> > > > > be over-kill ;)
> > > > > 
> > > > > 
> > > > > > Major changes from your version:
> > > > > > - count the number of orphan pages per zone and make the threshold per zone(4MB).
> > > > > > - As for type 2 of orphan SwapCache, they are usually set dirty by add_to_swap.
> > > > > >   But try_to_drop_swapcache(__remove_mapping) can't free dirty pages,
> > > > > >   so add a check and try_to_free_swap to the end of shrink_page_list.
> > > > > > 
> > > > > > It seems work fine, no "pseud leak" of SwapCache can be seen.
> > > > > > 
> > > > > > What do you think ?
> > > > > > If it's all right, I'll merge this with the orphan list framework patch
> > > > > > and send it to Andrew with other fixes of memcg that I have.
> > > > > > 
> > > > > I'm sorry but my answer is "please wait". The reason is..
> > > > > 
> > > > > 1. When global LRU works, the pages will be reclaimed.
> > > > > 2. Global LRU will work finally.
> > > > > 3. While testing, "stale" swap cache cannot be big amount.
> > > > > 
> > > > Hmm, I can't understand 2.
> > > > 
> > > > If (memsize on system) >> (swapsize on system), global LRU doesn't run
> > > > and all the swap space can be used up by these SwapCache.
> > > > This means setting mem.limit can use up all the swap space on the system.
> > > > I've tested with 50MB size of swap and it can be used up in less than 24h.
> > > > I think it's not small.
> > > > 
> > > 
> > > plz add hook to shrink_zone() to fix this as you did. 
> > > orphan list is overkilling at this stage.
> > > 
> > I see.
> > 
> > I'll make a patch, test it, and repost it in next week.
> > It can prevent at least type-2 of orphan SwapCache.
> > 
> BTW, type-1 still exits ?
> 
Ah, I meant adding to shrink_page_list():

@@ -785,6 +786,23 @@ activate_locked:
                SetPageActive(page);
                pgactivate++;
 keep_locked:
+               if (!scanning_global_lru(sc) && PageSwapCache(page)) {
+                       struct page_cgroup *pc;
+
+                       pc = lookup_page_cgroup(page);
+                       /*
+                        * Used bit of swapcache is solid under page lock.
+                        */
+                       if (unlikely(!PageCgroupUsed(pc)))
+                               /*
+                                * This can happen if the page is free'ed by
+                                * the owner process before it is added to
+                                * swapcache.
+                                * These swapcache cannot be managed by memcg
+                                * well, so free it here.
+                                */
+                               try_to_free_swap(page);
+               }
                unlock_page(page);
 keep:
                list_add(&page->lru, &ret_pages);

This cannot prevent type-1 orphan SwapCache(caused by the race
between exit() and swap-in readahead).
Type-1 can pressure the memsw usage(trigger OOM if memsw.limit is set, as a result)
and make struct mem_cgroup unfreeable even after rmdir(because it holds refcount
to mem_cgroup).

Do you have any ideas to solve orphan SwapCache problem by adding some hooks to shrink_zone() ?
(scan some pages from global LRU and check whether it's orphan SwapCache or not by
adding some code like above ?)

And, what do you think about adding above code to shrink_page_list() ?
I think it might be unnecessary if we can solve the problem in another way, though.


Thanks,
Daisuke Nishimura.

> > I'll revisit orphan list if needed in future.
> > 
> Thank you!.
> 
> Regards,
> -Kame
> 
> > 
> > Thanks,
> > Daisuke Nishimura.
> > 
> 

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