On 25/03/2011 17:52, "Dan Magenheimer" <dan.magenheimer@xxxxxxxxxx> wrote:
> Tmem (in Xen) does use rwlocks. After hearing from Jeremy that
> Linux maintainers wouldn't approve of new users of rwlocks, I
> redid the locking structure in the in-Linux-kernel version of tmem
> to avoid using them. I am fairly sure that the same approach used in
> zcache can be used in Xen, but have not tried, and it's likely
> to be a fairly big coding/testing effort that I can't undertake
> right now.
>
> I am also fairly sure that the current Xen tmem locking structure
> is not suitable for switching to normal spinlocks nor RCU,
> but am far from an expert in this area.
Why would a normal spinlock not work?
-- Keir
> Dan
>
>> -----Original Message-----
>> From: Keir Fraser [mailto:keir.xen@xxxxxxxxx]
>> Sent: Friday, March 25, 2011 11:09 AM
>> To: Jan Beulich; xen-devel@xxxxxxxxxxxxxxxxxxx
>> Subject: Re: [Xen-devel] [PATCH] switch rangeset's lock to rwlock
>>
>> I'd rather get rid of rwlocks altogether and use RCU in any cases where
>> we
>> really have contention. Rwlocks don't help unless the read-side
>> critical
>> sections are large enough to amortise the cache ping-pong cost of the
>> locking/unlocking operations. And in Xen we have very few if any
>> significantly sized critical sections.
>>
>> I need to double check, but I believe we have only a couple of rwlock
>> users
>> now, and none of the read-side critical sections are large, so in that
>> case
>> I suggest we switch them to use spinlocks and kill our rwlock
>> implementation.
>>
>> -- Keir
>>
>> On 25/03/2011 16:49, "Jan Beulich" <JBeulich@xxxxxxxxxx> wrote:
>>
>>> As a general library routine, it should behave as efficiently as
>>> possible, even if at present no significant contention is known here.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
>>>
>>> --- a/xen/common/rangeset.c
>>> +++ b/xen/common/rangeset.c
>>> @@ -25,7 +25,7 @@ struct rangeset {
>>>
>>> /* Ordered list of ranges contained in this set, and protecting
>> lock. */
>>> struct list_head range_list;
>>> - spinlock_t lock;
>>> + rwlock_t lock;
>>>
>>> /* Pretty-printing name. */
>>> char name[32];
>>> @@ -103,7 +103,7 @@ int rangeset_add_range(
>>>
>>> ASSERT(s <= e);
>>>
>>> - spin_lock(&r->lock);
>>> + write_lock(&r->lock);
>>>
>>> x = find_range(r, s);
>>> y = find_range(r, e);
>>> @@ -159,7 +159,7 @@ int rangeset_add_range(
>>> }
>>>
>>> out:
>>> - spin_unlock(&r->lock);
>>> + write_unlock(&r->lock);
>>> return rc;
>>> }
>>>
>>> @@ -175,7 +175,7 @@ int rangeset_remove_range(
>>>
>>> ASSERT(s <= e);
>>>
>>> - spin_lock(&r->lock);
>>> + write_lock(&r->lock);
>>>
>>> x = find_range(r, s);
>>> y = find_range(r, e);
>>> @@ -231,7 +231,7 @@ int rangeset_remove_range(
>>> }
>>>
>>> out:
>>> - spin_unlock(&r->lock);
>>> + write_unlock(&r->lock);
>>> return rc;
>>> }
>>>
>>> @@ -243,10 +243,10 @@ int rangeset_contains_range(
>>>
>>> ASSERT(s <= e);
>>>
>>> - spin_lock(&r->lock);
>>> + read_lock(&r->lock);
>>> x = find_range(r, s);
>>> contains = (x && (x->e >= e));
>>> - spin_unlock(&r->lock);
>>> + read_unlock(&r->lock);
>>>
>>> return contains;
>>> }
>>> @@ -259,10 +259,10 @@ int rangeset_overlaps_range(
>>>
>>> ASSERT(s <= e);
>>>
>>> - spin_lock(&r->lock);
>>> + read_lock(&r->lock);
>>> x = find_range(r, e);
>>> overlaps = (x && (s <= x->e));
>>> - spin_unlock(&r->lock);
>>> + read_unlock(&r->lock);
>>>
>>> return overlaps;
>>> }
>>> @@ -274,13 +274,13 @@ int rangeset_report_ranges(
>>> struct range *x;
>>> int rc = 0;
>>>
>>> - spin_lock(&r->lock);
>>> + read_lock(&r->lock);
>>>
>>> for ( x = find_range(r, s); x && (x->s <= e) && !rc; x =
>> next_range(r, x)
>>> )
>>> if ( x->e >= s )
>>> rc = cb(max(x->s, s), min(x->e, e), ctxt);
>>>
>>> - spin_unlock(&r->lock);
>>> + read_unlock(&r->lock);
>>>
>>> return rc;
>>> }
>>> @@ -318,7 +318,7 @@ struct rangeset *rangeset_new(
>>> if ( r == NULL )
>>> return NULL;
>>>
>>> - spin_lock_init(&r->lock);
>>> + rwlock_init(&r->lock);
>>> INIT_LIST_HEAD(&r->range_list);
>>>
>>> BUG_ON(flags & ~RANGESETF_prettyprint_hex);
>>> @@ -403,7 +403,7 @@ void rangeset_printk(
>>> int nr_printed = 0;
>>> struct range *x;
>>>
>>> - spin_lock(&r->lock);
>>> + read_lock(&r->lock);
>>>
>>> printk("%-10s {", r->name);
>>>
>>> @@ -422,7 +422,7 @@ void rangeset_printk(
>>>
>>> printk(" }");
>>>
>>> - spin_unlock(&r->lock);
>>> + read_unlock(&r->lock);
>>> }
>>>
>>> void rangeset_domain_printk(
>>>
>>>
>>>
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>>> http://lists.xensource.com/xen-devel
>>
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home