[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [For Xen-4.10 PATCH] memory: Re-introduce an erroneously dropped line





On 07/06/17 13:13, Jan Beulich wrote:
On 07.06.17 at 14:04, <punit.agrawal@xxxxxxx> wrote:
Commit 726b737574 makes an unrelated change deleting a line setting the
page from mfn. Although the page variable is not used, it is an
unrelated change. The setting of the page variable was introduced to
match the else part of is_domain_direct_mapped() in populate_physmap().

Re-introduce the missing hunk.

Fixes: 726b737574 ("Avoid excess icache flushes in populate_physmap() before
domain has been created")
Signed-off-by: Punit Agrawal <punit.agrawal@xxxxxxx>
---
 xen/common/memory.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/common/memory.c b/xen/common/memory.c
index 34d2dda8b4..a3cb572530 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -221,6 +221,7 @@ static void populate_physmap(struct memop_args *a)
                 }

                 mfn = gpfn;
+                page = mfn_to_page(mfn);
             }
             else
             {

While I certainly don't mind this being re-added, I'm also not sure
it's worthwhile now that the line is gone, and it's not needed for
anything. I'll let other REST maintainers give their opinions ...

I am not a REST maintainers but I think it would be better to keep the page = mfn_to_page(mfn). This is matching the behavior of the else part where page will always point to first base page.

Indeed we don't use it today, but nothing prevent a future patch to do it and it would be difficult to spot the mismatch...

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.