Re: [Xen-devel] [PATCH v6 07/11] xen: introduce rangeset_consume_ranges

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.09.17 at 17:29, <roger.pau@xxxxxxxxxx> wrote:
> This function allows to iterate over a rangeset while removing the
> processed regions.
> 
> It will be used by the following patches in order to store memory
> regions in rangesets, and remove them while iterating.

This really only repeats what the first paragraph already says. Instead
you want to state why this is actually needed (to be able to split
processing aiui).

> --- a/xen/common/rangeset.c
> +++ b/xen/common/rangeset.c
> @@ -298,6 +298,34 @@ int rangeset_report_ranges(
>      return rc;
>  }
>  
> +int rangeset_consume_ranges(
> +    struct rangeset *r,
> +    int (*cb)(unsigned long s, unsigned long e, void *, unsigned long *c),
> +    void *ctxt)
> +{
> +    int rc = 0;
> +
> +    write_lock(&r->lock);
> +    while ( !rangeset_is_empty(r) )
> +    {
> +        unsigned long consumed = 0;
> +        struct range *x = first_range(r);
> +
> +        rc = cb(x->s, x->e, ctxt, &consumed);
> +
> +        ASSERT(consumed <= x->e - x->s + 1);
> +        x->s += consumed;
> +        if ( x->s > x->e )
> +            destroy_range(r, x);
> +
> +        if ( rc )
> +            break;
> +    }
> +    write_unlock(&r->lock);
> +
> +    return rc;
> +}

Leaving the rangeset populated in case of error (other than -ERESTART)
looks to be potentially problematic/unexpected. Please at least add a
comment in the header stating this. Perhaps negative vs positive rc
from the callback could be used to direct intended behavior.

> --- a/xen/include/xen/rangeset.h
> +++ b/xen/include/xen/rangeset.h
> @@ -67,6 +67,10 @@ bool_t __must_check rangeset_overlaps_range(
>  int rangeset_report_ranges(
>      struct rangeset *r, unsigned long s, unsigned long e,
>      int (*cb)(unsigned long s, unsigned long e, void *), void *ctxt);
> +int rangeset_consume_ranges(
> +    struct rangeset *r,
> +    int (*cb)(unsigned long s, unsigned long e, void *, unsigned long *c),
> +    void *ctxt);

Indentation.

Jan


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