| Message ID | 20170607120410.14552-1-punit.agrawal@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 07.06.17 at 14:04, <punit.agrawal@arm.com> 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@arm.com> > --- > 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 ... Jan
On 07/06/17 13:13, Jan Beulich wrote: >>>> On 07.06.17 at 14:04, <punit.agrawal@arm.com> 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@arm.com> >> --- >> 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,
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 {
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@arm.com> --- xen/common/memory.c | 1 + 1 file changed, 1 insertion(+)