David Kahn wrote:
> A couple of replies in one email:
>
>
>
> Garrett D'Amore wrote:
>> I requested that the team not create a new DDI_QUIESCE command. The
>> expansion of detach(9e) and attach(9e) with new interfaces was, IMO,
>> a big mistake. The biggest problem is that it is impossible to tell
>> (at least easily) if a driver supports the new interfaces or not. By
>> adding a new devops entry point, it becomes nothing more than a
>> matter of routine inspection. (The current design is one of the
>> major limiting factors in suspend-to-ram. There is no predictability
>> about whether suspend will work or not -- the only way to know is to
>> actually try it.)
>
> I guess inspection is a good reason to create a new entry in dev_ops,
> but it's still not a guarantee that quiesce(9e) will succeed.
Yes. But in the case of suspend/resume, we could have been a lot
*smarter* here. I wouldn't mind have a new flag that drivers could
supply in devops that says "quiesce *always* succeeds". In fact, IMO,
there should be *no* reason for quiesce to *ever* fail -- short of a
fatal problem in the hardware. Maybe it should have been specified this
way?
That does raise a possible question, which is that in the event of an
FMA detected fault condition, the hardware may not be able to quiesce
properly, and for sanity's sake, a full power-cycle is probably
required. Does the project team take hardware FMA status into
consideration when deciding whether to fast reboot or not?
>
> You could require the driver to export a property if it supports
> quiesce(9e) and get the same effect you are looking for without
> expanding dev_ops if you wanted to.
Yes, you could. Properties (as opposed to dev ops flags) are a PITA.
>
> There's nothing wrong with expanding dev_ops, but we don't have
> to expand it if we don't want to.
I've (for a long time now) wanted to break SUSPEND/RESUME out of
attach/detach processing -- its entirely unpredictable right now.
My ideal world is:
* all drivers export (suspend/resume/quiesce).
* those entry points are assumed to always succeed/never fail
* a driver that actually may have a failure condition in suspend (and
possibly in quiesce) can set a dev ops flag indicating this
The point is, I would rather not have to look beyond the dev_ops routine
to learn whether a function is likely to succeed or not.
>
> Sherry Moore wrote:
>> Hi David,
>>
>>> Specifically, can open(9e) or any other entry point be called
>>> while the driver is in quiesce(9e)? I would think we would want
>>> to guard against that, and if not, the driver needs to know that.
>>> If the intent is that nothing else will be called while the
>>> driver is in quiesce(9e), please state that guarantee in the
>>> man page.
>>
>> Technically, yes; architecturally no. Since we're basically at the
>> same place as we would be when the reset() entry points are called, the
>> same sort of assumption hold here also.
>>
>> I will add the following to the man page:
>>
>> "open(9E) and other entry points should not be called while the driver
>> is in quiesce(9E)."
>
> I think the man page needs to provide a guarantee that the driver
> does not have to guard against open(9e) and close(9e) being called
> once the framework calls quiesce(9e), so that has to be worded
> appropriately. (Not a "should" for the implementation, but written
> as a normative requirement that the driver can rely on.)
>
> Something like:
>
> open(9e), close(9e), attach(9e), detach(9e) and other driver entry
> points shall not be called when quiesce(9e) is called.
>
> I'm not sure if that's enough. What if open, close, etc are in
> progress when the framework decides to call quiesce. Is that
> possible? So maybe it has to be worded even stronger such that
> the guarantee is that open, close, attach, detach and other driver
> entry points will not be active or called when the framework calls
> quiesce(9e). That still leaves the question of interrupt handlers,
> timers, callbacks, etc that could be called when the driver is
> in quiesce(9e) or might be active when the quiesce(9e) is called.
>
> This needs a bit of work, IMO.
This sound less architectural, and more wordsmithing. I won't disagree
that the docs can be cleaned up, made clearer....
>
>
>>
>>> Second, are there open instances of the driver when quiesce(9e)
>>> is called, or does the framework guarantee to close all open
>>> instances of the device first?
>>
>> Yes, as long as the driver can guarantee that no more asynchronous
>> writes occur to system memory as operation to the open instances.
>>
>> I will add that to the man page as well.
>
> Bear with me on this. A driver has scheduled the device to do
> a bunch of io by handing off a list of requests to the device.
> The driver probably uses completion interrupts or doorbells to
> the application directly to signal completions or errors from
> the device. Nothing has been done to tell the driver to stop
> doing that when we call quiesce(9e), so the device may still be
> writing to memory and sending interrupts. Will those interrupts
> be taken when the driver is in quiesce(9e)? If so, that's why
> the driver needs some sort of lock to guard against race conditions
> with it's interrupt handler. (Also, timers, callbacks, etc. Basically
> any interface that can asynchronously call into the driver while
> any other entry point is active.)
Ah, but what I'm saying is that the kernel should have disabled all
interrupts from occurring at the point that quiesce is called. So those
events, which may occur in hardware, won't trigger an ISR execution. No
locking is required.
>
> There's always the possibility of a race inside the driver if any
> interrupt can be taken when we start to call quiesce(9e).
See above. Interrupts should be disabled.
>
> Also, there's timer interrupts, soft interrupts, etc. Basically
> any sort of async event that the driver setup before quiesce(9e)
> was called.
>
> This needs some work also.
Again, no interrupts, totally single threaded... etc.
-- Garrett
