Re: [PATCH v2] drm/i915: Fix ref->mutex deadlock in i915_active_wait()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Greg KH (2020-04-11 12:39:57)
> On Fri, Apr 10, 2020 at 07:17:38AM -0700, Sultan Alsawaf wrote:
> > On Fri, Apr 10, 2020 at 11:08:38AM +0200, Greg KH wrote:
> > > On Tue, Apr 07, 2020 at 12:18:09AM -0700, Sultan Alsawaf wrote:
> > > > From: Sultan Alsawaf <sultan@xxxxxxxxxxxxxxx>
> > > > 
> > > > The following deadlock exists in i915_active_wait() due to a double lock
> > > > on ref->mutex (call chain listed in order from top to bottom):
> > > >  i915_active_wait();
> > > >  mutex_lock_interruptible(&ref->mutex); <-- ref->mutex first acquired
> > > >  i915_active_request_retire();
> > > >  node_retire();
> > > >  active_retire();
> > > >  mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); <-- DEADLOCK
> > > > 
> > > > Fix the deadlock by skipping the second ref->mutex lock when
> > > > active_retire() is called through i915_active_request_retire().
> > > > 
> > > > Note that this bug only affects 5.4 and has since been fixed in 5.5.
> > > > Normally, a backport of the fix from 5.5 would be in order, but the
> > > > patch set that fixes this deadlock involves massive changes that are
> > > > neither feasible nor desirable for backporting [1][2][3]. Therefore,
> > > > this small patch was made to address the deadlock specifically for 5.4.
> > > > 
> > > > [1] 274cbf20fd10 ("drm/i915: Push the i915_active.retire into a worker")
> > > > [2] 093b92287363 ("drm/i915: Split i915_active.mutex into an irq-safe spinlock for the rbtree")
> > > > [3] 750bde2fd4ff ("drm/i915: Serialise with remote retirement")
> > > > 
> > > > Fixes: 12c255b5dad1 ("drm/i915: Provide an i915_active.acquire callback")
> > > > Cc: <stable@xxxxxxxxxxxxxxx> # 5.4.x
> > > > Signed-off-by: Sultan Alsawaf <sultan@xxxxxxxxxxxxxxx>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_active.c | 27 +++++++++++++++++++++++----
> > > >  drivers/gpu/drm/i915/i915_active.h |  4 ++--
> > > >  2 files changed, 25 insertions(+), 6 deletions(-)
> > > 
> > > Now queued up, thanks.
> > > 
> > > greg k-h
> > 
> > I'm sorry, I meant the v3 [1]. Apologies for the confusion. The v3 was picked
> > into Ubuntu so that's what we're rolling with.
> 
> Ok, thanks, hopefully now I picked upthe right one...

The patch does not fix a deadlock. Greg, this patch is not a backport of
a bugfix, why is it in stable?
-Chris



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux