linux-kernel - Re: [PATCH 07/10] generic rwsem: implement down_read_critical() / up_read

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:	Wed, 19 May 2010 14:21:23 +0100
From:	David Howells <dhowells@...hat.com>
To:	Michel Lespinasse <walken@...gle.com>
Cc:	dhowells@...hat.com,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mike Waychison <mikew@...gle.com>,
	Suleiman Souhlal <suleiman@...gle.com>,
	Ying Han <yinghan@...gle.com>
Subject: Re: [PATCH 07/10] generic rwsem: implement down_read_critical() / up_read_critical()

Michel Lespinasse <walken@...gle.com> wrote:

> +void __sched down_read_critical(struct rw_semaphore *sem)
> +{
> +	might_sleep();
> +	rwsem_acquire_read(&sem->dep_map, 0, 0, _RET_IP_);
> +
> +	LOCK_CONTENDED(sem, __down_read_trylock, __down_read_unfair);
> +
> +	preempt_disable();

Shouldn't preemption really be disabled before __down_read_unfair() is called?
Otherwise you can get an unfair read on a sem and immediately get taken off
the CPU.  Of course, this means __down_read_unfair() would have to deal with
that in the slow path:-/

Oh, and something else that occurs to me:  Do unfair readers have to go at the
front of the wakeup queue?  Can they be slightly less unfair and go either
before the first reader in the queue or at the back of the queue instead?

David
--
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/