Re: [PATCH linux-2.6 01/04] brsem: implement big reader semaphore

Tejun Heo
2005-09-25T08:54:20Z

Hello, Nick.

Nick Piggin wrote:
> Tejun Heo wrote:
>
>>
>> As I've said in the other reply, I'll first rip off three stage init
>> thing for cpucontrol. That added pretty much complexity to it. And
>> with the weird naming scheme, please tell me how to fix it. I have no
>> problem renaming things.
>>
>
> OK, my criticism of your naming was not constructive so I apologise
> for that. I willll help you with some of those minor issues if we
> establish that your overall design is a a goer.
>

Thanks. :-)

>
>>> What would be wrong with an array of NR_CPUS rwsems? The only
>>> tiny trick you would have to do AFAIKS is have up_read remember
>>> what rwsem down_read took, but that could be returned from
>>> down_read as a token.
>>
>>
>>
>> Using array of rwsems means that every reader-side ops performs
>> (unnecessary) real down and up operations on the semaphore which
>> involve atomic memory op and probably memory barrier. These are
>> pretty expensive operations.
>>
>> What brsem tries to do is implementing rwsem semantics while
>> performing only normal (as opposed to atomic/barrier) intstructions
>> during reader-side operations. That's why all the call_on_all_cpus
>> stuff is needed while write-locking. Do you think avoiding
>> atomic/barrier stuff would be an overkill?
>>
>
> Yes I think so. I think the main problem on modern CPUs is
> not atomic operations as such, but cacheline bouncing.
>
> Without any numbers, I'd guess that your down_read is more
> expensive than mine simply due to touching more cachelines
> and having more branches.

Other than local_bh_disable/enable(), all brsem read ops do are

1. accessing sem->idx
2. calculate per-cpu rcnt address from sem->idx
3. do one branch on the value of per-cpu rcnt
4. inc/dec per-cpu rcnt

So, it does access one more cachline and, yeap, there is one branch
for bh enabling and several more inside local_bh_enable. I'll try to
get some benchmark numbers for comparison.

I'm thinking about adding down_read(&xxx->s_umount) to write(2) and
compare normal rwsem and brsem performance by repeitively writing short
data into a file on a UP machine. Do you have better ideas?

>
> The other thing is simply that you really want your
> synchronization primitives to be as simple and verifiable
> as possible. For example rwsems even recently have had subtle
> memory ordering and other implemntation corner cases, and
> they're much less complex than this brsem.
>
> Nick
>

Thanks.

--
tejun