[Intel-gfx] [PATCH] kernel.h: Add non_block_start/end()

[Daniel Vetter]

On Tue, May 21, 2019 at 08:24:53PM +0900, Tetsuo Handa wrote:
> On 2019/05/21 20:11, Michal Hocko wrote:
> > On Tue 21-05-19 20:04:34, Tetsuo Handa wrote:
> >> On 2019/05/21 19:51, Michal Hocko wrote:
> >>> On Tue 21-05-19 19:44:01, Tetsuo Handa wrote:
> >>>> On 2019/05/21 19:06, Daniel Vetter wrote:
> >>>>> In some special cases we must not block, but there's not a
> >>>>> spinlock, preempt-off, irqs-off or similar critical section already
> >>>>> that arms the might_sleep() debug checks. Add a non_block_start/end()
> >>>>> pair to annotate these.
> >>>>>
> >>>>> This will be used in the oom paths of mmu-notifiers, where blocking is
> >>>>> not allowed to make sure there's forward progress. Quoting Michal:
> >>>>>
> >>>>> "The notifier is called from quite a restricted context - oom_reaper -
> >>>>> which shouldn't depend on any locks or sleepable conditionals. The code
> >>>>> should be swift as well but we mostly do care about it to make a forward
> >>>>> progress. Checking for sleepable context is the best thing we could come
> >>>>> up with that would describe these demands at least partially."
> >>>>>
> >>>>
> >>>> Can this be checked for OOM notifier as well?
> >>>>
> >>>>  	if (!is_memcg_oom(oc)) {
> >>>> +		non_block_start();
> >>>>  		blocking_notifier_call_chain(&oom_notify_list, 0, &freed);
> >>>> +		non_block_end();
> >>>>  		if (freed > 0)
> >>>>  			/* Got some memory back in the last second. */
> >>>>  			return true;
> >>>>  	}
> >>>>
> >>>> It is not clear whether i915's oom_notifier function has such dependency.
> >>>
> >>> It is not but then we should be using the non-blocking API if this is
> >>> a real problem. The above code just doesn't make any sense. We have a
> >>> blocking API called and wrapped by non-blocking one.
> >>
> >> OOM notifiers should not depend on any locks or sleepable conditionals.
> >> If some lock directly or indirectly depended on __GFP_DIRECT_RECLAIM,
> >> it will deadlock. Thus, despite blocking API, this should effectively be
> >> non-blocking. All OOM notifier users except i915 seems to be atomic, but
> >> I can't evaluate i915 part...
> > 
> > Read again what I've written, please
> > 
> 
> Question to Daniel: Is i915's oom_notifier function atomic?

It's supposed to not block too much at least, I don't think it's entirely
atomic. Waking up the device (which we need to write some of the ptes)
will take some time and I think acquires a few mutexes, but not 100% sure.

If you want to see, send a patch to intel-gfx m-l and CI will pick it up
and test with our farm of machines.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch