Re: [RFC PATCH V2 3/8] xen/mem_paging: Convert mem_event_op to mem_paging_op

["Jan Beulich" <JBeulich@suse.com>]

>>> On 18.01.15 at 16:17, <tamas.lengyel@zentific.com> wrote:
> --- a/xen/arch/x86/mm/mem_paging.c
> +++ b/xen/arch/x86/mm/mem_paging.c
> @@ -25,31 +25,31 @@
>  #include <xen/mem_event.h>
>  
>  
> -int mem_paging_memop(struct domain *d, xen_mem_event_op_t *mec)
> +int mem_paging_memop(struct domain *d, xen_mem_paging_op_t *mpc)
>  {
>      if ( unlikely(!d->mem_event->paging.ring_page) )
>          return -ENODEV;
>  
> -    switch( mec->op )
> +    switch( mpc->op )
>      {
>      case XENMEM_paging_op_nominate:
>      {
> -        unsigned long gfn = mec->gfn;
> +        unsigned long gfn = mpc->gfn;
>          return p2m_mem_paging_nominate(d, gfn);

If you fiddle with this, please fix obvious style issues - here, blank
line after declaration(s). Albeit in the particular cases here I wonder
whether the local variable is really helpful.

> --- a/xen/common/mem_event.c
> +++ b/xen/common/mem_event.c
> @@ -474,7 +474,7 @@ int do_mem_event_op(int op, uint32_t domain, void *arg)
>      {
>  #ifdef HAS_MEM_PAGING
>          case XENMEM_paging_op:
> -            ret = mem_paging_memop(d, (xen_mem_event_op_t *) arg);
> +            ret = mem_paging_memop(d, (xen_mem_paging_op_t *) arg);

Afaict the cast is useless and should be dropped rather than replaced.

> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -372,7 +372,7 @@ typedef struct xen_pod_target xen_pod_target_t;
>  #define XENMEM_paging_op_evict              1
>  #define XENMEM_paging_op_prep               2
>  
> -struct xen_mem_event_op {
> +struct xen_mem_paging_op {
>      uint8_t     op;         /* XENMEM_*_op_* */

Especially this comment makes me think this was originally intended
for more than just paging. Are we really determined for this to no
longer be the case? If so, the comment should be updated too.

Jan