linux-kernel - Re: [PATCH v2 1/4] locking/mutex: Add waiter parameter to mutex_optimistic

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:	Mon, 15 Feb 2016 18:55:08 -0500
From:	Waiman Long <waiman.long@....com>
To:	Peter Zijlstra <peterz@...radead.org>
CC:	Ingo Molnar <mingo@...hat.com>, <linux-kernel@...r.kernel.org>,
	"Linus Torvalds" <torvalds@...ux-foundation.org>,
	Ding Tianhong <dingtianhong@...wei.com>,
	Jason Low <jason.low2@....com>,
	Davidlohr Bueso <dave@...olabs.net>,
	"Paul E. McKenney" <paulmck@...ibm.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Will Deacon <Will.Deacon@....com>,
	Tim Chen <tim.c.chen@...ux.intel.com>,
	Waiman Long <Waiman.Long@...com>
Subject: Re: [PATCH v2 1/4] locking/mutex: Add waiter parameter to mutex_optimistic_spin()

On 02/12/2016 03:40 PM, Peter Zijlstra wrote:
> On Fri, Feb 12, 2016 at 12:32:12PM -0500, Waiman Long wrote:
>> @@ -358,8 +373,8 @@ static bool mutex_optimistic_spin(struct mutex *lock,
>>   			}
>>
>>   			mutex_set_owner(lock);
>> -			osq_unlock(&lock->osq);
>> -			return true;
>> +			acquired = true;
>> +			break;
>>   		}
>>
>>   		/*
>> @@ -380,7 +395,10 @@ static bool mutex_optimistic_spin(struct mutex *lock,
>>   		cpu_relax_lowlatency();
>>   	}
>>
>> -	osq_unlock(&lock->osq);
>> +	if (!waiter)
>> +		osq_unlock(&lock->osq);
>> +	if (acquired || waiter)
>> +		return acquired;
>>   done:
>>   	/*
>>   	 * If we fell out of the spin path because of need_resched(),
> Is there a reason to not also preempt in the wait-loop? Surely the same
> reason is still valid there too?

The waiter does check for need_sched(). So it will break out of the loop 
and return false in this case. This causes the waiter to loop back and 
goes to sleep if the lock can't be acquired. That is why I don't think 
we need to do another schedule_preempt_disabled() here.

Cheers,
Longman