Date: Sat, 05 Sep 2009 02:36:34 -0600 (MDT) From: "M. Warner Losh" <imp@bsdimp.com> To: attilio@freebsd.org Cc: arch@freebsd.org Subject: Re: NEWBUS states Message-ID: <20090905.023634.831786645.imp@bsdimp.com> In-Reply-To: <20090904.172310.-1939841993.imp@bsdimp.com> References: <20090904.161634.-217944108.imp@bsdimp.com> <3bbf2fe10909041546y2b5633e1ue063955568df1a06@mail.gmail.com> <20090904.172310.-1939841993.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <20090904.172310.-1939841993.imp@bsdimp.com>
"M. Warner Losh" <imp@bsdimp.com> writes:
: OK. Let me ponder based on that... It might be better for this round
: of changes to leverage off the device 'flags' field to indicate that
: we're attaching/detaching. This would not break the
: device_is_attached() usage, and would solve the interlock problem
: nicely. While it isn't as aesthetically pleasing as the new states,
: it would allow us to easily MFC it without API/ABI breakage. This
: field surely would be covered by the same set of locks as the state
: field.
:
: I know that there's a good aesthetic argument to be made against this,
: but on the other hand 'compatibility' hacks can violate one's
: aesthetics. We can migrate to a more pleasing state-based model in 9
: and reduce the risk to other code from changing its semantics at this
: late date.
For a version of this hack, see
http://people.freebsd.org/~imp/newbus-flags.diff
This preserves the semantics of the state field as they exist today,
while also providing protection against reentrent code. It also
restores a check for GIANT_HELD to the attach routine, which seems to
have disappeared along the way. While not needed when the locking
does arrive, it is needed today, I believe.
It also has the advantage that the following could easily be added to
catch wayward drivers:
int
device_is_attached(device_t dev)
{
if (dev->flags & DF_TRANSITION)
printf(
"%s called %s while in attach/detach. This is no deprecated.",
device_get_nameunit(dev), __func__);
return (dev->state >= DS_ATTACHED);
}
Or make it a KASSERT later in the 9.x release cycle.
Hope this make it clear what my proposed alternative would be...
Warner
P.S. Yes, I'd rate this code as speculative as the code it replaces
(see my prior posts for this criticism). This is an example of how it
could be done with less impact to the existing code base, a
consideration only because we're in the high 50's of minutes in the
11th hour for the 8.0 release...
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090905.023634.831786645.imp>