Re: [Xen-devel] [RFC/PATCH v4] XENMEM_claim_pages (subop of existing) hypercall

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.11.12 at 00:52, Dan Magenheimer <dan.magenheimer@xxxxxxxxxx> wrote:
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c

Please put the tools side stuff in a separate second patch, even
if it's very small right now.

> @@ -685,6 +685,37 @@ long do_memory_op(unsigned long cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>          break;
>      }
>  
> +    case XENMEM_claim_pages:
> +        if ( !(IS_PRIV(current->domain)) )

Pointless extra parens around a function call (or equivalent macro).

> +            return -EPERM;
> +
> +        if ( copy_from_guest(&reservation, arg, 1) )
> +            return -EFAULT;
> +
> +        if ( !(guest_handle_is_null(reservation.extent_start)) )

Pointless parens again.

> +            return -EINVAL;
> +
> +        if ( reservation.extent_order != 0 )
> +            return -EINVAL;
> +
> +        rc = rcu_lock_target_domain_by_id(reservation.domid, &d);

With the IS_PRIV() above, this ought to be
rcu_lock_domain_by_any_id() or, if creating claims for itself is
deemed pointless/bogus, rcu_lock_domain_by_id().

> +        if ( rc )
> +            return rc;

break;

> +
> +        rc = domain_set_unclaimed_pages(d, reservation.nr_extents,
> +                                        reservation.mem_flags);
> +
> +        rcu_unlock_domain(d);
> +
> +        break;
> +
> +    case XENMEM_get_unclaimed_pages:
> +        if ( !(IS_PRIV(current->domain)) )

Pointless parens again.

> +            return -EPERM;
> +
> +        rc = get_total_unclaimed_pages();
> +        break;
> +
>      default:
>          rc = arch_memory_op(op, arg);
>          break;
>...
> @@ -443,6 +548,15 @@ static struct page_info *alloc_heap_pages(
>      spin_lock(&heap_lock);
>  
>      /*
> +     * Claimed memory is considered unavailable unless the request
> +     * is made by a domain with sufficient unclaimed pages.
> +     */
> +    if ( (total_unclaimed_pages + request >
> +           total_avail_pages + tmem_freeable_pages()) &&

This line is indented on space too much (should align with the
first char inside the innermost common parens).

Jan

> +          (d == NULL || d->unclaimed_pages < request) )
> +        goto not_found;
> +
> +    /*
>       * TMEM: When available memory is scarce due to tmem absorbing it, allow
>       * only mid-size allocations to avoid worst of fragmentation issues.
>       * Others try tmem pools then fail.  This is a workaround until all



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