On Tue 06-08-19 19:26:12, Tetsuo Handa wrote: > On 2019/08/05 23:26, Michal Hocko wrote: > > On Mon 05-08-19 23:00:12, Tetsuo Handa wrote: > >> On 2019/08/05 20:44, Michal Hocko wrote: > >>>> Allowing forced charge due to being unable to invoke memcg OOM killer > >>>> will lead to global OOM situation, and just returning -ENOMEM will not > >>>> solve memcg OOM situation. > >>> > >>> Returning -ENOMEM would effectivelly lead to triggering the oom killer > >>> from the page fault bail out path. So effectively get us back to before > >>> 29ef680ae7c21110. But it is true that this is riskier from the > >>> observability POV when a) the OOM path wouldn't point to the culprit and > >>> b) it would leak ENOMEM from g-u-p path. > >>> > >> > >> Excuse me? But according to my experiment, below code showed flood of > >> "Returning -ENOMEM" message instead of invoking the OOM killer. > >> I didn't find it gets us back to before 29ef680ae7c21110... > > > > You would need to declare OOM_ASYNC to return ENOMEM properly from the > > charge (which is effectivelly a revert of 29ef680ae7c21110 for NOFS > > allocations). Something like the following > > > > OK. We need to set current->memcg_* before declaring something other than > OOM_SUCCESS and OOM_FAILED... Well, it seems that returning -ENOMEM after > setting current->memcg_* works as expected. What's wrong with your approach? As I've said, and hoped you could pick up parts for your changelog for the ENOMEM part, a) oom path is lost b) some paths will leak ENOMEM e.g. g-u-p. So your patch to trigger the oom even for NOFS is a better alternative I just found your ENOMEM note misleading and something that could improve. -- Michal Hocko SUSE Labs
