Re: [PATCH 07/10] generic rwsem: implement down_read_critical() / up_read_critical()

From: Michel Lespinasse
Date: Thu May 20 2010 - 23:36:02 EST

On Wed, May 19, 2010 at 6:21 AM, David Howells <dhowells@xxxxxxxxxx> wrote:
> Michel Lespinasse <walken@xxxxxxxxxx> 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:-/

I think it's not that bad - I mean, the unfairness does not come into
factor here. If you tried to do a regular down_read(), you could also
get preempted on your way to the blocking path. Being preempted on the
way to your critical section after a successful (if unfair) acquire
really is no worse.

The critical section prevents you from blocking on long-latency events
such as disk accesses; being preempted but still runnable is not
nearly as bad.

Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at