On 2017/10/26 16:49, Michal Hocko wrote: > On Wed 25-10-17 15:49:21, Greg Thelen wrote: >> Johannes Weiner <hannes@xxxxxxxxxxx> wrote: >> >>> On Wed, Oct 25, 2017 at 09:00:57PM +0200, Michal Hocko wrote: > [...] >>>> So just to make it clear you would be OK with the retry on successful >>>> OOM killer invocation and force charge on oom failure, right? >>> >>> Yeah, that sounds reasonable to me. >> >> Assuming we're talking about retrying within try_charge(), then there's >> a detail to iron out... >> >> If there is a pending oom victim blocked on a lock held by try_charge() caller >> (the "#2 Locks" case), then I think repeated calls to out_of_memory() will >> return true until the victim either gets MMF_OOM_SKIP or disappears. > > true. And oom_reaper guarantees that MMF_OOM_SKIP gets set in the finit > amount of time. Just a confirmation. You are talking about kmemcg, aren't you? And kmemcg depends on CONFIG_MMU=y, doesn't it? If no, there is no such guarantee. > >> So a force >> charge fallback might be a needed even with oom killer successful invocations. >> Or we'll need to teach out_of_memory() to return three values (e.g. NO_VICTIM, >> NEW_VICTIM, PENDING_VICTIM) and try_charge() can loop on NEW_VICTIM. > > No we, really want to wait for the oom victim to do its job. The only > thing we should be worried about is when out_of_memory doesn't invoke > the reaper. There is only one case like that AFAIK - GFP_NOFS request. I > have to think about this case some more. We currently fail in that case > the request. > Do we really need to apply /* * The OOM killer does not compensate for IO-less reclaim. * pagefault_out_of_memory lost its gfp context so we have to * make sure exclude 0 mask - all other users should have at least * ___GFP_DIRECT_RECLAIM to get here. */ if (oc->gfp_mask && !(oc->gfp_mask & __GFP_FS)) return true; unconditionally? We can encourage !__GFP_FS allocations to use __GFP_NORETRY or __GFP_RETRY_MAYFAIL if their allocations are not important. Then, only important !__GFP_FS allocations will be checked here. I think that we can allow such important allocations to invoke the OOM killer (i.e. remove this check) because situation is already hopeless if important !__GFP_FS allocations cannot make progress.
