Re: [PATCH][KVM][retry 1] Add support for Pause Filtering to AMD SVM

[Avi Kivity <avi@redhat.com>]

Ingo Molnar wrote:
>   
>>>  +static int pause_interception(struct vcpu_svm *svm, struct kvm_run 
>>> *kvm_run)
>>> +{
>>> +	/* Simple yield */
>>> +	vcpu_put(&svm->vcpu);
>>> +	schedule();
>>> +	vcpu_load(&svm->vcpu);
>>> +	return 1;
>>> +
>>>       
>> Ingo, will this do anything under CFS, or will CFS note that 
>> nothing has changed in the accounting and reschedule us 
>> immediately?
>>     
>
> The scheduler will yield to another task only if the current task 
> has become ineligible. I.e schedule() is largely a NOP on 
> TASK_RUNNING tasks (i.e. here).
>   

Especially on preemptible kernels, where the schedule() would have 
already happened if it could cause anything, IIUC.

> I.e. this is a somewhat poor solution as far as scheduling goes. But 
> i'm wondering what the CPU side does. Can REP-NOP really take 
> thousands of cycles? If yes, under what circumstances?
>   

The guest is running rep-nop in a loop while trying to acquire a 
spinlock.  The hardware detects this (most likely, repeated rep-nop with 
the same rip) and exits.  We can program the loop count; obviously if 
we're spinning for only a short while it's better to keep spinning while 
hoping the lock will be released soon.

The idea is to detect that the guest is not making forward progress and 
yield.  If I could tell the scheduler, you may charge me a couple of 
milliseconds, I promise not to sue, that would be ideal.  Other tasks 
can become eligible, hopefully the task holding the spinlock, and by the 
time we're scheduled back the long running task will have finished and 
released the lock.

For newer Linux as a guest we're better off paravirtualizing this, so we 
can tell the host which vcpu holds the lock; in this case kvm will want 
to say, take a couple milliseconds off my account and transfer it to 
this task (so called directed yield).  However there's no reason to 
paravirtualize all cpu_relax() calls.

-- 
error compiling committee.c: too many arguments to function