[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 2/2] xen: delay xen_hvm_init_time_ops() if kdump is boot on vcpu>=32


  • To: Dongli Zhang <dongli.zhang@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, x86@xxxxxxxxxx
  • From: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
  • Date: Wed, 2 Mar 2022 21:11:24 -0500
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=7Cp9J//bQQUjc+F/H7wFhBgQLPh2oU3W65xAyFcURaU=; b=EePiwM2cYi467hhCc4Tr3739HmniYUrYerZO4b+iUQO8s2SVyYnANS3GUMxcHDpeDxDMC4gTttD9e7ybK1X35iZlHUHbUyRaBvcbz/Bq/yD3TJHf7kfcYrVLk7dJgGFWx0g9mWYuC5H4gSGcr696ucOtseDsxMfCa54APcEROLtjhbbVEjJgG+UbRCDE+FIJRX3BLWaEgPLfVITkJTWq6ZjW8NJ7nMA+dnGTrXpUsxs7v8/6lwCM76N1+yzKI5wWVBqVJK4DOJKoOPzjoBHcpACKIewo2FNXn13kWeESlWeJ92C7DW3ISGDs/X2pWQdLKlXT99MlLB5GilKyqZfP4g==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kE0kR4lmsVCKea8ALDlasw9bh7yJ+1TVg2rKUt8AUKGpfCQPROSU/85PPyeXFnUymcQT1+xoj/3vNgmfwUwh8RshEW0zXcClxcKUmljA3u2/ai+NMcuqt1o5CNteh7OO4XKCOFXgGoN3LVtwfs3tALC0u3Iny4MwY0ieCggz7ibJYQ4NeLnV5NVxL0rcWUeaWMBm++SwI+DAtUENIW62D2CscunPZW5yhEHWP0IRxS2J25N9qP/IY7OEbI+Oeii0dSk162tPhzpk4JJKUTD3KFg4qPj5vBOieK92rACS80ufgFAp8pFfzr4Qb4TGNIyYQlWr76dasJHKBfQxXyoLaw==
  • Cc: linux-kernel@xxxxxxxxxxxxxxx, jgross@xxxxxxxx, sstabellini@xxxxxxxxxx, tglx@xxxxxxxxxxxxx, mingo@xxxxxxxxxx, bp@xxxxxxxxx, dave.hansen@xxxxxxxxxxxxxxx, joe.jin@xxxxxxxxxx
  • Delivery-date: Thu, 03 Mar 2022 02:11:58 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>


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


Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>


Thank you very much!

Dongli Zhang

+
+    /*
+     * Only MAX_VIRT_CPUS 'vcpu_info' are embedded inside 'shared_info'.
+     * The __this_cpu_read(xen_vcpu) is still NULL when Xen HVM guest
+     * boots on vcpu >= MAX_VIRT_CPUS (e.g., kexec), To access
+     * __this_cpu_read(xen_vcpu) via xen_clocksource_read() will panic.
+     *
+     * The xen_hvm_init_time_ops() should be called again later after
+     * __this_cpu_read(xen_vcpu) is available.
+     */
+    if (!__this_cpu_read(xen_vcpu)) {
+        pr_info("Delay xen_init_time_common() as kernel is running on
vcpu=%d\n",
+            xen_vcpu_nr(0));
           return;
       }
   @@ -577,6 +597,9 @@ void __init xen_hvm_init_time_ops(void)
       x86_cpuinit.setup_percpu_clockev = xen_hvm_setup_cpu_clockevents;
         x86_platform.set_wallclock = xen_set_wallclock;
+
+exit:
+    hvm_time_initialized = true;
   }
   #endif



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.