On Tue, Jun 01, 2010 at 12:29:41PM +0900, KOSAKI Motohiro wrote:
> CC to memcg folks.
>
> > I agree with the direction of this patch, but I am seeing a hang when
> > testing with mmotm-2010-05-21-16-05. The following test hangs, unless I
> > remove this patch from mmotm:
> > mount -t cgroup none /cgroups -o memory
> > mkdir /cgroups/cg1
> > echo $$ > /cgroups/cg1/tasks
> > dd bs=1024 count=1024 if=/dev/null of=/data/foo
> > echo $$ > /cgroups/tasks
> > echo 1 > /cgroups/cg1/memory.force_empty
> >
> > I think the hang is caused by the following portion of
> > mem_cgroup_force_empty():
> > while (nr_retries && mem->res.usage > 0) {
> > int progress;
> >
> > if (signal_pending(current)) {
> > ret = -EINTR;
> > goto out;
> > }
> > progress = try_to_free_mem_cgroup_pages(mem, GFP_KERNEL,
> > false, get_swappiness(mem));
> > if (!progress) {
> > nr_retries--;
> > /* maybe some writeback is necessary */
> > congestion_wait(BLK_RW_ASYNC, HZ/10);
> > }
> >
> > }
> >
> > With this patch applied, it is possible that when do_try_to_free_pages()
> > calls shrink_zones() for priority 0 that shrink_zones() may return 1
> > indicating progress, even though no pages may have been reclaimed.
> > Because this is a cgroup operation, scanning_global_lru() is false and
> > the following portion of do_try_to_free_pages() fails to set ret=0.
> > > if (ret && scanning_global_lru(sc))
> > > ret = sc->nr_reclaimed;
> > This leaves ret=1 indicating that do_try_to_free_pages() reclaimed 1
> > page even though it did not reclaim any pages. Therefore
> > mem_cgroup_force_empty() erroneously believes that
> > try_to_free_mem_cgroup_pages() is making progress (one page at a time),
> > so there is an endless loop.
>
> Good catch!
>
> Yeah, your analysis is fine. thank you for both your testing and
> making analysis.
>
> Unfortunatelly, this logic need more fix. because It have already been
> corrupted by another regression. my point is, if priority==0 reclaim
> failure occur, "ret = sc->nr_reclaimed" makes no sense at all.
>
> The fixing patch is here. What do you think?
>
>
>
> From 49a395b21fe1b2f864112e71d027ffcafbdc9fc0 Mon Sep 17 00:00:00 2001
> From: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>
> Date: Tue, 1 Jun 2010 11:29:50 +0900
> Subject: [PATCH] vmscan: Fix do_try_to_free_pages() return value when priority==0 reclaim failure
>
> Greg Thelen reported recent Johannes's stack diet patch makes kernel
> hang. His test is following.
>
> mount -t cgroup none /cgroups -o memory
> mkdir /cgroups/cg1
> echo $$ > /cgroups/cg1/tasks
> dd bs=1024 count=1024 if=/dev/null of=/data/foo
> echo $$ > /cgroups/tasks
> echo 1 > /cgroups/cg1/memory.force_empty
>
> Actually, This OOM hard to try logic have been corrupted
> since following two years old patch.
>
> commit a41f24ea9fd6169b147c53c2392e2887cc1d9247
> Author: Nishanth Aravamudan <nacc@xxxxxxxxxx>
> Date: Tue Apr 29 00:58:25 2008 -0700
>
> page allocator: smarter retry of costly-order allocations
>
> Original intention was "return success if the system have shrinkable
> zones though priority==0 reclaim was failure". But the above patch
> changed to "return nr_reclaimed if .....". Oh, That forgot nr_reclaimed
> may be 0 if priority==0 reclaim failure.
>
> And Johannes's patch made more corrupt. Originally, priority==0 recliam
> failure on memcg return 0, but this patch changed to return 1. It
> totally confused memcg.
>
> This patch fixes it completely.
>
> Reported-by: Greg Thelen <gthelen@xxxxxxxxxx>
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>
> Cc: Johannes Weiner <hannes@xxxxxxxxxxx>
Greg, thanks for this great bug report. KOSAKI-san, thanks a lot
for fixing it. And I am sorry I did not reply earlier, there is
just too much going on right now :/
Acked-by: Johannes Weiner <hannes@xxxxxxxxxxx>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>