Re: [KVM PATCH v3 1/3] KVM: fix race in irq_routing logic

From: Gregory Haskins
Date: Tue Oct 27 2009 - 11:02:34 EST


Thanks for this, Paul.

Some questions and statements below.

Paul E. McKenney wrote:
> On Tue, Oct 27, 2009 at 04:02:37PM +0200, Gleb Natapov wrote:
>> On Tue, Oct 27, 2009 at 09:39:03AM -0400, Gregory Haskins wrote:
>
> [ . . . ]
>
>>> standard RCU RSCS, which is what SRCU is designed for. So rather than
>>> inventing an awkward two-phased stack based solution, it's better to
>>> reuse the provided tools, IMO.
>>>
>>> To flip it around: Is there any reason why an SRCU would not work here,
>>> and thus we were forced to use something like the stack-copy approach?
>>>
>> If SRCU has no disadvantage comparing to RCU why not use it always? :)
>
> The disadvantages of SRCU compared to RCU include the following:
>
> 1. SRCU requires that the return value of srcu_read_lock()
> be fed into srcu_read_unlock(). This is usually not a problem,
> but can be painful if there are multiple levels of function
> call separating the two.

Right, and this is simple/neat w.r.t. its usage in irq_routing, so no
problem there.

>
> 2. SRCU's grace periods are about 4x slower than those of RCU.
> And they also don't scale all that well with extremely large
> numbers of CPUs (but this can be fixed when/if it becomes a
> real problem).

The irq_routing update path is extremely infrequent, so this should not
be an issue.

>
> 3. SRCU's read-side primitives are also significantly slower than
> those of RCU.
>

Are the 10ns vs 45ns numbers that I mentioned in my last reply the
proper ballpark? How do these compare to an atomic-op, say an
uncontended spinlock on modern hardware? The assumption is that
srcu_read_lock() should be significantly cheaper than a read-lock(). If
its not, then we might as well use something else, I suppose. But if
its not, I guess you probably wouldn't have bothered to invent it in the
first place ;)

> 4. SRCU does not have a call_srcu(). One could be provided, but
> its semantics would be a bit strange due to the need to limit
> the number of callbacks, given that general blocking is
> permitted in SRCU read-side critical sections. (And it would
> take some doing to convince me to supply an SRCU!)

This is not an issue in our design.

>
> 5. The current SRCU has no reasonable way to implement read-side
> priority boosting, as there is no record of which task
> is read-holding which SRCU.

Given the infrequency of the update path, I do not see this as a problem.

Kind Regards,
-Greg

Attachment: signature.asc
Description: OpenPGP digital signature