On 12/13/2012 11:05 AM, Michal Hocko wrote:> On Thu 13-12-12 16:29:59, Michal Hocko wrote:

quoted

On Thu 13-12-12 10:34:20, Mel Gorman wrote:

quoted

On Wed, Dec 12, 2012 at 04:43:34PM -0500, Johannes Weiner wrote:

quoted

When a reclaim scanner is doing its final scan before giving up and 
there is swap space available, pay no attention to swappiness 
preference anymore.  Just swap.

Note that this change won't make too big of a difference for 
general
reclaim: anonymous pages are already force-scanned when there is 
only very little file cache left, and there very likely isn't when 
the reclaimer enters this final cycle.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Ok, I see the motivation for your patch but is the block inside 
still wrong for what you want? After your patch the block looks like 
this

                if (sc->priority || noswap) {
                        scan >>= sc->priority;
                        if (!scan && force_scan)
                                scan = SWAP_CLUSTER_MAX;
                        scan = div64_u64(scan * fraction[file], denominator);
                }

if sc->priority == 0 and swappiness==0 then you enter this block but 
fraction[0] for anonymous pages will also be 0 and because of the 
ordering of statements there, scan will be

scan = scan * 0 / denominator

so you are still not reclaiming anonymous pages in the swappiness=0 
case. What did I miss?

Yes, now that you have mentioned that I realized that it really 
doesn't make any sense. fraction[0] is _always_ 0 for swappiness==0. 
So we just made a bigger pressure on file LRUs. So this sounds like a 
misuse of the swappiness. This all has been introduced with fe35004f 
(mm: avoid swapping out with swappiness==0).

I think that removing swappiness check make sense but I am not sure 
it does what the changelog says. It should have said that checking 
swappiness doesn't make any sense for small LRUs.

Bahh, wait a moment. Now I remember why the check made sense 
especially for memcg.
It made "don't swap _at all_ for swappiness==0" for real - you are 
even willing to sacrifice OOM. Maybe this is OK for the global case 
because noswap would safe you here (assuming that there is no swap if 
somebody doesn't want to swap at all and swappiness doesn't play such 
a big role) but for memcg you really might want to prevent from 
swapping - not everybody has memcg swap extension enabled and swappiness is handy then.
So I am not sure this is actually what we want. Need to think about it.

I introduced swappiness check here with fe35004f because, in some
cases, we prefer OOM to swap out pages to detect problems as soon
as possible. Basically, we design the system not to swap out and
so if it causes swapping, something goes wrong.

Regards,
Satoru

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