[RFC PATCH] drivers: hv: Convert open-coded timeouts to msecs_to_jiffies()

[RFC PATCH] drivers: hv: Convert open-coded timeouts to msecs_to_jiffies()

[Easwar Hariharan]

We have several places where timeouts are open-coded as N (seconds) * HZ,
but best practice is to use msecs_to_jiffies(). Convert the timeouts to
make them HZ invariant.

Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com>
---
 drivers/hv/hv_balloon.c  | 9 +++++----
 drivers/hv/hv_kvp.c      | 4 ++--
 drivers/hv/hv_snapshot.c | 6 ++++--
 drivers/hv/vmbus_drv.c   | 2 +-
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index c38dcdfcb914d..3017d41f12681 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -756,7 +756,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
 		 * adding succeeded, it is ok to proceed even if the memory was
 		 * not onlined in time.
 		 */
-		wait_for_completion_timeout(&dm_device.ol_waitevent, 5 * HZ);
+		wait_for_completion_timeout(&dm_device.ol_waitevent, msecs_to_jiffies(5 * 1000));
 		post_status(&dm_device);
 	}
 }
@@ -1373,7 +1373,8 @@ static int dm_thread_func(void *dm_dev)
 	struct hv_dynmem_device *dm = dm_dev;
 
 	while (!kthread_should_stop()) {
-		wait_for_completion_interruptible_timeout(&dm_device.config_event, 1 * HZ);
+		wait_for_completion_interruptible_timeout(&dm_device.config_event,
+							  msecs_to_jiffies(1 * 1000));
 		/*
 		 * The host expects us to post information on the memory
 		 * pressure every second.
@@ -1748,7 +1749,7 @@ static int balloon_connect_vsp(struct hv_device *dev)
 	if (ret)
 		goto out;
 
-	t = wait_for_completion_timeout(&dm_device.host_event, 5 * HZ);
+	t = wait_for_completion_timeout(&dm_device.host_event, msecs_to_jiffies(5 * 1000));
 	if (t == 0) {
 		ret = -ETIMEDOUT;
 		goto out;
@@ -1806,7 +1807,7 @@ static int balloon_connect_vsp(struct hv_device *dev)
 	if (ret)
 		goto out;
 
-	t = wait_for_completion_timeout(&dm_device.host_event, 5 * HZ);
+	t = wait_for_completion_timeout(&dm_device.host_event, msecs_to_jiffies(5 * 1000));
 	if (t == 0) {
 		ret = -ETIMEDOUT;
 		goto out;
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index d35b60c061148..8d9ae1b0e8788 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -655,7 +655,7 @@ void hv_kvp_onchannelcallback(void *context)
 		if (host_negotiatied == NEGO_NOT_STARTED) {
 			host_negotiatied = NEGO_IN_PROGRESS;
 			schedule_delayed_work(&kvp_host_handshake_work,
-				      HV_UTIL_NEGO_TIMEOUT * HZ);
+						msecs_to_jiffies(HV_UTIL_NEGO_TIMEOUT * 1000));
 		}
 		return;
 	}
@@ -724,7 +724,7 @@ void hv_kvp_onchannelcallback(void *context)
 		 */
 		schedule_work(&kvp_sendkey_work);
 		schedule_delayed_work(&kvp_timeout_work,
-					HV_UTIL_TIMEOUT * HZ);
+				      msecs_to_jiffies(HV_UTIL_TIMEOUT * 1000));
 
 		return;
 
diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
index 0d2184be16912..be4955613db35 100644
--- a/drivers/hv/hv_snapshot.c
+++ b/drivers/hv/hv_snapshot.c
@@ -192,8 +192,10 @@ static void vss_send_op(void)
 
 	vss_transaction.state = HVUTIL_USERSPACE_REQ;
 
