2018-08-23 20:32:27

by Dexuan Cui

[permalink] [raw]
Subject: [PATCH] Drivers: hv: vmbus: Use raw_smp_processor_id() in vmbus_connect()


With CONFIG_DEBUG_PREEMPT=y, I always see this warning:
BUG: using smp_processor_id() in preemptible [00000000]

Fix the false warning by using raw_smp_processor_id().

Here vmbus_connect() sends a message to the host and waits for the
host's response. The host will deliver the response message and an
interrupt on CPU msg->target_vcpu, and later the interrupt handler
will wake up vmbus_connect(). vmbus_connect() doesn't really have
to run on the same cpu as CPU msg->target_vcpu, so it's safe to
use raw_smp_processor_id() here.

Signed-off-by: Dexuan Cui <[email protected]>
Cc: [email protected]
Cc: K. Y. Srinivasan <[email protected]>
Cc: Haiyang Zhang <[email protected]>
Cc: Stephen Hemminger <[email protected]>
---
drivers/hv/connection.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index ced0418..c3b7040 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -119,8 +119,8 @@ static int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo,
*/
if (version >= VERSION_WIN8_1) {
msg->target_vcpu =
- hv_cpu_number_to_vp_number(smp_processor_id());
- vmbus_connection.connect_cpu = smp_processor_id();
+ hv_cpu_number_to_vp_number(raw_smp_processor_id());
+ vmbus_connection.connect_cpu = raw_smp_processor_id();
} else {
msg->target_vcpu = 0;
vmbus_connection.connect_cpu = 0;
--
2.7.4



2018-08-30 16:30:19

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH] Drivers: hv: vmbus: Use raw_smp_processor_id() in vmbus_connect()

