On Tue, May 05, 2020 at 08:35:45AM -0700, Shakeel Butt wrote: > On Tue, May 5, 2020 at 8:27 AM Johannes Weiner <hannes@xxxxxxxxxxx> wrote: > > > > On Mon, May 04, 2020 at 12:23:51PM -0700, Shakeel Butt wrote: > > > On Mon, May 4, 2020 at 9:06 AM Michal Hocko <mhocko@xxxxxxxxxx> wrote: > > > > I really hate to repeat myself but this is no different from a regular > > > > oom situation. > > > > > > Conceptually yes there is no difference but there is no *divine > > > restriction* to not make a difference if there is a real world > > > use-case which would benefit from it. > > > > I would wholeheartedly agree with this in general. > > > > However, we're talking about the very semantics that set memory.max > > apart from memory.high: triggering OOM kills to enforce the limit. > > > > > > when the kernel cannot act and mentions that along with the > > > > oom report so that whoever consumes that information can debug or act on > > > > that fact. > > > > > > > > Silencing the oom report is simply removing a potentially useful > > > > aid to debug further a potential problem. > > > > > > *Potentially* useful for debugging versus actually beneficial for > > > "sweep before tear down" use-case. Also I am not saying to make "no > > > dumps for memory.max when no eligible tasks" a set in stone rule. We > > > can always reevaluate when such information will actually be useful. > > > > > > Johannes/Andrew, what's your opinion? > > > > I still think that if you want to sweep without triggering OOMs, > > memory.high has the matching semantics. > > > > As you pointed out, it doesn't work well for foreign charges, but that > > is more of a limitation in the implementation than in the semantics: > > > > /* > > * If the hierarchy is above the normal consumption range, schedule > > * reclaim on returning to userland. We can perform reclaim here > > * if __GFP_RECLAIM but let's always punt for simplicity and so that > > * GFP_KERNEL can consistently be used during reclaim. @memcg is > > * not recorded as it most likely matches current's and won't > > * change in the meantime. As high limit is checked again before > > * reclaim, the cost of mismatch is negligible. > > */ > > > > Wouldn't it be more useful to fix that instead? It shouldn't be much > > of a code change to do sync reclaim in try_charge(). > > Sync reclaim would really simplify the remote charging case. Though > should sync reclaim only be done for remote charging or for all? I would do it for all __GFP_RECLAIM callers, no need to special case remote charging unnecessarily IMO. We can do both the reclaim as well as the penalty throttling synchronously, i.e. all of mem_cgroup_handle_over_high(). And then punt to the userspace resume when we either didn't reclaim or are still over the threshold after reclaim. Btw we should probably kick off high_work rather than set userspace resume on foreign charges, right? Otherwise we may not reclaim at all when we push a group over its limit from outside (and in a !__GFP_RECLAIM context).
