lkml.org 
[lkml]   [2018]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 1/4] staging: lustre: obdclass: change spinlock of key to rwlock
On Fri, May 04 2018, Dilger, Andreas wrote:

> On May 3, 2018, at 07:50, David Laight <David.Laight@ACULAB.COM> wrote:
>>
>> From: James Simmons
>>> Sent: 02 May 2018 19:22
>>> From: Li Xi <lixi@ddn.com>
>>>
>>> Most of the time, keys are never changed. So rwlock might be
>>> better for the concurrency of key read.
>>
>> OTOH unless there is contention on the spin lock during reads the
>> additional cost of a rwlock (probably double that of a spinlock)
>> will hurt performance.
>>
>> ...
>>> - spin_lock(&lu_keys_guard);
>>> + read_lock(&lu_keys_guard);
>>> atomic_inc(&lu_key_initing_cnt);
>>> - spin_unlock(&lu_keys_guard);
>>> + read_unlock(&lu_keys_guard);
>>
>> WTF, seems unlikely that you need to hold any kind of lock
>> over an atomic_inc().
>>
>> If this is just ensuring that no code holds the lock then
>> it would need to request the write_lock().
>> (and would need a comment)
>
> There was a fair amount of benchmarking done for this that shows the
> performance is significantly improved with the patch, which can be
> seen in the ticket that was referenced in the original commit comment:
>
> https://jira.hpdd.intel.com/browse/LU-6800?focusedCommentId=121776#comment-121776

That does surprise me. The only places where the lock is held for read
are very short - clearing a few fields or incrementing a value.
But numbers don't lie.
I wonder if the next patch would have had just as big an effect. Taking
and dropping the lock 40 times is not likely to be good for performance.

Thanks,
NeilBrown


>
> That said, it might be good to include this information into the
> commit comment itself.
>
> Cheers, Andreas
> --
> Andreas Dilger
> Lustre Principal Architect
> Intel Corporation
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2018-05-04 02:54    [W:0.450 / U:0.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site