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

[Avi Kivity]

 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/