Re: [PATCH] switch rangeset's lock to rwlock

[Keir Fraser <keir.xen@gmail.com>]

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@novell.com> 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@novell.com>
> 
> --- 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