[Intel-gfx] [PATCH 1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-01-10 10:54:33)
> 
> On 10/01/2019 10:47, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-01-10 10:24:09)
> >>
> >> On 09/01/2019 16:42, Chris Wilson wrote:
> >>> If the current process is being killed (it was interrupted with SIGKILL
> >>> or equivalent), it will not make any progress in page allocation and we
> >>> can abort performing the shrinking on its behalf. So we can use
> >>> mutex_lock_killable() instead (although this path should only be
> >>> reachable from kswapd currently).
> >>
> >> kswapd is hopefully not killable so it is not reachable via that route.
> >> But should be via other i915_gem_shrink_all callers. Is it starting to
> >> look like we need to pass some flags to say
> >> normal/interruptible/killable (kswapd/debugfs/?)?
> > 
> > killable is justifiable for all callers, I think, even if SIGKILL may
> > never be delivered. interruptible? Do we want to conceptually fail a
> 
> As long as using mutex_lock_killable doesn't make something killable 
> which otherwise wouldn't be. I have to say I don't know how the details 
> of that work.

I don't think so... (Famous last words.) My understanding was that
signals, not even SIGKILL, would be delivered to kthreads.
mutex_lock_killable doesn't install a signal handler, it just allows the
scheduler to wake up the process should a high priority signal be
delivered.
 
> > kmalloc due to a signal, as that's likely to end up with ENOMEM and not
> > EINTR. (Pretty sure that's not common practice but there's a bit of
> > shrink-unless-killable around.) So I don't think we need to make
> > normal aka uninterruptible a special case, and returning before
> > shrinking on any signal seems unexpected.
> 
> debugfs was the only reason I considered interruptible. There I think 
> makes sense to allow bail up. I hate stuck shell sessions at least so 
> anything which can be done to avoid them is tempting.

I see. killable is an improvement for debugfs, not much of one but still
infinitely better ;)
-Chris