Re: [PATCH v2 02/28] vmscan: take at least one pass with shrinkers

[Glauber Costa <glommer@parallels.com>]

On 04/08/2013 01:01 PM, Joonsoo Kim wrote:
> On Mon, Apr 08, 2013 at 12:47:14PM +0400, Glauber Costa wrote:
>> On 04/08/2013 12:42 PM, Joonsoo Kim wrote:
>>> Hello, Glauber.
>>>
>>> On Fri, Mar 29, 2013 at 01:13:44PM +0400, Glauber Costa wrote:
>>>> In very low free kernel memory situations, it may be the case that we
>>>> have less objects to free than our initial batch size. If this is the
>>>> case, it is better to shrink those, and open space for the new workload
>>>> then to keep them and fail the new allocations.
>>>>
>>>> More specifically, this happens because we encode this in a loop with
>>>> the condition: "while (total_scan >= batch_size)". So if we are in such
>>>> a case, we'll not even enter the loop.
>>>>
>>>> This patch modifies turns it into a do () while {} loop, that will
>>>> guarantee that we scan it at least once, while keeping the behaviour
>>>> exactly the same for the cases in which total_scan > batch_size.
>>>
>>> Current user of shrinker not only use their own condition, but also
>>> use batch_size and seeks to throttle their behavior. So IMHO,
>>> this behavior change is very dangerous to some users.
>>>
>>> For example, think lowmemorykiller.
>>> With this patch, he always kill some process whenever shrink_slab() is
>>> called and their low memory condition is satisfied.
>>> Before this, total_scan also prevent us to go into lowmemorykiller, so
>>> killing innocent process is limited as much as possible.
>>>
>> shrinking is part of the normal operation of the Linux kernel and
>> happens all the time. Not only the call to shrink_slab, but actual
>> shrinking of unused objects.
>>
>> I don't know therefore about any code that would kill process only
>> because they have reached shrink_slab.
>>
>> In normal systems, this loop will be executed many, many times. So we're
>> not shrinking *more*, we're just guaranteeing that at least one pass
>> will be made.
> 
> This one pass guarantee is a problem for lowmemory killer.
> 
>> Also, anyone looking at this to see if we should kill processes, is a
>> lot more likely to kill something if we tried to shrink but didn't, than
>> if we successfully shrunk something.
> 
> lowmemory killer is hacky user of shrink_slab interface.

Well, it says it all =)

In special, I really can't see how, hacky or not, it makes sense to kill
a process if we *actually* shrunk memory.

Moreover, I don't see the code in drivers/staging/android/lowmemory.c
doing anything even remotely close to that. Could you point me to some
code that does it ?