From: Punit Agrawal <punit.agrawal@arm.com>
To: Julien Grall <julien.grall@arm.com>
Cc: sstabellini@kernel.org, wei.liu2@citrix.com,
George.Dunlap@eu.citrix.com,
Andrew Cooper <andrew.cooper3@citrix.com>,
tim@xen.org, xen-devel@lists.xen.org, jbeulich@suse.com,
ian.jackson@eu.citrix.com
Subject: Re: [For Xen-4.10 PATCH v2 3/3] Avoid excess icache flushes in populate_physmap() before domain has been created
Date: Wed, 07 Jun 2017 12:41:17 +0100 [thread overview]
Message-ID: <87zidk10v6.fsf@e105922-lin.cambridge.arm.com> (raw)
In-Reply-To: <a3de5dac-5f8f-1ff4-533e-6573c9d14e53@arm.com> (Julien Grall's message of "Wed, 7 Jun 2017 12:32:25 +0100")
Julien Grall <julien.grall@arm.com> writes:
> On 07/06/17 12:19, Andrew Cooper wrote:
>> On 26/05/17 12:14, Punit Agrawal wrote:
>>> @@ -211,7 +221,6 @@ static void populate_physmap(struct memop_args *a)
>>> }
>>>
>>> mfn = gpfn;
>>> - page = mfn_to_page(mfn);
>>
>> What is the purpose of this hunk?
>>
>> It is not mentioned in the commit message at all, and looks unsafe to me.
>
> Not answering why it has been dropped (leave Punit to answer this), I
> introduced this page = mfn_to_page(...) to match the else part of (
> is_domain_direct_mapped). In the else, 'page' will always point to
> first base page.
>
> However, today, nobody is using 'page' after. Unless there is a reason
> to drop it, I would prefer to keep it so the if and else part match.
It's an unintended change that's crept into this patch. There's no
reason to drop it here.
I'll send a fix adding it back.
>
> Cheers,
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2017-06-07 11:41 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-26 11:14 [For Xen-4.10 PATCH v2 0/3] Reduce unnecessary icache maintenance operations Punit Agrawal
2017-05-26 11:14 ` [For Xen-4.10 PATCH v2 1/3] Allow control of icache invalidations when calling flush_page_to_ram() Punit Agrawal
2017-05-26 11:14 ` [For Xen-4.10 PATCH v2 2/3] arm: p2m: Prevent redundant icache flushes Punit Agrawal
2017-05-26 11:14 ` [For Xen-4.10 PATCH v2 3/3] Avoid excess icache flushes in populate_physmap() before domain has been created Punit Agrawal
2017-05-26 11:51 ` Jan Beulich
2017-06-07 8:38 ` Jan Beulich
2017-06-07 8:59 ` Punit Agrawal
2017-06-07 9:34 ` [For Xen-4.10 PATCH] Ensure invalidate_icache() definition is visible only when !__ASSEMBLY__ Punit Agrawal
2017-06-07 10:37 ` Jan Beulich
2017-06-07 11:19 ` [For Xen-4.10 PATCH v2 3/3] Avoid excess icache flushes in populate_physmap() before domain has been created Andrew Cooper
2017-06-07 11:32 ` Julien Grall
2017-06-07 11:41 ` Punit Agrawal [this message]
2017-06-07 12:04 ` [For Xen-4.10 PATCH] memory: Re-introduce an erroneously dropped line Punit Agrawal
2017-06-07 12:13 ` Jan Beulich
2017-06-07 12:16 ` Julien Grall
2017-06-06 16:32 ` [For Xen-4.10 PATCH v2 0/3] Reduce unnecessary icache maintenance operations Julien Grall
2017-06-06 16:36 ` Jan Beulich
2017-06-06 18:51 ` Stefano Stabellini
2017-06-07 9:46 ` Punit Agrawal
2017-06-07 17:46 ` Stefano Stabellini
2017-06-07 17:47 ` Julien Grall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zidk10v6.fsf@e105922-lin.cambridge.arm.com \
--to=punit.agrawal@arm.com \
--cc=George.Dunlap@eu.citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=ian.jackson@eu.citrix.com \
--cc=jbeulich@suse.com \
--cc=julien.grall@arm.com \
--cc=sstabellini@kernel.org \
--cc=tim@xen.org \
--cc=wei.liu2@citrix.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).