All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Trippelsdorf <markus@trippelsdorf.de>
To: "Deucher, Alexander" <Alexander.Deucher@amd.com>
Cc: Peter Chubb <peter.chubb@nicta.com.au>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: Re: Can no longer shutdown after  drm/radeon: Implement radeon_pci_shutdown
Date: Thu, 12 Dec 2013 14:56:14 +0100	[thread overview]
Message-ID: <20131212135614.GA273@x4> (raw)
In-Reply-To: <A3397C8B8B789E45844E7EC5DEAD89D030AA1B12@satlexdag05.amd.com>

On 2013.12.12 at 03:27 +0000, Deucher, Alexander wrote:
> > On 2013.12.11 at 23:46 +0000, Deucher, Alexander wrote:
> > > > -----Original Message-----
> > > > From: Peter Chubb [mailto:peter.chubb@nicta.com.au]
> > > > Sent: Wednesday, December 11, 2013 5:11 PM
> > > > To: Markus Trippelsdorf
> > > > Cc: Peter Chubb; Deucher, Alexander; airlied@linux.ie; dri-
> > > > devel@lists.freedesktop.org
> > > > Subject: Re: Can no longer shutdown after drm/radeon: Implement
> > > > radeon_pci_shutdown
> > > >
> > > > >>>>> "Markus" == Markus Trippelsdorf <markus@trippelsdorf.de>
> > writes:
> > > >
> > > > Markus> On 2013.12.11 at 11:37 +1100, Peter Chubb wrote:
> > > >
> > > > Markus> It would be interesting to know where exactly it hangs.  Could
> > > > Markus> you comment out the *_fini(rdev) calls in
> > > > Markus> radeon_driver_unload_kms
> > > > (drivers/gpu/drm/radeon/radeon_kms.c)
> > > > Markus> one after the other to find out which one is responsible?
> > > >
> > > > It's radeon_device_fini() that is the problem.
> > >
> > > I think the problem is that the drm subsystem tears down the device
> > > via drm_driver.unload in drm_dev_unregister(), but now that we have a
> > > pci_driver.shutdown callback (which is needed for kexec) that gets
> > > called too so the driver gets torn down twice.
> > 
> > If that is the case the following patch should fix the issue.
> > Can you give it a try, Peter?
(Peter:)
> Thanks that works.  I tested shutdown, kexec, and s2disk --- all work
> correctly.

> That may work, but I think it's just papering over a race which may
> still bite someone else depending on the timing.

This leaves three possibilities:

1) Revert 846ae41ae99d now and come up with a solution with proper
locking for 3.14
2) Add my simple fix now and implement additional locking if the need
arises for 3.14.
3) Implement a fix with proper locking now.

It's your choice Alex.

-- 
Markus

  reply	other threads:[~2013-12-12 13:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-11  0:37 Can no longer shutdown after drm/radeon: Implement radeon_pci_shutdown Peter Chubb
2013-12-11  7:26 ` Markus Trippelsdorf
2013-12-11 22:11   ` Peter Chubb
2013-12-11 23:46     ` Deucher, Alexander
2013-12-12  0:04       ` Peter Chubb
2013-12-12  2:58       ` Markus Trippelsdorf
2013-12-12  3:10         ` Peter Chubb
2013-12-12  3:27         ` Deucher, Alexander
2013-12-12 13:56           ` Markus Trippelsdorf [this message]
2013-12-12 14:11             ` Alex Deucher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131212135614.GA273@x4 \
    --to=markus@trippelsdorf.de \
    --cc=Alexander.Deucher@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=peter.chubb@nicta.com.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.