-	schedule_delayed_work(&vss_timeout_work, op == VSS_OP_FREEZE ?
-			VSS_FREEZE_TIMEOUT * HZ : HV_UTIL_TIMEOUT * HZ);
+	schedule_delayed_work(&vss_timeout_work,
+				op == VSS_OP_FREEZE ?
+				msecs_to_jiffies(VSS_FREEZE_TIMEOUT * 1000) :
+				msecs_to_jiffies(HV_UTIL_TIMEOUT * 1000));
 
 	rc = hvutil_transport_send(hvt, vss_msg, sizeof(*vss_msg), NULL);
 	if (rc) {
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 9b15f7daf5059..e35e0a615cb58 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2507,7 +2507,7 @@ static int vmbus_bus_resume(struct device *dev)
 	vmbus_request_offers();
 
 	if (wait_for_completion_timeout(
-		&vmbus_connection.ready_for_resume_event, 10 * HZ) == 0)
+		&vmbus_connection.ready_for_resume_event, msecs_to_jiffies(10 * 1000)) == 0)
 		pr_err("Some vmbus device is missing after suspending?\n");
 
 	/* Reset the event for the next suspend. */
-- 
2.43.0

Re: [RFC PATCH] drivers: hv: Convert open-coded timeouts to msecs_to_jiffies()

[Praveen Kumar]

On 17-10-2024 04:07, Easwar Hariharan wrote:
> We have several places where timeouts are open-coded as N (seconds) * HZ,
> but best practice is to use msecs_to_jiffies(). Convert the timeouts to
> make them HZ invariant.
> > Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com>
> ---
>  drivers/hv/hv_balloon.c  | 9 +++++----
>  drivers/hv/hv_kvp.c      | 4 ++--
>  drivers/hv/hv_snapshot.c | 6 ++++--
>  drivers/hv/vmbus_drv.c   | 2 +-
>  4 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index c38dcdfcb914d..3017d41f12681 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -756,7 +756,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
>  		 * adding succeeded, it is ok to proceed even if the memory was
>  		 * not onlined in time.
>  		 */
> -		wait_for_completion_timeout(&dm_device.ol_waitevent, 5 * HZ);
> +		wait_for_completion_timeout(&dm_device.ol_waitevent, msecs_to_jiffies(5 * 1000));

Is it correct to convert HZ to 1000 ?
Also, how are you testing these changes ?

Regards,

~Praveen.

Re: [RFC PATCH] drivers: hv: Convert open-coded timeouts to msecs_to_jiffies()

[Easwar Hariharan]

On 10/18/2024 12:54 AM, Praveen Kumar wrote:
> On 17-10-2024 04:07, Easwar Hariharan wrote:
>> We have several places where timeouts are open-coded as N (seconds) * HZ,
>> but best practice is to use msecs_to_jiffies(). Convert the timeouts to
>> make them HZ invariant.
>>> Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com>
>> ---
>>  drivers/hv/hv_balloon.c  | 9 +++++----
>>  drivers/hv/hv_kvp.c      | 4 ++--
>>  drivers/hv/hv_snapshot.c | 6 ++++--
>>  drivers/hv/vmbus_drv.c   | 2 +-
>>  4 files changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
>> index c38dcdfcb914d..3017d41f12681 100644
>> --- a/drivers/hv/hv_balloon.c
>> +++ b/drivers/hv/hv_balloon.c
>> @@ -756,7 +756,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
>>  		 * adding succeeded, it is ok to proceed even if the memory was
>>  		 * not onlined in time.
>>  		 */
>> -		wait_for_completion_timeout(&dm_device.ol_waitevent, 5 * HZ);
>> +		wait_for_completion_timeout(&dm_device.ol_waitevent, msecs_to_jiffies(5 * 1000));
> 
> Is it correct to convert HZ to 1000 ?
> Also, how are you testing these changes ?
> 

It's a conversion of milliseconds to seconds, rather than HZ to 1000. :)
msecs_to_jiffies() handles the conversion to jiffies with HZ. As Naman
mentioned, this could be equivalently written as 5 * MSECS_PER_SEC, and
would probably be more readable. On testing, this is only
compile-tested, and that's part of the reason why it's an RFC, since I'm
not 100% sure every one of these timeouts is measured in seconds. Hoping
for folks more familiar with the code to take a look.

Thanks,
Easwar

