Re: [Xen-devel] [PATCH 3/4] xen/watchdog: Drop all locked operations on the watchdog_inuse_map

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.05.19 at 20:28, <andrew.cooper3@xxxxxxxxxx> wrote:
> All modifications to the watchdog_inuse_map happen with d->watchdog_lock held,
> so there are no concurrency problems to deal with.

But concurrency problems can also occur for readers. While
not a problem afaict, dump_domains() actually has such an
example (and hence might be worth mentioning here).

> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -1068,17 +1068,15 @@ static long domain_watchdog(struct domain *d, 
> uint32_t id, uint32_t timeout)
>      }
>      else /* Allocate the next available timer. */
>      {
> -        for ( id = 0; id < NR_DOMAIN_WATCHDOG_TIMERS; id++ )
> -        {
> -            if ( test_and_set_bit(id, &d->watchdog_inuse_map) )
> -                continue;
> -            break;
> -        }
> -        if ( id == NR_DOMAIN_WATCHDOG_TIMERS )
> +        id = ffs(~d->watchdog_inuse_map) - 1;

I'm surprised we have no (universally available) ffz(). I wonder
though whether find_first_zero_bit() wouldn't be the better
choice here anyway, as the result wouldn't be undefined in
case NR_DOMAIN_WATCHDOG_TIMERS grew to 32.

Also while this looks to be logically independent of patch 2, it
doesn't look like it would apply on its own.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel