Re: [Xen-devel] [PATCH] xen/common: Properly reference count DOMCTL_{, un}pausedomain hypercalls

["Slutz, Donald Christopher" <dslutz@xxxxxxxxxxx>]

On 07/01/14 15:57, Andrew Cooper wrote:
> For safety reasons, c/s 6ae2df93c27 "mem_access: Add helper API to setup
> ring and enable mem_access" has to pause the domain while it performs a set of
> operations.
>
> However without properly reference counted hypercalls, xc_mem_event_enable()
> now unconditionally unpauses a previously paused domain.
>
> To prevent toolstack software running wild, there is an arbitrary limit of 255
> on the toolstack pause count.  This is high enough for several components of
> the toolstack to safely use, but prevents over/underflow of d->pause_count.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: Keir Fraser <keir@xxxxxxx>
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Ian Campbell <ian.campbell@xxxxxxxxxx>
> CC: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> CC: Tim Deegan <tim@xxxxxxx>
>
> ---
>
> This has been functionally tested on x86, and compile tested on both flavours
> of ARM.
>
> Personally I dislike the addition of the spinlock, but I can't find a safe way
> of preventing over/underflow of d->pause_count with an atomic_t
> toolstack_pause_count alone.  At least they are fine-grained locks and
> unlikely to be contended in practice.

atomic_cmpxchg() can be used to safely preventing over/underflow.

For example:


int safe_atomic_inc(int * intp)
{
     int prev, old, new;

     prev = atomic_read(*intp);
     do {
         old = prev;
         new = old + 1;
         if ( new > 255 )
             return 1;
         prev = atomic_cmpxchg(intp, old, new);
     } while ( prev != old );
     return 0;
}

int safe_atomic_dec(int * intp)
{
     int prev, old, new;

     prev = atomic_read(*intp);
     do {
         old = prev;
         new = old - 1;
         if ( new < 0 )
             return 1;
         prev = atomic_cmpxchg(intp, old, new);
     } while ( prev != old );
     return 0;
}


does what you are asking about (adjusted for correct types).  Here is
a quick c prog using them:

#include <stdio.h>

#define atomic_read(x) x
#define atomic_cmpxchg __sync_val_compare_and_swap

int safe_atomic_inc(int * intp)
{
     int prev, old, new;

     prev = atomic_read(*intp);
     do {
         old = prev;
         new = old + 1;
         if ( new > 255 )
             return 1;
         prev = atomic_cmpxchg(intp, old, new);
     } while ( prev != old );
     return 0;
}

int safe_atomic_dec(int * intp)
{
     int prev, old, new;

     prev = atomic_read(*intp);
     do {
         old = prev;
         new = old - 1;
         if ( new < 0 )
             return 1;
         prev = atomic_cmpxchg(intp, old, new);
     } while ( prev != old );
     return 0;
}

int
main(void)
{
     int foo = 0;
     int rc;

     printf("foo=%d\n", foo);
     rc = safe_atomic_dec(&foo);
     printf("rc=%d foo--=%d\n", rc, foo);
     rc = safe_atomic_inc(&foo);
     printf("rc=%d foo++=%d\n", rc, foo);
     foo = 253;
     printf("foo=%d\n", foo);
     rc = safe_atomic_dec(&foo);
     printf("rc=%d foo--=%d\n", rc, foo);
     rc = safe_atomic_inc(&foo);
     printf("rc=%d foo++=%d\n", rc, foo);
     rc = safe_atomic_inc(&foo);
     printf("rc=%d foo++=%d\n", rc, foo);
     rc = safe_atomic_inc(&foo);
     printf("rc=%d foo++=%d\n", rc, foo);
     rc = safe_atomic_inc(&foo);
     printf("rc=%d foo++=%d\n", rc, foo);

     return rc;
}

     -Don Slutz


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel