[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] x86/HVM: clean up HVMOP_set_mem_type processing
On 30/04/14 14:47, Jan Beulich wrote: > - drop unused variable "mfn" > - consistently do not use "else" when the prior "if" ends in "goto" > - use printk() referencing the target domain instead of gdprintk() > (which references the current domain) and slightly shorten message > - annotate -EINVAL results in paging/shared paths to actually need > switching to -EAGAIN (possible only when preemption logic got fixed > to use -ERESTART) > > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> > > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -4522,21 +4522,20 @@ long do_hvm_op(unsigned long op, XEN_GUE > while ( a.nr > start_iter ) > { > unsigned long pfn = a.first_pfn + start_iter; > - p2m_type_t t; > - p2m_type_t nt; > - mfn_t mfn; > - mfn = get_gfn_unshare(d, pfn, &t); > + p2m_type_t t, nt; > + > + get_gfn_unshare(d, pfn, &t); > if ( p2m_is_paging(t) ) > { > put_gfn(d, pfn); > p2m_mem_paging_populate(d, pfn); > - rc = -EINVAL; > + rc = -EINVAL; /* XXX EAGAIN */ > goto param_fail4; > } > if ( p2m_is_shared(t) ) > { > put_gfn(d, pfn); > - rc = -EINVAL; > + rc = -EINVAL; /* XXX EAGAIN */ > goto param_fail4; > } Could you nuke the trailing piece of whitespace on this brace as this is a cleanup patch and it is within-context. > if ( !p2m_is_ram(t) && > @@ -4545,18 +4544,14 @@ long do_hvm_op(unsigned long op, XEN_GUE > put_gfn(d, pfn); > goto param_fail4; > } > - else I know this is inconsistently applied, but a line before setting nt would look neater. > + nt = p2m_change_type(d, pfn, t, memtype[a.hvmmem_type]); > + if ( nt != t ) > { > - nt = p2m_change_type(d, pfn, t, memtype[a.hvmmem_type]); > - if ( nt != t ) > - { > - put_gfn(d, pfn); > - gdprintk(XENLOG_WARNING, > - "type of pfn %#lx changed from %d to %d while " > - "we were trying to change it to %d\n", > - pfn, t, nt, memtype[a.hvmmem_type]); > - goto param_fail4; > - } > + put_gfn(d, pfn); > + printk(XENLOG_G_WARNING > + "d%d: GFN %#lx type changed from %d to %d while > trying to change it to %d\n", > + d->domain_id, pfn, t, nt, memtype[a.hvmmem_type]); > + goto param_fail4; > } > put_gfn(d, pfn); > Other than those two nits, content wise Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |