linux-kernel - Re: [PATCH] locking/osq: Drop the overload of osq lock

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]

Date:	Sun, 26 Jun 2016 23:11:22 +0800
From:	panxinhui <xinhui@...ux.vnet.ibm.com>
To:	Boqun Feng <boqun.feng@...il.com>
Cc:	panxinhui <xinhui@...ux.vnet.ibm.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Pan Xinhui <xinhui.pan@...ux.vnet.ibm.com>,
	linux-kernel@...r.kernel.org, mingo@...hat.com, dave@...olabs.net,
	will.deacon@....com, Waiman.Long@....com, benh@...nel.crashing.org
Subject: Re: [PATCH] locking/osq: Drop the overload of osq lock


> 在 2016年6月26日，22:29，Boqun Feng <boqun.feng@...il.com> 写道：
> 
> On Sun, Jun 26, 2016 at 03:08:20PM +0800, panxinhui wrote:
> [snip]
>>> @@ -106,6 +109,9 @@ bool osq_lock(struct optimistic_spin_queue *lock)
>>> 	node->prev = prev;
>>> 	WRITE_ONCE(prev->next, node);
>>> 
>>> +	old = old - 1;
>>> +	vpc = vcpu_preempt_count();
>>> +
>>> 	/*
>>> 	 * Normally @prev is untouchable after the above store; because at that
>>> 	 * moment unlock can proceed and wipe the node element from stack.
>>> @@ -118,8 +124,14 @@ bool osq_lock(struct optimistic_spin_queue *lock)
>>> 	while (!READ_ONCE(node->locked)) {
>>> 		/*
>>> 		 * If we need to reschedule bail... so we can block.
>>> +		 * An over-committed guest with more vCPUs than pCPUs
>>> +		 * might fall in this loop and cause a huge overload.
>>> +		 * This is because vCPU A(prev) hold the osq lock and yield out,
>>> +		 * vCPU B(node) wait ->locked to be set, IOW, wait till
>>> +		 * vCPU A run and unlock the osq lock.
>>> +		 * NOTE that vCPU A and vCPU B might run on same physical cpu.
>>> 		 */
>>> -		if (need_resched())
>>> +		if (need_resched() || vcpu_is_preempted(old) || vcpu_has_preempted(vpc))
>>> 			goto unqueue;
>>> 
>> 
>> the prev might change, so we need  read node->prev every loop, then check vcpu preempted.
>> 
> 
> Right you are on the possibility of the prev's change, however, even if
> we reread node->prev, the prev is still not stable after we read, that
> is the prev can change after we read in the loop and before we check the

but the next loops we will detect the owner is preempted, right?

> vcpu preemption in the next loop, therefore whether the reread is

well,think the case below.

3 vcpu  try to get the osq lock. IF you don’t re-read the prev.

A				B 									C	
preempted 	
			detect A preempted, then break loops 
			and do unqueue
											spin loops, because B (the prev ) is running,



			preempted
											detect B preempted then break loops and do queue.// looks like it’ too late to break the loops 
running


SO we can get the rules, 
IF the queue nodes looks like
vcpu A,B,C,D,E,F,G,….. more spinners

say, if vcpu  B,F is preempted, then, we could know vcpu C,D,E should stop the spinning. right? and all other vcpu after F, say G,H,…should stop spinning too.
 

thanks
xinhui

> Regards,
> Boqun
> 
>>> 		cpu_relax_lowlatency();
>>