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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4C738A8E.7050502@redhat.com>
Date:	Tue, 24 Aug 2010 12:02:06 +0300
From:	Avi Kivity <avi@...hat.com>
To:	Gleb Natapov <gleb@...hat.com>
CC:	kvm@...r.kernel.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, mingo@...e.hu,
	a.p.zijlstra@...llo.nl, tglx@...utronix.de, hpa@...or.com,
	riel@...hat.com, cl@...ux-foundation.org, mtosatti@...hat.com
Subject: Re: [PATCH v5 04/12] Provide special async page fault handler when
 async PF capability is detected

  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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/