> -----Original Message-----
> From: Dexuan Cui
> Sent: Thursday, August 23, 2018 3:31 PM
> To: '[email protected]' <[email protected]>; KY
> Srinivasan <[email protected]>; Stephen Hemminger
> <[email protected]>; Haiyang Zhang <[email protected]>
> Cc: '[email protected]' <[email protected]>;
> '[email protected]' <driverdev-
> [email protected]>; '[email protected]' <[email protected]>;
> '[email protected]' <[email protected]>; '[email protected]'
> <[email protected]>; vkuznets <[email protected]>;
> '[email protected]' <[email protected]>
> Subject: [PATCH] Drivers: hv: vmbus: Use raw_smp_processor_id() in
> vmbus_connect()
>
>
> With CONFIG_DEBUG_PREEMPT=y, I always see this warning:
> BUG: using smp_processor_id() in preemptible [00000000]
>
> Fix the false warning by using raw_smp_processor_id().
>
> Here vmbus_connect() sends a message to the host and waits for the
> host's response. The host will deliver the response message and an
> interrupt on CPU msg->target_vcpu, and later the interrupt handler
> will wake up vmbus_connect(). vmbus_connect() doesn't really have
> to run on the same cpu as CPU msg->target_vcpu, so it's safe to
> use raw_smp_processor_id() here.
>
> Signed-off-by: Dexuan Cui <[email protected]>
> Cc: [email protected]
> Cc: K. Y. Srinivasan <[email protected]>
> Cc: Haiyang Zhang <[email protected]>
> Cc: Stephen Hemminger <[email protected]>
> ---
> drivers/hv/connection.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index ced0418..c3b7040 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -119,8 +119,8 @@ static int vmbus_negotiate_version(struct
> vmbus_channel_msginfo *msginfo,
> */
> if (version >= VERSION_WIN8_1) {
> msg->target_vcpu =
> -
> hv_cpu_number_to_vp_number(smp_processor_id());
> - vmbus_connection.connect_cpu = smp_processor_id();
> +
> hv_cpu_number_to_vp_number(raw_smp_processor_id());
> + vmbus_connection.connect_cpu = raw_smp_processor_id();
Dexuan,

We want a consistent view of the CPU we are currently running on. How about doing something
like this:

diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index ced041899456..4cc0877ba38a 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -119,8 +119,9 @@ static int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo,
*/
if (version >= VERSION_WIN8_1) {
msg->target_vcpu =
- hv_cpu_number_to_vp_number(smp_processor_id());
- vmbus_connection.connect_cpu = smp_processor_id();
+ hv_cpu_number_to_vp_number(get_cpu());
+ vmbus_connection.connect_cpu = msg->target_vcpu;
+ put_cpu();
} else {
msg->target_vcpu = 0;
vmbus_connection.connect_cpu = 0;

> } else {
> msg->target_vcpu = 0;
> vmbus_connection.connect_cpu = 0;
> --
> 2.7.4


2018-08-30 17:29:58

by Dexuan Cui

[permalink] [raw]
Subject: RE: [PATCH] Drivers: hv: vmbus: Use raw_smp_processor_id() in vmbus_connect()

> From: KY Srinivasan
> Sent: Thursday, August 30, 2018 09:28
> ...
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> @@ -119,8 +119,9 @@ static int vmbus_negotiate_version(struct
> vmbus_channel_msginfo *msginfo,
> */
> if (version >= VERSION_WIN8_1) {
> msg->target_vcpu =
> - hv_cpu_number_to_vp_number(smp_processor_id());
> - vmbus_connection.connect_cpu = smp_processor_id();

> + hv_cpu_number_to_vp_number(get_cpu());
> + vmbus_connection.connect_cpu = msg->target_vcpu;
> + put_cpu();

While the warning " BUG: using smp_processor_id() in preemptible" can also
be avoided, the line
+ vmbus_connection.connect_cpu = msg->target_vcpu;
seems incorrect, as the "connect_cpu" should reflect Linux CPU numbering
rather than Hyper-V's vCPU numbering.

-- Dexuan

2018-08-30 17:34:20

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH] Drivers: hv: vmbus: Use raw_smp_processor_id() in vmbus_connect()



> -----Original Message-----
> From: Dexuan Cui
> Sent: Thursday, August 30, 2018 12:27 PM
> To: KY Srinivasan <[email protected]>; '[email protected]'
> <[email protected]>; Stephen Hemminger
> <[email protected]>; Haiyang Zhang <[email protected]>
> Cc: '[email protected]' <[email protected]>;
> '[email protected]' <driverdev-
> [email protected]>; '[email protected]' <[email protected]>;
> '[email protected]' <[email protected]>; '[email protected]'
> <[email protected]>; vkuznets <[email protected]>;
> '[email protected]' <[email protected]>
> Subject: RE: [PATCH] Drivers: hv: vmbus: Use raw_smp_processor_id() in
> vmbus_connect()
>
> > From: KY Srinivasan
> > Sent: Thursday, August 30, 2018 09:28
> > ...
> > diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> > @@ -119,8 +119,9 @@ static int vmbus_negotiate_version(struct
> > vmbus_channel_msginfo *msginfo,
> > */
> > if (version >= VERSION_WIN8_1) {
> > msg->target_vcpu =
> > - hv_cpu_number_to_vp_number(smp_processor_id());
> > - vmbus_connection.connect_cpu = smp_processor_id();
>
> > + hv_cpu_number_to_vp_number(get_cpu());
> > + vmbus_connection.connect_cpu = msg->target_vcpu;
> > + put_cpu();
>
> While the warning " BUG: using smp_processor_id() in preemptible" can also
> be avoided, the line
> + vmbus_connection.connect_cpu = msg->target_vcpu;
> seems incorrect, as the "connect_cpu" should reflect Linux CPU numbering
> rather than Hyper-V's vCPU numbering.

Yes of course! Can you send me a patch with the fix.

K. Y
>
> -- Dexuan

2018-08-30 17:43:19

by Dexuan Cui

[permalink] [raw]
Subject: RE: [PATCH] Drivers: hv: vmbus: Use raw_smp_processor_id() in vmbus_connect()

> From: KY Srinivasan
> Sent: Thursday, August 30, 2018 10:32
> > > */
> > > if (version >= VERSION_WIN8_1) {
> > > msg->target_vcpu =
> > > - hv_cpu_number_to_vp_number(smp_processor_id());
> > > - vmbus_connection.connect_cpu = smp_processor_id();
> >
> > > + hv_cpu_number_to_vp_number(get_cpu());
> > > + vmbus_connection.connect_cpu = msg->target_vcpu;
> > > + put_cpu();
> >
> > While the warning " BUG: using smp_processor_id() in preemptible" can also
> > be avoided, the line
> > + vmbus_connection.connect_cpu = msg->target_vcpu;
> > seems incorrect, as the "connect_cpu" should reflect Linux CPU numbering
> > rather than Hyper-V's vCPU numbering.
>
> Yes of course! Can you send me a patch with the fix.
>
> K. Y

Ok. Will do.

-- Dexuan