RE: [RFC PATCH] drivers: hv: Convert open-coded timeouts to msecs_to_jiffies()

[Michael Kelley]

From: Easwar Hariharan <eahariha@linux.microsoft.com> Sent: Friday, October 18, 2024 3:50 PM
> 
> On 10/18/2024 12:54 AM, Praveen Kumar wrote:
> > On 17-10-2024 04:07, Easwar Hariharan wrote:
> >> We have several places where timeouts are open-coded as N (seconds) * HZ,
> >> but best practice is to use msecs_to_jiffies(). Convert the timeouts to
> >> make them HZ invariant.
> >>> Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com>
> >> ---
> >>  drivers/hv/hv_balloon.c  | 9 +++++----
> >>  drivers/hv/hv_kvp.c      | 4 ++--
> >>  drivers/hv/hv_snapshot.c | 6 ++++--
> >>  drivers/hv/vmbus_drv.c   | 2 +-
> >>  4 files changed, 12 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> >> index c38dcdfcb914d..3017d41f12681 100644
> >> --- a/drivers/hv/hv_balloon.c
> >> +++ b/drivers/hv/hv_balloon.c
> >> @@ -756,7 +756,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
> >>  		 * adding succeeded, it is ok to proceed even if the memory was
> >>  		 * not onlined in time.
> >>  		 */
> >> -		wait_for_completion_timeout(&dm_device.ol_waitevent, 5 * HZ);
> >> +		wait_for_completion_timeout(&dm_device.ol_waitevent, msecs_to_jiffies(5 * 1000));
> >
> > Is it correct to convert HZ to 1000 ?
> > Also, how are you testing these changes ?
> >
> 
> It's a conversion of milliseconds to seconds, rather than HZ to 1000. :)
> msecs_to_jiffies() handles the conversion to jiffies with HZ. As Naman
> mentioned, this could be equivalently written as 5 * MSECS_PER_SEC, and
> would probably be more readable. On testing, this is only
> compile-tested, and that's part of the reason why it's an RFC, since I'm
> not 100% sure every one of these timeouts is measured in seconds. Hoping
> for folks more familiar with the code to take a look.
> 

I believe the current code is correct.  Two things:

1) The values multiplied by HZ are indeed in seconds. The number of
seconds are somewhat arbitrary in some of the cases, so you might
argue for a different number of seconds. But as coded, the values
are in seconds.

2) Unless I'm missing something, the current code uses the correct
timeout regardless of the value of HZ because the number of jiffies
per second *is* HZ.

As such, it might help to be explicit in the commit message that this
patch isn't fixing any bugs.  As the commit message says, the patch is
to bring the code into conformance with best practices. I presume
the best practice you reference is described in
Documentation/scheduler/completion.rst.

I don't understand the statement about making the code "HZ invariant",
which I think came from the aforementioned documentation.  Per
my #2 above, I think the existing code is already "HZ invariant", at
least for how I would interpret "HZ invariant".

Regardless of the meaning of "HZ invariant", I agree with the idea of
eliminating the use of HZ in cases like this, and letting msecs_to_jiffies()
handle it. Unfortunately, converting from "5 * HZ" to 
"msecs_to_jiffies(5 * 1000)" makes the code really clunky. I would
advocate for adding something like this to include/linux/jiffies.h:

#define secs_to_jiffies(secs)    msecs_to_jiffies((secs) * 1000)

and then using secs_to_jiffies() for all the cases in this patch. That
reduces the clunkiness. But maybe somebody in the past tried to
add secs_to_jiffies() and got shot down -- I don't know. It seems like
an obvious thing to add ....

Michael

Re: [RFC PATCH] drivers: hv: Convert open-coded timeouts to msecs_to_jiffies()

[Easwar Hariharan]

