Re: [PATCH 1/2] mm, memory_hotplug: do not fail offlining too early

[Michal Hocko]

On Wed 13-09-17 13:41:20, Vlastimil Babka wrote:
> On 09/11/2017 10:17 AM, Michal Hocko wrote:
[...]
> > Yes, we should be able to distinguish the two and hopefully we can teach
> > the migration code to distinguish between EBUSY (likely permanent) and
> > EGAIN (temporal) failure. This sound like something we should aim for
> > longterm I guess. Anyway as I've said in other email. If somebody really
> > wants to have a guaratee of a bounded retry then it is trivial to set up
> > an alarm and send a signal itself to bail out.
> 
> Sure, I would just be careful about not breaking existing userspace
> (udev?) when offline triggered via ACPI from some management interface
> (or whatever the exact mechanism is).

The thing is that there is absolutely no timing guarantee even with
retry limit in place. We are doing allocations, potentially bouncing on
locks which can be taken elsewhere etc... So if somebody really depend
on this then it is pretty much broken already.

> > Do you think that the changelog should be more clear about this?
> 
> It certainly wouldn't hurt :)

So what do you think about the following wording:

commit 23c4ded55c2ba880165a9f5b8a67694361fb6bc7
Author: Michal Hocko <mhocko@xxxxxxxx>
Date:   Mon Aug 28 13:13:06 2017 +0200

    mm, memory_hotplug: remove timeout from __offline_memory
    
    We have a hardcoded 120s timeout after which the memory offline fails
    basically since the hot remove has been introduced. This is essentially
    a policy implemented in the kernel. Moreover there is no way to adjust
    the timeout and so we are sometimes facing memory offline failures if
    the system is under a heavy memory pressure or very intensive CPU
    workload on large machines.
    
    It is not very clear what purpose the timeout actually serves. The
    offline operation is interruptible by a signal so if userspace wants
    some timeout based termination this can be done trivially by sending a
    signal.
    
    If there is a strong usecase to do this from the kernel then we should
    do it properly and have a it tunable from the userspace with the timeout
    disabled by default along with the explanation who uses it and for what
    purporse.
    
    Acked-by: Vlastimil Babka <vbabka@xxxxxxx>
    Signed-off-by: Michal Hocko <mhocko@xxxxxxxx>
-- 
Michal Hocko
SUSE Labs