From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH] x86/mm: drop further relics of translated PV domains
Date: Mon, 12 Jun 2017 11:37:36 +0100 [thread overview]
Message-ID: <f9bc15d7-d811-b437-8327-7b20e5f30bd7@citrix.com> (raw)
In-Reply-To: <593E50B00200007800161AAD@prv-mh.provo.novell.com>
On 12/06/17 07:28, Jan Beulich wrote:
>>>> On 09.06.17 at 19:38, <andrew.cooper3@citrix.com> wrote:
>> On 08/06/17 16:30, Jan Beulich wrote:
>>> For PV domains paging_mode_{refcounts,translate}() are always false as
>>> of commits 4045953527 ("x86/paging: Enforce PG_external == PG_translate
>>> == PG_refcounts") and 92942fd3d4 ("x86/mm: drop
>>> guest_{map,get_eff}_l1e() hooks").
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Thanks.
>
>> There are more cases as well. I will rebase my series over this patch
>> when you commit it, because the extra cases only become obvious after
>> the other cleanup which is still pending.
> Oh, interesting. I'm curious to see what further ones I didn't spot.
There is a pattern in several do_mmuext_op() subops which is:
if ( currd != pg_owner )
rc = -EPERM;
else if ( paging_mode_translate(currd) )
rc = -EINVAL;
This is equivalent to paging_mode_translate(pg_owner).
>
>> One style query though...
>>
>>> @@ -3384,11 +3368,9 @@ long do_mmuext_op(
>>>
>>> if ( op.arg1.mfn != 0 )
>>> {
>>> - if ( paging_mode_refcounts(d) )
>>> - rc = get_page_from_pagenr(op.arg1.mfn, d) ? 0 : -EINVAL;
>>> - else
>>> - rc = get_page_and_type_from_pagenr(
>>> - op.arg1.mfn, PGT_root_page_table, d, 0, 1);
>>> + rc = get_page_and_type_from_pagenr(op.arg1.mfn,
>>> + PGT_root_page_table,
>>> + d, 0, 1);
>> Why do you choose to squash the parameters on the right hand side? For
>> cases like this, the style of the old code is neater IMO.
> I think this alternative style is contrary to general style guidelines,
Which guidelines where?
> and hence I'm trying to eliminate it wherever the result doesn't end
> up being completely unreadable. (Of course this also is a general
> hint to not use overly long function names.)
We should of course try to have shorter names where possible, and one of
my cleanup patches changes pagenr to mfn in this case.
However, I don't think it is sensible to squash everything on the right
hand side.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2017-06-12 10:37 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-08 15:30 [PATCH] x86/mm: drop further relics of translated PV domains Jan Beulich
2017-06-09 17:38 ` Andrew Cooper
2017-06-12 6:28 ` Jan Beulich
2017-06-12 10:37 ` Andrew Cooper [this message]
2017-06-12 10:44 ` Jan Beulich
2017-06-12 10:52 ` Andrew Cooper
2017-06-12 11:19 ` Jan Beulich
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=f9bc15d7-d811-b437-8327-7b20e5f30bd7@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=JBeulich@suse.com \
--cc=xen-devel@lists.xenproject.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).