On 10/18/2024 9:59 PM, Michael Kelley wrote:
> From: Easwar Hariharan <eahariha@linux.microsoft.com> Sent: Friday, October 18, 2024 3:50 PM
>>
>> On 10/18/2024 12:54 AM, Praveen Kumar wrote:
>>> On 17-10-2024 04:07, Easwar Hariharan wrote:
>>>> We have several places where timeouts are open-coded as N (seconds) * HZ,
>>>> but best practice is to use msecs_to_jiffies(). Convert the timeouts to
>>>> make them HZ invariant.
>>>>> Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com>
>>>> ---
>>>>  drivers/hv/hv_balloon.c  | 9 +++++----
>>>>  drivers/hv/hv_kvp.c      | 4 ++--
>>>>  drivers/hv/hv_snapshot.c | 6 ++++--
>>>>  drivers/hv/vmbus_drv.c   | 2 +-
>>>>  4 files changed, 12 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
>>>> index c38dcdfcb914d..3017d41f12681 100644
>>>> --- a/drivers/hv/hv_balloon.c
>>>> +++ b/drivers/hv/hv_balloon.c
>>>> @@ -756,7 +756,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
>>>>  		 * adding succeeded, it is ok to proceed even if the memory was
>>>>  		 * not onlined in time.
>>>>  		 */
>>>> -		wait_for_completion_timeout(&dm_device.ol_waitevent, 5 * HZ);
>>>> +		wait_for_completion_timeout(&dm_device.ol_waitevent, msecs_to_jiffies(5 * 1000));
>>>
>>> Is it correct to convert HZ to 1000 ?
>>> Also, how are you testing these changes ?
>>>
>>
>> It's a conversion of milliseconds to seconds, rather than HZ to 1000. :)
>> msecs_to_jiffies() handles the conversion to jiffies with HZ. As Naman
>> mentioned, this could be equivalently written as 5 * MSECS_PER_SEC, and
>> would probably be more readable. On testing, this is only
>> compile-tested, and that's part of the reason why it's an RFC, since I'm
>> not 100% sure every one of these timeouts is measured in seconds. Hoping
>> for folks more familiar with the code to take a look.
>>
> 
> I believe the current code is correct.  Two things:
> 
> 1) The values multiplied by HZ are indeed in seconds. The number of
> seconds are somewhat arbitrary in some of the cases, so you might
> argue for a different number of seconds. But as coded, the values
> are in seconds.

Thanks for reviewing, Michael, and for the confirmation.

> 
> 2) Unless I'm missing something, the current code uses the correct
> timeout regardless of the value of HZ because the number of jiffies
> per second *is* HZ.
> 
> As such, it might help to be explicit in the commit message that this
> patch isn't fixing any bugs.

Will do.

> As the commit message says, the patch is
> to bring the code into conformance with best practices. I presume
> the best practice you reference is described in
> Documentation/scheduler/completion.rst.
> 
> I don't understand the statement about making the code "HZ invariant",
> which I think came from the aforementioned documentation.  Per
> my #2 above, I think the existing code is already "HZ invariant", at
> least for how I would interpret "HZ invariant".
> 

That's right, both the best practice and the statement of HZ-invariance
came from the scheduler documentation you pointed out. While I can't
find the source with a quick search right now, I understand that HZ
varies with CPU frequency and I figure that's what the statement is
referring to. Unfortunately, there wasn't any discussion on
HZ-invariance I can find on the lore thread where the statement was
added. [1] It seems to be one of those "it's so self explanatory it
doesn't warrant a mention" unless you're one of today's 10,000. [2]

[1]
https://lore.kernel.org/all/1539183392-239389-1-git-send-email-john.garry@huawei.com/T/#u
[2] https://xkcd.com/1053/

> Regardless of the meaning of "HZ invariant", I agree with the idea of
> eliminating the use of HZ in cases like this, and letting msecs_to_jiffies()
> handle it. Unfortunately, converting from "5 * HZ" to 
> "msecs_to_jiffies(5 * 1000)" makes the code really clunky. I would
> advocate for adding something like this to include/linux/jiffies.h:
> 
> #define secs_to_jiffies(secs)    msecs_to_jiffies((secs) * 1000)
> 
> and then using secs_to_jiffies() for all the cases in this patch. That
> reduces the clunkiness. But maybe somebody in the past tried to
> add secs_to_jiffies() and got shot down -- I don't know. It seems like
> an obvious thing to add ....
> 
> Michael

