newbus integration of MOD_QUIESCE

[Poul-Henning Kamp]

In message <20040714.132847.23012460.imp at bsdimp.com>, "M. Warner Losh" writes:

>: Yes, they can assume that.  There is the footnote that we can have
>: multiple modules in one kld and if one of them vetos the MOD_MODQUIESCE
>: the MOD_UNLOAD may not happen until the user tries to kldunload again
>: or at all.
>
>We should document that they should.

The only way to fix it is to do a three-phase commit and that would
be overkill, that's for sure.

>: Notice that MOD_UNLOAD can also be veto'ed and that we already have
>: this multi-modules in one KLD issue if one of the modules veto
>: MOD_UNLOAD and the others don't.
>
>True.  But a failure of one of them doesn't cause us to call
>MOD_QUIESCE and then fail to call MOD_UNLOAD when it is zero.

Actually we do.  If a kld contains two modules and the second one
refuses MOD_UNLOAD, the first one already completed MOD_UNLOAD and
has destroyed and detached everything whereas the second is still
entirely functional.

>Every single routine will have an extra check?  and what happens if
>the sorry_we_re_closed is set after the check, but before the routine
>returns?  It is unclear how to do this efficiently.  Do I have to lock
>this somehow?  Does this force a global lock for each driver?  It
>sounds very simple, but I think that closing the races here might be
>hard.

This is no different from MOD_UNLOAD warner.  All these problems
exist today already and we can only fix them with proper locking,
newbus cannot solve that problem for us either.

>You are looking at it backwards.  I want to map this button to some
>newbus method.  I also want to map the MOD_QUIESCE to this same newbus
>method if possible to reduce the load on driver writers.  They are the
>same thing.  In the case of the pci hotswap example, only one driver
>would get this message, while on a MOD_QUICESE all device_t's in that
>module would get the quiesce call.

That is not enough.  You need to send the event to "all device_t's
in all modules in the kld".

And the problem with sending the existing instances a newbus event
is that you still need to send another event to stop the probe from
discovering new things before you do the unload.

And then you still need to send MOD_QUIESCE to the module as such
for the case where it doesn't use newbus.

>: How is MOD_QUIESCE any different from MOD_UNLOAD with respect to
>: newbus ?
>
>I've already gone into that: it is a mess now and MOD_QUIESECE makes
>it a little messier and I'm trying to clean up the mess.

I think you are trying to hard to collaps two things which are not
the same.

It's a mess now and MOD_QUIESCE gives us a way to clean up the mess.

>We view the change differently then.  As drivers start to implement
>this, you get different behavior between those that have been
>converted and those that haven't.  Those that have been converted
>kldunload turns into 'unload if not busy' while those that haven't are
>a 'force unload'.  This can lead to different behavior and a bad user
>experience.  While MOD_QUIESCE appears to be optional, having
>different behavior for different modules will be a problem for our
>users.

Yes, that is why I added a lever we could use to make the two different
but equally disirable behaviours coexist, rather than being haphazardly
chosen on a module(-family) by module(-family) basis as they are today.

>: If you want consistent unload behaviour, you need to start out by
>: defining what it should be.
>
>Shouldn't you have done that before this commit?  You can't put that
>back on me since you are the agent of this change.

I did, see:

>: For GEOM we currently refuse unloading if anything is busy.  This is
>: based on the behaviour of unmount which refuses if any vnodes are in use.
>: 
>: This entire thing comes about because sometimes you may want to
>: unload busy GEOM classes, and that's what -f(orce) gives you a
>: lever for.
>:
>: If we have modules which today unload while being in use then I would
>: personally argue that it is a POLA violation, but given the lack of
>: the -f(orce) lever until now, I can fully understand the reason
>: why it got that way.
>
>Yes.  However, we have to fix that.  Suddenly declaring all modules in
>the tree to be doing the wrong thing seems a bit excessive.

I didn't declare a single module as doing the wrong thing, backwards
compatibility is preserved.

>: Hopefully now with -f(orce) we can get a consistent behaviour
>: which IMO should be:
>: 
>: 	"kldunload" fails if the module is in use.
>: 	"kldunload -f" suceeds even if the module is in use.
>
>I agree that's a good goal.

Cool, then at least we agree on one thing :-)

>: >And none of this helps us to unload idle drivers, but I guess that
>: >wasn't your goal.
>: 
>: You mean automatically unload drivers which are not/no longer used ?
>: No, that was nowhere near my goals.  If you mean something else
>: you'll have to explain to me what you meant.
>
>No, that's what I mean.

Could you explain how you want this to work ?  Is it only for device
drivers or also for kld's like the linux-o-lator and GEOM classes ?
Would it only happen when the hardware disappears or would it make
it impossible to preload a device driver and provide the hardware
later ?

>While some of them are true before, this whole thing feels rushed in
>and I'm trying hard to styfle my frustration at the speed it went into
>the tree and move forward to make things work in a sensible way.

There's nothing rushed about this.

-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
phk at FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.