* [PATCH] x86/HVM: clean up HVMOP_set_mem_type processing
@ 2014-04-30 13:47 Jan Beulich
2014-04-30 14:13 ` Andrew Cooper
2014-05-01 14:50 ` Tim Deegan
0 siblings, 2 replies; 3+ messages in thread
From: Jan Beulich @ 2014-04-30 13:47 UTC (permalink / raw)
To: xen-devel; +Cc: Tim Deegan, Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 2554 bytes --]
- 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@suse.com>
--- 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;
}
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
+ 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);
[-- Attachment #2: x86-HVM-cleanup-set-mem-type.patch --]
[-- Type: text/plain, Size: 2599 bytes --]
x86/HVM: clean up HVMOP_set_mem_type processing
- 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@suse.com>
--- 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;
}
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
+ 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);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/HVM: clean up HVMOP_set_mem_type processing
2014-04-30 13:47 [PATCH] x86/HVM: clean up HVMOP_set_mem_type processing Jan Beulich
@ 2014-04-30 14:13 ` Andrew Cooper
2014-05-01 14:50 ` Tim Deegan
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Cooper @ 2014-04-30 14:13 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser, Tim Deegan
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@suse.com>
>
> --- 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@citrix.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/HVM: clean up HVMOP_set_mem_type processing
2014-04-30 13:47 [PATCH] x86/HVM: clean up HVMOP_set_mem_type processing Jan Beulich
2014-04-30 14:13 ` Andrew Cooper
@ 2014-05-01 14:50 ` Tim Deegan
1 sibling, 0 replies; 3+ messages in thread
From: Tim Deegan @ 2014-05-01 14:50 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser
At 14:47 +0100 on 30 Apr (1398865674), 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@suse.com>
Acked-by: Tim Deegan <tim@xen.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-05-01 14:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-30 13:47 [PATCH] x86/HVM: clean up HVMOP_set_mem_type processing Jan Beulich
2014-04-30 14:13 ` Andrew Cooper
2014-05-01 14:50 ` Tim Deegan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.