From a quick search on lore with dfb:secs_to_jiffies, it doesn't look
like anyone has tried to add secs_to_jiffies() to jiffies.h. There is
one instance of secs_to_jiffies() being defined in
net/bluetooth/hci_event.c, added in 2021.

Thanks,
Easwar

RE: [RFC PATCH] drivers: hv: Convert open-coded timeouts to msecs_to_jiffies()

[Michael Kelley]

From: Easwar Hariharan <eahariha@linux.microsoft.com> Sent: Sunday, October 20, 2024 8:42 PM
> 
> On 10/18/2024 9:59 PM, Michael Kelley wrote:
> > From: Easwar Hariharan <eahariha@linux.microsoft.com> Sent: Friday, October 18, 2024 3:50 PM
> >>
> >> On 10/18/2024 12:54 AM, Praveen Kumar wrote:
> >>> On 17-10-2024 04:07, Easwar Hariharan wrote:
> >>>> We have several places where timeouts are open-coded as N (seconds) * HZ,
> >>>> but best practice is to use msecs_to_jiffies(). Convert the timeouts to
> >>>> make them HZ invariant.
> >>>>> Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com>
> >>>> ---
> >>>>  drivers/hv/hv_balloon.c  | 9 +++++----
> >>>>  drivers/hv/hv_kvp.c      | 4 ++--
> >>>>  drivers/hv/hv_snapshot.c | 6 ++++--
> >>>>  drivers/hv/vmbus_drv.c   | 2 +-
> >>>>  4 files changed, 12 insertions(+), 9 deletions(-)
> >>>>
> >>>> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> >>>> index c38dcdfcb914d..3017d41f12681 100644
> >>>> --- a/drivers/hv/hv_balloon.c
> >>>> +++ b/drivers/hv/hv_balloon.c
> >>>> @@ -756,7 +756,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
> >>>>  		 * adding succeeded, it is ok to proceed even if the memory was
> >>>>  		 * not onlined in time.
> >>>>  		 */
> >>>> -		wait_for_completion_timeout(&dm_device.ol_waitevent, 5 * HZ);
> >>>> +		wait_for_completion_timeout(&dm_device.ol_waitevent, msecs_to_jiffies(5 * 1000));
> >>>
> >>> Is it correct to convert HZ to 1000 ?
> >>> Also, how are you testing these changes ?
> >>>
> >>
> >> It's a conversion of milliseconds to seconds, rather than HZ to 1000. :)
> >> msecs_to_jiffies() handles the conversion to jiffies with HZ. As Naman
> >> mentioned, this could be equivalently written as 5 * MSECS_PER_SEC, and
> >> would probably be more readable. On testing, this is only
> >> compile-tested, and that's part of the reason why it's an RFC, since I'm
> >> not 100% sure every one of these timeouts is measured in seconds. Hoping
> >> for folks more familiar with the code to take a look.
> >>
> >
> > I believe the current code is correct.  Two things:
> >
> > 1) The values multiplied by HZ are indeed in seconds. The number of
> > seconds are somewhat arbitrary in some of the cases, so you might
> > argue for a different number of seconds. But as coded, the values
> > are in seconds.
> 
> Thanks for reviewing, Michael, and for the confirmation.
> 
> >
> > 2) Unless I'm missing something, the current code uses the correct
> > timeout regardless of the value of HZ because the number of jiffies
> > per second *is* HZ.
> >
> > As such, it might help to be explicit in the commit message that this
> > patch isn't fixing any bugs.
> 
> Will do.
> 
> > As the commit message says, the patch is
> > to bring the code into conformance with best practices. I presume
> > the best practice you reference is described in
> > Documentation/scheduler/completion.rst.
> >
> > I don't understand the statement about making the code "HZ invariant",
> > which I think came from the aforementioned documentation.  Per
> > my #2 above, I think the existing code is already "HZ invariant", at
> > least for how I would interpret "HZ invariant".
> >
> 
> That's right, both the best practice and the statement of HZ-invariance
> came from the scheduler documentation you pointed out. While I can't
> find the source with a quick search right now, I understand that HZ
> varies with CPU frequency and I figure that's what the statement is
> referring to. Unfortunately, there wasn't any discussion on
> HZ-invariance I can find on the lore thread where the statement was
> added. [1] It seems to be one of those "it's so self explanatory it
> doesn't warrant a mention" unless you're one of today's 10,000. [2]

