Does CONFIG_PARAVIRT imply usage of byte locks?

Does CONFIG_PARAVIRT imply usage of byte locks?

[Jiri Kosina]

Hi,

in [1] Linus states that CONFIG_PARAVIRT implies usage of inferior locks.

Looking at the code, I wonder whether are we in fact really using byte 
locks in CONFIG_PARAVIRT situation? Where are we actually setting 
pv_lock_ops.spin_lock pointer to point to __byte_spin_lock?

Such initialization seems to happen only in paravirt_use_bytelocks()
function, but my blind eyes prevent me from finding a callsite from which
this function would eventually get called.

It seems to me that paravirt_use_bytelocks() is a dead code that gets 
never called, and the same applies to the implementations of write locks. 
What did I miss?

[1] http://marc.info/?l=linux-kernel&m=123144211719754&w=2

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: Does CONFIG_PARAVIRT imply usage of byte locks?

[Jiri Kosina]

On Fri, 9 Jan 2009, Jiri Kosina wrote:

> It seems to me that paravirt_use_bytelocks() is a dead code that gets 
> never called, and the same applies to the implementations of write locks. 

	s/of write locks/of byte locks

of course, sorry.

-- 
Jiri Kosina
SUSE Labs

Re: Does CONFIG_PARAVIRT imply usage of byte locks?

[Jeremy Fitzhardinge]

Jiri Kosina wrote:
> Hi,
>
> in [1] Linus states that CONFIG_PARAVIRT implies usage of inferior locks.
>   

Yes, he's wrong about that.  In the normal case, the CONFIG_PARAVIRT 
case will continue to use the ticket-lock algorithm.

> Looking at the code, I wonder whether are we in fact really using byte 
> locks in CONFIG_PARAVIRT situation? Where are we actually setting 
> pv_lock_ops.spin_lock pointer to point to __byte_spin_lock?
>
> Such initialization seems to happen only in paravirt_use_bytelocks()
> function, but my blind eyes prevent me from finding a callsite from which
> this function would eventually get called.
>
> It seems to me that paravirt_use_bytelocks() is a dead code that gets 
> never called, and the same applies to the implementations of write locks. 
> What did I miss?
>   

Probably nothing.  Xen has its own optimised spinlock implementation, 
and so doesn't use the plain byte locks.  But ticket locks are awesomely 
bad for any virtual environment which doesn't gang schedule VCPUs, so 
the kvm folks would do well to at least consider using them.

    J

Re: Does CONFIG_PARAVIRT imply usage of byte locks?

[Jiri Kosina]

On Sun, 11 Jan 2009, Jeremy Fitzhardinge wrote:

> > Looking at the code, I wonder whether are we in fact really using byte 
> > locks in CONFIG_PARAVIRT situation? Where are we actually setting 
> > pv_lock_ops.spin_lock pointer to point to __byte_spin_lock?
> > 
> > Such initialization seems to happen only in paravirt_use_bytelocks()
> > function, but my blind eyes prevent me from finding a callsite from which
> > this function would eventually get called.
> > 
> > It seems to me that paravirt_use_bytelocks() is a dead code that gets never
> > called, and the same applies to the implementations of write locks. What did
> > I miss?
> 
> Probably nothing.  Xen has its own optimised spinlock implementation, and so
> doesn't use the plain byte locks.  But ticket locks are awesomely bad for any
> virtual environment which doesn't gang schedule VCPUs, so the kvm folks would
> do well to at least consider using them.

Avi, are there any plans to make KVM use byte locks implementation for 
spinlocks in the near future?

If not, we should remove them completely, as it's completely dead code.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: Does CONFIG_PARAVIRT imply usage of byte locks?

[Avi Kivity]

Jiri Kosina wrote:
> Avi, are there any plans to make KVM use byte locks implementation for 
> spinlocks in the near future?
>   

No, although I note anything coming out of the spinning mutex discussion 
could be applied towards a sleeping spinlock for paravirt.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.