All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Juergen Gross <jgross@suse.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org,
	x86@kernel.org
Subject: Re: [PATCH 2/2] xen: rename wrong named pfn related variables
Date: Mon, 16 Aug 2021 14:57:53 +0200	[thread overview]
Message-ID: <257be998-be20-407b-144b-db66ca4b5928@suse.com> (raw)
In-Reply-To: <4a5d8b43-7ffa-cbcd-943c-3c459d56e55e@suse.com>

On 16.08.2021 07:25, Juergen Gross wrote:
> On 03.08.21 12:42, Jan Beulich wrote:
>> On 30.07.2021 11:00, Juergen Gross wrote:
>>> On 16.06.21 12:43, Juergen Gross wrote:
>>>> On 16.06.21 11:56, Jan Beulich wrote:
>>>>> On 16.06.2021 09:30, Juergen Gross wrote:
>>>>>> --- a/arch/x86/xen/p2m.c
>>>>>> +++ b/arch/x86/xen/p2m.c
>>>>>> @@ -95,8 +95,8 @@ unsigned long *xen_p2m_addr __read_mostly;
>>>>>>    EXPORT_SYMBOL_GPL(xen_p2m_addr);
>>>>>>    unsigned long xen_p2m_size __read_mostly;
>>>>>>    EXPORT_SYMBOL_GPL(xen_p2m_size);
>>>>>> -unsigned long xen_max_p2m_pfn __read_mostly;
>>>>>> -EXPORT_SYMBOL_GPL(xen_max_p2m_pfn);
>>>>>> +unsigned long xen_p2m_max_size __read_mostly;
>>>>>> +EXPORT_SYMBOL_GPL(xen_p2m_max_size);
>>>>>
>>>>> Instead of renaming the exported variable (which will break consumers
>>>>> anyway), how about dropping the apparently unneeded export at this
>>>>> occasion?
>>>>
>>>> Why do you think it isn't needed? It is being referenced via the inline
>>>> function __pfn_to_mfn() in arch/x86/include/asm/xen/page.h. And
>>>> __pfn_to_mfn() is used via lots of other inline functions and macros.
>>>>
>>>>> Further it looks to me as if xen_p2m_size and this variable
>>>>> were actually always kept in sync, so I'd like to put up the question
>>>>> of dropping one of the two.
>>>>
>>>> Hmm, should be possible, yes.
>>>
>>> Looking into this it seems this is not possible.
>>>
>>> xen_p2m_size always holds the number of p2m entries in the p2m table,
>>> including invalid ones at the end. xen_p2m_pfn_limit however contains
>>> the (rounded up) index after the last valid p2m entry.
>>
>> I'm afraid I can't follow:
>>
>> xen_build_dynamic_phys_to_machine() sets xen_p2m_size and then syncs
>> its value to what so far has been xen_max_p2m_pfn.
>>
>> xen_vmalloc_p2m_tree() sets xen_max_p2m_pfn and then syncs its value
>> to xen_p2m_size.
>>
>> I therefore can't see how the two values would hold different values,
>> except for the brief periods between updating one and then the other.
> 
> The brief period in xen_vmalloc_p2m_tree() is the problematic one. The
> different values are especially important for the calls of
> __pfn_to_mfn() during xen_rebuild_p2m_list().

I'm still in trouble: Such __pfn_to_mfn() invocations would, afaics,
occur only in the context of page directory entry manipulation. Isn't
it the case that all valid RAM is below xen_p2m_size, and hence no
use of

	else if (unlikely(pfn < xen_max_p2m_pfn))
		return get_phys_to_machine(pfn);

would occur during that time window? (We're still way ahead of SMP
init, so what other CPUs might do does not look to be of concern
here.)

Jan


      reply	other threads:[~2021-08-16 12:58 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-16  7:30 [PATCH 0/2] xen: fix max_pfn handling for pv guests Juergen Gross
2021-06-16  7:30 ` [PATCH 1/2] xen: fix setting of max_pfn in shared_info Juergen Gross
2021-06-16  9:52   ` Jan Beulich
2021-06-16 10:37     ` Juergen Gross
2021-06-16 10:56       ` Jan Beulich
2021-06-16 11:18         ` Juergen Gross
2021-06-16  7:30 ` [PATCH 2/2] xen: rename wrong named pfn related variables Juergen Gross
2021-06-16  9:56   ` Jan Beulich
2021-06-16 10:43     ` Juergen Gross
2021-06-16 11:01       ` Jan Beulich
2021-07-30  9:00       ` Juergen Gross
2021-08-03 10:42         ` Jan Beulich
2021-08-16  5:25           ` Juergen Gross
2021-08-16 12:57             ` Jan Beulich [this message]

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=257be998-be20-407b-144b-db66ca4b5928@suse.com \
    --to=jbeulich@suse.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=jgross@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=sstabellini@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    --subject='Re: [PATCH 2/2] xen: rename wrong named pfn related variables' \
    /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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.