* [PATCH] x86/shadow: sh_pagetable_dying() cleanup
@ 2016-06-08 13:17 Jan Beulich
2016-06-09 10:46 ` Tim Deegan
0 siblings, 1 reply; 2+ messages in thread
From: Jan Beulich @ 2016-06-08 13:17 UTC (permalink / raw)
To: xen-devel; +Cc: Tim Deegan
[-- Attachment #1: Type: text/plain, Size: 1588 bytes --]
Don't call shadow_hash_lookup() at all when get_gfn_query_unlocked()
didn't return a valid MFN.
Also no need for local variables used only once, the more with scopes
much wider than their actual use.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Question is whether we shouldn't also get rid of
guest_l[234]e_get_paddr(), as they're now effectively unused.
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -4501,7 +4501,6 @@ static void sh_pagetable_dying(struct vc
p2m_type_t p2mt;
char *gl3pa = NULL;
guest_l3e_t *gl3e = NULL;
- paddr_t gl2a = 0;
unsigned long l3gfn;
mfn_t l3mfn;
@@ -4528,7 +4527,6 @@ static void sh_pagetable_dying(struct vc
}
for ( i = 0; i < 4; i++ )
{
- unsigned long gfn;
mfn_t smfn, gmfn;
if ( fast_path ) {
@@ -4540,10 +4538,11 @@ static void sh_pagetable_dying(struct vc
else
{
/* retrieving the l2s */
- gl2a = guest_l3e_get_paddr(gl3e[i]);
- gfn = gl2a >> PAGE_SHIFT;
- gmfn = get_gfn_query_unlocked(d, gfn, &p2mt);
- smfn = shadow_hash_lookup(d, mfn_x(gmfn), SH_type_l2_pae_shadow);
+ gmfn = get_gfn_query_unlocked(d, gfn_x(guest_l3e_get_gfn(gl3e[i])),
+ &p2mt);
+ smfn = likely(mfn_x(gmfn) != INVALID_MFN)
+ ? shadow_hash_lookup(d, mfn_x(gmfn), SH_type_l2_pae_shadow)
+ : gmfn;
}
if ( mfn_valid(smfn) )
[-- Attachment #2: x86-sh-get-gfn.patch --]
[-- Type: text/plain, Size: 1626 bytes --]
x86/shadow: sh_pagetable_dying() cleanup
Don't call shadow_hash_lookup() at all when get_gfn_query_unlocked()
didn't return a valid MFN.
Also no need for local variables used only once, the more with scopes
much wider than their actual use.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Question is whether we shouldn't also get rid of
guest_l[234]e_get_paddr(), as they're now effectively unused.
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -4501,7 +4501,6 @@ static void sh_pagetable_dying(struct vc
p2m_type_t p2mt;
char *gl3pa = NULL;
guest_l3e_t *gl3e = NULL;
- paddr_t gl2a = 0;
unsigned long l3gfn;
mfn_t l3mfn;
@@ -4528,7 +4527,6 @@ static void sh_pagetable_dying(struct vc
}
for ( i = 0; i < 4; i++ )
{
- unsigned long gfn;
mfn_t smfn, gmfn;
if ( fast_path ) {
@@ -4540,10 +4538,11 @@ static void sh_pagetable_dying(struct vc
else
{
/* retrieving the l2s */
- gl2a = guest_l3e_get_paddr(gl3e[i]);
- gfn = gl2a >> PAGE_SHIFT;
- gmfn = get_gfn_query_unlocked(d, gfn, &p2mt);
- smfn = shadow_hash_lookup(d, mfn_x(gmfn), SH_type_l2_pae_shadow);
+ gmfn = get_gfn_query_unlocked(d, gfn_x(guest_l3e_get_gfn(gl3e[i])),
+ &p2mt);
+ smfn = likely(mfn_x(gmfn) != INVALID_MFN)
+ ? shadow_hash_lookup(d, mfn_x(gmfn), SH_type_l2_pae_shadow)
+ : gmfn;
}
if ( mfn_valid(smfn) )
[-- 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] 2+ messages in thread
* Re: [PATCH] x86/shadow: sh_pagetable_dying() cleanup
2016-06-08 13:17 [PATCH] x86/shadow: sh_pagetable_dying() cleanup Jan Beulich
@ 2016-06-09 10:46 ` Tim Deegan
0 siblings, 0 replies; 2+ messages in thread
From: Tim Deegan @ 2016-06-09 10:46 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
At 07:17 -0600 on 08 Jun (1465370220), Jan Beulich wrote:
> Don't call shadow_hash_lookup() at all when get_gfn_query_unlocked()
> didn't return a valid MFN.
>
> Also no need for local variables used only once, the more with scopes
> much wider than their actual use.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Question is whether we shouldn't also get rid of
> guest_l[234]e_get_paddr(), as they're now effectively unused.
>
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -4501,7 +4501,6 @@ static void sh_pagetable_dying(struct vc
> p2m_type_t p2mt;
> char *gl3pa = NULL;
> guest_l3e_t *gl3e = NULL;
> - paddr_t gl2a = 0;
> unsigned long l3gfn;
> mfn_t l3mfn;
>
> @@ -4528,7 +4527,6 @@ static void sh_pagetable_dying(struct vc
> }
> for ( i = 0; i < 4; i++ )
> {
> - unsigned long gfn;
> mfn_t smfn, gmfn;
>
> if ( fast_path ) {
> @@ -4540,10 +4538,11 @@ static void sh_pagetable_dying(struct vc
> else
> {
> /* retrieving the l2s */
> - gl2a = guest_l3e_get_paddr(gl3e[i]);
> - gfn = gl2a >> PAGE_SHIFT;
> - gmfn = get_gfn_query_unlocked(d, gfn, &p2mt);
> - smfn = shadow_hash_lookup(d, mfn_x(gmfn), SH_type_l2_pae_shadow);
> + gmfn = get_gfn_query_unlocked(d, gfn_x(guest_l3e_get_gfn(gl3e[i])),
> + &p2mt);
> + smfn = likely(mfn_x(gmfn) != INVALID_MFN)
> + ? shadow_hash_lookup(d, mfn_x(gmfn), SH_type_l2_pae_shadow)
> + : gmfn;
I think this is clearer as:
smfn = unlikely(mfn_x(gmfn) == INVALID_MFN)
: _mfn(INVALID_MFN)
? shadow_hash_lookup(d, mfn_x(gmfn), SH_type_l2_pae_shadow);
With that change, Acked-by: Tim Deegan <tim@xen.org>
And yes, we can drop the _get_paddr() functions, if you like.
Cheers,
Tim.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-06-09 10:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-08 13:17 [PATCH] x86/shadow: sh_pagetable_dying() cleanup Jan Beulich
2016-06-09 10:46 ` Tim Deegan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).