2012-02-01 16:58:57

by Eric Paris

[permalink] [raw]
Subject: Re: [RFC][PATCH v1 5/9] ima: allocating iint improvements

On Mon, Jan 30, 2012 at 5:14 PM, Mimi Zohar <[email protected]> wrote:
> From: Dmitry Kasatkin <[email protected]>
>

> ?static struct rb_root integrity_iint_tree = RB_ROOT;
> -static DEFINE_SPINLOCK(integrity_iint_lock);
> +static DEFINE_RWLOCK(integrity_iint_lock);
> ?static struct kmem_cache *iint_cache __read_mostly;

Has any profiling been done here? rwlocks have been shown to
actually be slower on multi processor systems in a number of cases due
to the cache line bouncing required. I believe the current kernel
logic is that if you have a short critical section and you can't show
profile data the rwlocks are better, just stick with a spinlock.


2012-02-01 18:46:59

by Kasatkin, Dmitry

[permalink] [raw]
Subject: Re: [RFC][PATCH v1 5/9] ima: allocating iint improvements

On Wed, Feb 1, 2012 at 6:58 PM, Eric Paris <[email protected]> wrote:
> On Mon, Jan 30, 2012 at 5:14 PM, Mimi Zohar <[email protected]> wrote:
>> From: Dmitry Kasatkin <[email protected]>
>>
>
>>  static struct rb_root integrity_iint_tree = RB_ROOT;
>> -static DEFINE_SPINLOCK(integrity_iint_lock);
>> +static DEFINE_RWLOCK(integrity_iint_lock);
>>  static struct kmem_cache *iint_cache __read_mostly;
>
> Has any profiling been done here?   rwlocks have been shown to
> actually be slower on multi processor systems in a number of cases due
> to the cache line bouncing required.  I believe the current kernel
> logic is that if you have a short critical section and you can't show
> profile data the rwlocks are better, just stick with a spinlock.

No, I have not done any profiling.
My assumption was that rwlocks are better when there many readers.
If what you say is true then rwlocks are useless...
With big sections it is necessary to use rw semaphores.

- Dmitry

2012-02-09 09:40:25

by Kasatkin, Dmitry

[permalink] [raw]
Subject: Re: [RFC][PATCH v1 5/9] ima: allocating iint improvements

On Wed, Feb 1, 2012 at 8:46 PM, Kasatkin, Dmitry
<[email protected]> wrote:
> On Wed, Feb 1, 2012 at 6:58 PM, Eric Paris <[email protected]> wrote:
>> On Mon, Jan 30, 2012 at 5:14 PM, Mimi Zohar <[email protected]> wrote:
>>> From: Dmitry Kasatkin <[email protected]>
>>>
>>
>>>  static struct rb_root integrity_iint_tree = RB_ROOT;
>>> -static DEFINE_SPINLOCK(integrity_iint_lock);
>>> +static DEFINE_RWLOCK(integrity_iint_lock);
>>>  static struct kmem_cache *iint_cache __read_mostly;
>>
>> Has any profiling been done here?   rwlocks have been shown to
>> actually be slower on multi processor systems in a number of cases due
>> to the cache line bouncing required.  I believe the current kernel
>> logic is that if you have a short critical section and you can't show
>> profile data the rwlocks are better, just stick with a spinlock.
>
> No, I have not done any profiling.
> My assumption was that rwlocks are better when there many readers.
> If what you say is true then rwlocks are useless...
> With big sections it is necessary to use rw semaphores.
>

Hello,

I and Mimi made performance measurements with rwlocks and spinlocks.
We used kernel compilation with multiple jobs as a test,
because it reads and creates lots of files..

In all cases rwlocks implementation performed better than spinlocks,
but very insignificantly. For example with total compilation time
around 6 minutes, with rwlocks time was 1 - 3 seconds shorter... But
always like that.

So as conclusion I can make, that usage of rwlocks is justified...

Thanks for bringing this up...

> - Dmitry