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

[Gregory Haskins <gregory.haskins@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2524 bytes --]

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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 267 bytes --]