Re: [PATCH v5 04/12] Provide special async page fault handler when async PF capability is detected

Avi Kivity
2010-08-24T09:03:17Z

On 08/24/2010 10:31 AM, Gleb Natapov wrote:
> +
>>> +static void apf_task_wait(struct task_struct *tsk, u32 token)
>>> +{
>>> + u32 key = hash_32(token, KVM_TASK_SLEEP_HASHBITS);
>>> + struct kvm_task_sleep_head *b =&async_pf_sleepers[key];
>>> + struct kvm_task_sleep_node n, *e;
>>> + DEFINE_WAIT(wait);
>>> +
>>> + spin_lock(&b->lock);
>>> + e = _find_apf_task(b, token);
>>> + if (e) {
>>> + /* dummy entry exist -> wake up was delivered ahead of PF */
>>> + hlist_del(&e->link);
>>> + kfree(e);
>>> + spin_unlock(&b->lock);
>>> + return;
>>> + }
>>> +
>>> + n.token = token;
>>> + n.cpu = smp_processor_id();
>> What's the meaning of cpu? Won't the waiter migrate to other cpus?
> Waiter cannot migrate to other cpu since it is sleeping. It may be
> scheduled to run on any cpu when it will be waked.

What if you have a spurious wakeup? Also, nothing prevents the
scheduler from migrating the thread even if it is sleeping. It may not
do so now, but it might do it in the future.

Oh, it probably does now on cpu hotunplug.

Why do you need n.cpu?

>>> + spin_unlock(&b->lock);
>>> + cpu_relax();
>>> + goto again;
>>> + }
>> The other cpu might be waiting for us to yield. We can fix it later
>> with the the pv spinlock infrastructure.
>>
> This busy wait happens only if (very small) allocation fails, so if
> a guest ever hits this code path I expect it to be on his way to die
> anyway.

Hm. I don't have a good feel on how rare atomic allocation failures are
on common workloads.

Note a kmem_cache for apfs will make failures even more rare.

>> Or, we can avoid the allocation. If at most one apf can be pending
>> (is this true?), we can use a per-cpu variable for this dummy entry.
>>
> We can have may outstanding apfs.

But, while we're processing an apf, we can't take any more.

So we can have a buffer of one pre-allocated entry per cpu, and do
something like:

apf:
disable apf for this cpu
handle apf using buffered entry
enable interrupts
allocate new entry
buffer it
enable apf for that cpu

this trades off a bigger apf disabled window for not busy looping.

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