No, HZ is not related to the CPU frequency. HZ is a compile-time fixed
value specified in the .config file.  grep for HZ in the .config file. The
allowed values are 100, 250, 300, and 1000. The jiffies code is set up
so the number of jiffies per sec is HZ. So specifying "5 * HZ" (for
example) as a jiffies value means 5 seconds, regardless of which 
value of HZ the kernel was compiled with. In my interpretation, that
means using "5 * HZ" as a jiffies value is "HZ invariant".

HZ (or some predecessor) originated back in the day when physical
hardware did not offer high precision timers like it does today. Timer
hardware generated "ticks", and ticks were normalized across a wide
range of hardware to occur at frequency HZ. Usually that meant a
timer interrupted HZ times per second. I don't know the full history here,
but jiffies were the coarse measure of the passage of time in the kernel,
so mapping jiffies to HZ made sense. Older internal kernel APIs use
jiffies, mostly for historical reasons even though much higher resolution
timers are usually available. And there may be additional nuances here
that I'm not aware of.

> 
> > Regardless of the meaning of "HZ invariant", I agree with the idea of
> > eliminating the use of HZ in cases like this, and letting msecs_to_jiffies()
> > handle it. Unfortunately, converting from "5 * HZ" to
> > "msecs_to_jiffies(5 * 1000)" makes the code really clunky. I would
> > advocate for adding something like this to include/linux/jiffies.h:
> >
> > #define secs_to_jiffies(secs)    msecs_to_jiffies((secs) * 1000)
> >
> > and then using secs_to_jiffies() for all the cases in this patch. That
> > reduces the clunkiness. But maybe somebody in the past tried to
> > add secs_to_jiffies() and got shot down -- I don't know. It seems like
> > an obvious thing to add ....
> >
> > Michael
> 
> From a quick search on lore with dfb:secs_to_jiffies, it doesn't look
> like anyone has tried to add secs_to_jiffies() to jiffies.h. There is
> one instance of secs_to_jiffies() being defined in
> net/bluetooth/hci_event.c, added in 2021.

That's interesting!  Somebody else had the same idea. Move that
to the jiffies.h file, and then use it in your patch.

Michael

Re: [RFC PATCH] drivers: hv: Convert open-coded timeouts to msecs_to_jiffies()

[Naman Jain]

On 10/17/2024 4:07 AM, Easwar Hariharan wrote:
> We have several places where timeouts are open-coded as N (seconds) * HZ,
> but best practice is to use msecs_to_jiffies(). Convert the timeouts to
> make them HZ invariant.
> 
> Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com>
> ---
>   drivers/hv/hv_balloon.c  | 9 +++++----
>   drivers/hv/hv_kvp.c      | 4 ++--
>   drivers/hv/hv_snapshot.c | 6 ++++--
>   drivers/hv/vmbus_drv.c   | 2 +-
>   4 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index c38dcdfcb914d..3017d41f12681 100644
> --- a/drivers/hv/hv_balloon.c

<.>


>   	if (wait_for_completion_timeout(
> -		&vmbus_connection.ready_for_resume_event, 10 * HZ) == 0)
> +		&vmbus_connection.ready_for_resume_event, msecs_to_jiffies(10 * 1000)) == 0)
>   		pr_err("Some vmbus device is missing after suspending?\n");
>   
>   	/* Reset the event for the next suspend. */


Looks good to me. There can be different ways of passing arg to
msecs_to_jiffies though-

for 10 seconds
* 10000
* 10 * 1000
* 10 * MSEC_PER_SEC

I don't have any strong opinion on this, and you can probably choose
whichever feels better.

Even the current implementation with x * HZ works fine, with different
HZ values. But, yes, I agree that using msecs_to_jiffies is better.

Regards,
Naman