Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Subject: Re: [PATCH v4 2/2] xen: delay xen_hvm_init_time_ops() if kdump is
 boot on vcpu>=32
To:     Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        xen-devel@lists.xenproject.org, x86@kernel.org
Cc:     linux-kernel@vger.kernel.org, jgross@suse.com,
        sstabellini@kernel.org, tglx@linutronix.de, mingo@redhat.com,
        bp@alien8.de, dave.hansen@linux.intel.com, joe.jin@oracle.com
References: <20220302164032.14569-1-dongli.zhang@oracle.com>
 <20220302164032.14569-3-dongli.zhang@oracle.com>
 <bacae45c-9f8e-456b-1036-7f68e1e4374d@oracle.com>
 <0ac50fb6-0aca-e61a-1f38-8badbc8c967a@oracle.com>
 <272a69ec-4cc8-f282-8499-88b24b61f130@oracle.com>
From:   Dongli Zhang <dongli.zhang@oracle.com>
Message-ID: <a40326c0-bdad-da76-9271-61bd8f3fa436@oracle.com>
Date:   Wed, 2 Mar 2022 19:08:11 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.1
In-Reply-To: <272a69ec-4cc8-f282-8499-88b24b61f130@oracle.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TjhTYVlZQkZVYS9pZjlGdWdtT3R2V01XUm5YQkFNZEVQTGhaMHNEcG1DMC9w?=
 =?utf-8?B?TkFONm1qenVRWHFSNXRmZWsrWElZRW9BamlRM0NSNHQ0eGRkZmlkaEQ5S01I?=
 =?utf-8?B?RTY4QzFWUktzdXJhYnJ0YmNyVlUrdHpVWXdTZkFZQnk3ZXBES3VTM3dvc3JL?=
 =?utf-8?B?ck82aWM4Y1lSejEyUDFNSmpkZHdvWjl1OC8wOGlpcHk0Y1lJZEpXUkhmc1c5?=
 =?utf-8?B?Tk9lYUdFWnlkWnUyazBsRW9Ua1YxSlFFQ1k1dW9laWZQV0RTa0xJaE1wR3ND?=
 =?utf-8?B?Qjk0cTlxQVlLenNhekVCdUhFbWdnNGxKdk9oa0svaHFsUnNSc2dvYWdDRFJV?=
 =?utf-8?B?TmhGSjhzNWVJTzl4TWRRYmVwNXVpcjZMaTh1bEkzbGVTS1NLS0dzajdhNGpl?=
 =?utf-8?B?Um55TDkrYlJybDJwWDlRelZQTDBZNXJOUnZGVXo4cnFRdnJBMEN4OGZUSEsv?=
 =?utf-8?B?b1pwTEM0c3ZLNWp5cTUxZ0FIdmZQVVZ3QWVYZ0RkemVhVWVhQ2IxREJkc3JF?=
 =?utf-8?B?YXpxTUYyWldQK2twUU8yOCtzNHRlRXJCdVp0U2NVaTlVcW1reGh5M0g5eWxQ?=
 =?utf-8?B?dS9kV21FVk0vTGlxajMzNmQvUkVrMFdmNlZvdDJEVW01SHkvenRFa09uODRx?=
 =?utf-8?B?Q2VsYVEvSkppcFJ1WWFoT1pRZVZHeHBPbk56VkU1NUVZSmQ0enB0VVZPQlF5?=
 =?utf-8?B?OHozV05DNnNoWDMwdWluZG1WVm5qejZOZjRtc3ZXVDVhdURUaG9UVFlrVDdn?=
 =?utf-8?B?WWdiVHFobzMwSDc3Q2F3ZTJENEM2ZlhmZS9UOThrZmJkdy8vT0pEdWNTcmx5?=
 =?utf-8?B?aWxLOFp0VHdRUjZYUm42cHVXVFRKdVRXRVI2UnpHWE55UEcxZHY1YkpsYkdq?=
 =?utf-8?B?U1BwTDRDQXVNVTJQcndaekZ5Ri9IWWJISHpycjA1S0Zac240MXorL0pGeGlS?=
 =?utf-8?B?bTBHdmlCc2V1SmdNUXZ6WTZGcjNWNFl0YVRyaWtsYWY2SWh6ZGpaTmF5cEUy?=
 =?utf-8?B?UW5MSDh4WmlyRENlMjBpai84N1lHZ2lzV2x4NE5ZSDRoODVHTmdKWElRNlZI?=
 =?utf-8?B?UTFrWXBZa0ZlU1c0R1ZhSEU0K1pVblFBSFhsb1VJbkZmWDBYRUlheXRDVmEw?=
 =?utf-8?B?TDVXWHFwSXlDMHRmWEs5SW5FdnhFY3FOUmdTbE1QUzlUQ3hTdU15NlB2Mytt?=
 =?utf-8?B?YVNZSHRzcll3ZjEyMGFKMkttdE5ycG9iTlhnRGF4VUJXaVdPeEJpSHUwLzZL?=
 =?utf-8?B?azJCOTB4OG9KMlpZelFzTGxPS2RreDVwcTVwRmVhSzJWUW5uWWFLNE84QTA3?=
 =?utf-8?B?QnUrdXNEY1NuMmhsdmVwWFhNUnNERnFJeFB2N2k4NXU2WUpaQjBDN1AzSjVR?=
 =?utf-8?B?b2xNY3pueGdXMi9Nb1ExVzRhd2FqazQ1akc3SUhjcXRjaFBLbjhKZmZienhv?=
 =?utf-8?B?YVkwOTVRaFVZOEVsZkJjd0lETk0raE1YR1BYNVhybmNERjY4NDdBY00xOXFQ?=
 =?utf-8?B?SUtDWkVNSXRXMTlsZ3UxOGdmRkYzTmF1OUdxbVBpOGtHdnc1UUhXMnpzclZv?=
 =?utf-8?B?UVZ2Q0hocGVJZkI0TUFzaHIwZll3eEJPNmppNHE4VmwrV1FUVmgySWtyV01H?=
 =?utf-8?B?TS9MVERpR0M4K2pTSGVMNm5DaFcrdlRZWUxZSklMUkV0MHo3cUZLeGFFTzJr?=
 =?utf-8?B?RmRnQTYxTUxVVFVFUHI2RUl5V3NxU0s3VkdqdGpXZTZjS2llWmdRVzVsa1hW?=
 =?utf-8?B?RTg5R3hDWlVMb2ZUTnAwUWJhOS93YXg5azFuQmNVaGgrTDhRUllZeDhCSWFB?=
 =?utf-8?B?c1piTHVSY0VidDNsTWF2aVB2UWJzRDNQMjRhWWJKblRwc0JkZjZNamlPQlBH?=
 =?utf-8?B?dXNwcVM5bTNJLzlaS0xoeXBnTWV3REdiMjkxdFB4SzNoZmExOGtBTzJBMXZk?=
 =?utf-8?B?alJ2Zk5kMnJ2ZzYxYllvT3g5VlA5K2tkL0pycytvc0E2Z0ZCMUdRM3VMckRE?=
 =?utf-8?B?cWllSHhGRWEwN3NpS2VIS1lSQVpIQ3ZabmwwcEQ0TmhnUWlkemoyd29DL1A2?=
 =?utf-8?B?dGFBK2JoMU5VWS9DYlNpdW9wMWR5Yk5sb0RtK3FxdCtybVdjcENOV3o4R0VD?=
 =?utf-8?B?TUViTmMyVk5FMnRlYnB0eTRCdk8wSDJzUHdWRVRES2NqbDJlYmZsV1ZGWHEz?=
 =?utf-8?Q?YLJSMCbGyYczwRnpZ8ylwURsCrwqL6OsQjO+/o2YL32K?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0dd4993f-b93a-4d1d-c0a3-08d9fcc30f2b
X-MS-Exchange-CrossTenant-AuthSource: BYAPR10MB2663.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Mar 2022 03:08:15.4389
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: zbpf01CTvl83xg083UnYxM6arfRYKrx/snxv6Nq2+2pJtSJbsbV6Z9CIR4JcbdivwxSBRavh9pbg3xoZ6Et2Tg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLAPR10MB5266
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10274 signatures=686787
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 bulkscore=0 malwarescore=0
 mlxscore=0 phishscore=0 suspectscore=0 adultscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000
 definitions=main-2203030012
X-Proofpoint-ORIG-GUID: RB8ru-RFzQv-fnzFJG3-adb2CHHvpdIW
X-Proofpoint-GUID: RB8ru-RFzQv-fnzFJG3-adb2CHHvpdIW
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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