Fix the subsystem prefix in the subject.
On Wed, Jan 04, 2017 at 02:39:31PM -0800, Long Li wrote:
> From: Long Li <longli@xxxxxxxxxxxxx>
>
> Hyper-v host guarantees that a hypercall will succeed. Retry infinitely to avoid returning transient failures to upper layer.
>
> Signed-off-by: Long Li <longli@xxxxxxxxxxxxx>
> ---
> drivers/hv/connection.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index 6ce8b87..4bcb099 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -439,7 +439,6 @@ int vmbus_post_msg(void *buffer, size_t buflen)
> {
> union hv_connection_id conn_id;
> int ret = 0;
Btw, when you disable GCC's uninitialized variable checking by storing
bogus values in "ret", it's eventually going to bite you in the bum.
Eventually you're going to get a bug that should have been detected
through static analysis if only you hadn't disabled it.
> - int retries = 0;
> u32 usec = 1;
>
> conn_id.asu32 = 0;
> @@ -447,10 +446,10 @@ int vmbus_post_msg(void *buffer, size_t buflen)
>
> /*
> * hv_post_message() can have transient failures because of
> - * insufficient resources. Retry the operation a couple of
> - * times before giving up.
> + * insufficient resources. We retry infinitely on these failures
> + * because host guarantees hypercall will eventually succeed.
> */
> - while (retries < 20) {
> + while (1) {
> ret = hv_post_message(conn_id, 1, buffer, buflen);
>
> switch (ret) {
> @@ -459,11 +458,11 @@ int vmbus_post_msg(void *buffer, size_t buflen)
> * We could get this if we send messages too
> * frequently.
> */
Move the comment above the code it's commenting about.
/*
* We could get INVALID_CONNECTION_ID if we flood the
* host with too many messages.
*/
case HV_STATUS_INVALID_CONNECTION_ID:
case HV_STATUS_INSUFFICIENT_MEMORY:
case HV_STATUS_INSUFFICIENT_BUFFERS:
break;
> - ret = -EAGAIN;
> - break;
> case HV_STATUS_INSUFFICIENT_MEMORY:
> case HV_STATUS_INSUFFICIENT_BUFFERS:
> - ret = -ENOMEM;
> + /*
> + * Temporary failure out of resources
> + */
> break;
> case HV_STATUS_SUCCESS:
> return ret;
return 0;
Better to be more explicit. When I looked at this I got briefly
confused if this function was supposed to return HV_ statuses or
standard kernel error codes. It turns out that HV_STATUS_SUCCESS is
zero the success returns map directly to linux kernel code for success
but it's clearer to be explicit.
> @@ -472,12 +471,12 @@ int vmbus_post_msg(void *buffer, size_t buflen)
> return -EINVAL;
> }
> - retries++;
> udelay(usec);
> if (usec < 2048)
> usec *= 2;
> }
> - return ret;
> + /* Impossible to get here */
> + BUG_ON(1);
Remove the comment and the BUG_ON().
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
