[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 2 Mar 2022 19:08:11 -0800
From: Dongli Zhang <dongli.zhang@...cle.com>
To: Boris Ostrovsky <boris.ostrovsky@...cle.com>,
xen-devel@...ts.xenproject.org, x86@...nel.org
Cc: linux-kernel@...r.kernel.org, jgross@...e.com,
sstabellini@...nel.org, tglx@...utronix.de, mingo@...hat.com,
bp@...en8.de, dave.hansen@...ux.intel.com, joe.jin@...cle.com
Subject: Re: [PATCH v4 2/2] xen: delay xen_hvm_init_time_ops() if kdump is
boot on vcpu>=32
Hi Boris,
On 3/2/22 6:11 PM, Boris Ostrovsky wrote:
>
> On 3/2/22 7:31 PM, Dongli Zhang wrote:
>> Hi Boris,
>>
>> On 3/2/22 4:20 PM, Boris Ostrovsky wrote:
>>> On 3/2/22 11:40 AM, Dongli Zhang wrote:
>>>> void __init xen_hvm_init_time_ops(void)
>>>> {
>>>> + static bool hvm_time_initialized;
>>>> +
>>>> + if (hvm_time_initialized)
>>>> + return;
>>>> +
>>>> /*
>>>> * vector callback is needed otherwise we cannot receive interrupts
>>>> * on cpu > 0 and at this point we don't know how many cpus are
>>>> * available.
>>>> */
>>>> if (!xen_have_vector_callback)
>>>> - return;
>>>> + goto exit;
>>>
>>> Why not just return? Do we expect the value of xen_have_vector_callback to
>>> change?
>> I just want to keep above sync with ....
>>
>>>
>>> -boris
>>>
>>>
>>>> if (!xen_feature(XENFEAT_hvm_safe_pvclock)) {
>>>> pr_info("Xen doesn't support pvclock on HVM, disable pv timer");
>>>> + goto exit;
>>>> + }
>> ... here.
>>
>> That is, I want the main logic of xen_hvm_init_time_ops() to run for at most
>> once. Both of above two if statements will "go to exit".
>
>
> I didn't notice this actually.
>
>
> I think both of them should return early, there is no reason to set
> hvm_time_initialized to true when, in fact, we have not initialized anything.
> And to avoid printing the warning twice we can just replace it with pr_info_once().
>
>
> I can fix it up when committing so no need to resend. So unless you disagree
Thank you very much for fixing it during committing.
Dongli Zhang
Powered by blists - more mailing lists