Re: [Xen-devel] [PATCH RFC v1 43/74] xen: introduce rangeset_reserve_hole

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.01.18 at 14:05, <wei.liu2@xxxxxxxxxx> wrote:
> Reserve a hole in a rangeset.

At the end of this operation the new range isn't distinguishable
from a range added by rangeset_add_range(). Hence I don't think
the term "hole" is really appropriate.

> --- a/xen/common/rangeset.c
> +++ b/xen/common/rangeset.c
> @@ -298,6 +298,57 @@ int rangeset_report_ranges(
>      return rc;
>  }
>  
> +int rangeset_reserve_hole(struct rangeset *r, unsigned long size,
> +                          unsigned long *s)

Therefore, how about "rangeset_claim_range()" or
"rangeset_add_dyn_range()"?

> +{
> +    struct range *prev, *next;
> +
> +    *s = 0;

I think it would be better to use a local variable here, and set *s
only on the success path.

> +    write_lock(&r->lock);
> +
> +    for ( prev = NULL, next = first_range(r);
> +          next;
> +          prev = next, next = next_range(r, next) )
> +    {
> +        if ( (next->s - *s) >= size )
> +            goto insert;
> +
> +        if ( next->e == ~0UL )
> +            goto out;
> +
> +        *s = next->e + 1;
> +    }
> +
> +    if ( (~0UL - *s) + 1 >= size )
> +        goto insert;
> +
> + out:
> +    write_unlock(&r->lock);
> +    return -ENOSPC;
> +
> + insert:
> +    if ( !prev )

unlikely()?

> --- a/xen/include/xen/rangeset.h
> +++ b/xen/include/xen/rangeset.h
> @@ -76,6 +76,10 @@ int __must_check rangeset_remove_singleton(
>  bool_t __must_check rangeset_contains_singleton(
>      struct rangeset *r, unsigned long s);
>  
> +/* Reserve a region of the specified size. */
> +int __must_check rangeset_reserve_hole(struct rangeset *r, unsigned long 
> size,
> +                                       unsigned long *s);

I think this would better be placed closer to rangeset_add_range().

Jan


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