public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Greg KH <greg@kroah.com>,
	Linux Kernel list <linux-kernel@vger.kernel.org>,
	David Woodhouse <dwmw2@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Device core removal ordering brokenness
Date: Sun, 10 May 2009 09:29:21 +1000	[thread overview]
Message-ID: <1241911761.19955.8.camel@pasglop> (raw)

Hi Alan !

I was looking at git history regarding the various BUS_NOTIFY_*
notifiers (since David needs some stuff for his DMA debug code that
isn't provided by the current set) when I noticed that commit of yours:

ec0676ee28528dc8dda13a93ee4b1f215a0c2f9d

Unless I'm mistaken, which is very possible, this moves the
BUS_NOTIFY_DEL_DEVICE callback to -before- the driver remove() callback
is invoked. This sounds very illogical and potentially dangerous to me.

In fact, the original ordering and the only one that, to me, makes sense
in term of semantics is:

ADD / ->probe() / BOUND ... UNBIND / ->remove() / DEL

And not the current (since your patch):

ADD / ->probe() / BOUND ... DEL / UNBIND / ->remove()

IE. The DEL callback might tear down data structures used by the driver,
such as DMA mapping stuff etc...  (In fact, that's pretty much the whole
point of this callback). ADD/DEL should be invoked while no driver is
active on the device.

Now if I look at the reason for your change, I discover what look to me
like added brokenness in the core, but again, I may be missing something
obvious. IE. The addition and removal path don't look symetric to me,
and you moved the DEL callback because in the first place, the core
tears down various things (such as PM or sysfs related data structures)
before the driver is unbound from the device.

Whatever you guys think is the right approach for those sysfs and PM
structures, I do believe that moving around the DEL callback was a
mistake and I can see that becoming an issue on various platforms (if
not already).

Cheers,
Ben.





             reply	other threads:[~2009-05-09 23:34 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-09 23:29 Benjamin Herrenschmidt [this message]
2009-05-10 14:58 ` Device core removal ordering brokenness Alan Stern
2009-05-10 22:17   ` Benjamin Herrenschmidt
2009-05-11 14:02     ` Alan Stern

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=1241911761.19955.8.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=akpm@linux-foundation.org \
    --cc=dwmw2@infradead.org \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox