From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH] switch rangeset's lock to rwlock Date: Fri, 25 Mar 2011 17:08:44 +0000 Message-ID: References: <4D8CD5BB02000078000386C1@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4D8CD5BB02000078000386C1@vpn.id2.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich , "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org 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" 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 > > --- 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@lists.xensource.com > http://lists.xensource.com/xen-devel