linux-kernel - Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field

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: <57350E1B.8060304@hpe.com>
Date:	Thu, 12 May 2016 19:13:31 -0400
From:	Waiman Long <waiman.long@....com>
To:	Peter Hurley <peter@...leysoftware.com>
CC:	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>, <linux-kernel@...r.kernel.org>,
	Davidlohr Bueso <dave@...olabs.net>,
	Jason Low <jason.low2@...com>,
	Dave Chinner <david@...morbit.com>,
	Scott J Norton <scott.norton@....com>,
	Douglas Hatch <doug.hatch@....com>
Subject: Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner
 field

On 05/12/2016 05:27 PM, Peter Hurley wrote:
>>> Arguably, this check should be bumped out to the optimistic spin and
>>> reload/check the owner there?
>>>
>>> Or better yet; don't pass the owner in as a parameter at all, but
>>> instead snapshot the owner and check its ownership on entry.
>> That will make the main optimistic spinning loop more complex.
> ??
>
> Simpler.
>
> 	while (rwsem_spin_on_owner(sem)) {
> 		if (rwsem_try_write_lock_unqueued(sem)) {
> 			taken = true;
> 			break;
> 		}
>
> 		if (!sem->owner&&  (need_resched() || rt_task(current)))
> 			break;
>
> 		cpu_relax_lowlatency();
> 	}
>
>
>
>
> bool rwsem_spin_on_owner(struct rw_semaphore *sem)
> {
> 	struct task_struct *owner = READ_ONCE(sem->owner);
>
> 	if (!rwsem_is_writer_owned(owner))
> 		return false;
>
> 	rcu_read_lock();
> 	while (sem->owner == owner) {
> 		....
> 	}
> 	rcu_read_unlock();
> 	
> 	return !rwsem_is_reader_owned(sem->owner);
> }
>

I have been thinking about something like that, but my inclination is to 
make as few changes as possible to the existing patch. I did add a new 
patch to streamline the code as suggested.

Cheers,
Longman