[v2,for-4.4,07/16] Drivers: hv: get rid of redundant messagecount in create_gpadl_header()

Message ID 1492019030-13567-8-git-send-email-sumit.semwal@linaro.org
State New
Headers show
Series
  • Stable commits from Ubuntu Xenial 4.4-lts
Related show

Commit Message

Sumit Semwal April 12, 2017, 5:43 p.m.
From: Vitaly Kuznetsov <vkuznets@redhat.com>


[ Upstream commit 4d63763296ab7865a98bc29cc7d77145815ef89f ]

We use messagecount only once in vmbus_establish_gpadl() to check if
it is safe to iterate through the submsglist. We can just initialize
the list header in all cases in create_gpadl_header() instead.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>

---
 drivers/hv/channel.c | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

-- 
2.7.4

Comments

Greg KH April 16, 2017, 8 a.m. | #1
On Wed, Apr 12, 2017 at 11:13:41PM +0530, Sumit Semwal wrote:
> From: Vitaly Kuznetsov <vkuznets@redhat.com>

> 

> [ Upstream commit 4d63763296ab7865a98bc29cc7d77145815ef89f ]

> 

> We use messagecount only once in vmbus_establish_gpadl() to check if

> it is safe to iterate through the submsglist. We can just initialize

> the list header in all cases in create_gpadl_header() instead.

> 

> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>

> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>

> ---

>  drivers/hv/channel.c | 38 ++++++++++++++++----------------------

>  1 file changed, 16 insertions(+), 22 deletions(-)

> 

> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c

> index 1ef37c7..fb1e3df 100644

> --- a/drivers/hv/channel.c

> +++ b/drivers/hv/channel.c

> @@ -223,8 +223,7 @@ EXPORT_SYMBOL_GPL(vmbus_open);

>   * create_gpadl_header - Creates a gpadl for the specified buffer

>   */

>  static int create_gpadl_header(void *kbuffer, u32 size,

> -					 struct vmbus_channel_msginfo **msginfo,

> -					 u32 *messagecount)

> +			       struct vmbus_channel_msginfo **msginfo)

>  {

>  	int i;

>  	int pagecount;

> @@ -268,7 +267,6 @@ static int create_gpadl_header(void *kbuffer, u32 size,

>  			gpadl_header->range[0].pfn_array[i] = slow_virt_to_phys(

>  				kbuffer + PAGE_SIZE * i) >> PAGE_SHIFT;

>  		*msginfo = msgheader;

> -		*messagecount = 1;

>  

>  		pfnsum = pfncount;

>  		pfnleft = pagecount - pfncount;

> @@ -308,7 +306,6 @@ static int create_gpadl_header(void *kbuffer, u32 size,

>  			}

>  

>  			msgbody->msgsize = msgsize;

> -			(*messagecount)++;

>  			gpadl_body =

>  				(struct vmbus_channel_gpadl_body *)msgbody->msg;

>  

> @@ -337,6 +334,8 @@ static int create_gpadl_header(void *kbuffer, u32 size,

>  		msgheader = kzalloc(msgsize, GFP_KERNEL);

>  		if (msgheader == NULL)

>  			goto nomem;

> +

> +		INIT_LIST_HEAD(&msgheader->submsglist);

>  		msgheader->msgsize = msgsize;

>  

>  		gpadl_header = (struct vmbus_channel_gpadl_header *)

> @@ -351,7 +350,6 @@ static int create_gpadl_header(void *kbuffer, u32 size,

>  				kbuffer + PAGE_SIZE * i) >> PAGE_SHIFT;

>  

>  		*msginfo = msgheader;

> -		*messagecount = 1;

>  	}

>  

>  	return 0;

> @@ -376,7 +374,6 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,

>  	struct vmbus_channel_gpadl_body *gpadl_body;

>  	struct vmbus_channel_msginfo *msginfo = NULL;

>  	struct vmbus_channel_msginfo *submsginfo;

> -	u32 msgcount;

>  	struct list_head *curr;

>  	u32 next_gpadl_handle;

>  	unsigned long flags;

> @@ -385,7 +382,7 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,

>  	next_gpadl_handle =

>  		(atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1);

>  

> -	ret = create_gpadl_header(kbuffer, size, &msginfo, &msgcount);

> +	ret = create_gpadl_header(kbuffer, size, &msginfo);

>  	if (ret)

>  		return ret;

>  

> @@ -408,24 +405,21 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,

>  	if (ret != 0)

>  		goto cleanup;

>  

> -	if (msgcount > 1) {

> -		list_for_each(curr, &msginfo->submsglist) {

> +	list_for_each(curr, &msginfo->submsglist) {

> +		submsginfo = (struct vmbus_channel_msginfo *)curr;

> +		gpadl_body =

> +			(struct vmbus_channel_gpadl_body *)submsginfo->msg;

>  

> -			submsginfo = (struct vmbus_channel_msginfo *)curr;

> -			gpadl_body =

> -			     (struct vmbus_channel_gpadl_body *)submsginfo->msg;

> +		gpadl_body->header.msgtype =

> +			CHANNELMSG_GPADL_BODY;

> +		gpadl_body->gpadl = next_gpadl_handle;

>  

> -			gpadl_body->header.msgtype =

> -				CHANNELMSG_GPADL_BODY;

> -			gpadl_body->gpadl = next_gpadl_handle;

> +		ret = vmbus_post_msg(gpadl_body,

> +				     submsginfo->msgsize -

> +				     sizeof(*submsginfo));

> +		if (ret != 0)

> +			goto cleanup;

>  

> -			ret = vmbus_post_msg(gpadl_body,

> -					       submsginfo->msgsize -

> -					       sizeof(*submsginfo));

> -			if (ret != 0)

> -				goto cleanup;

> -

> -		}

>  	}

>  	wait_for_completion(&msginfo->waitevent);

>  


Why should this go into a stable kernel? What does it fix?

thanks,

greg k-h
Sumit Semwal April 17, 2017, 2:49 p.m. | #2
Hi Greg,

On 16 April 2017 at 13:30, Greg KH <gregkh@linuxfoundation.org> wrote:
>

> Why should this go into a stable kernel? What does it fix?

>

You're right; it doesn't need to get into stable kernel; my bad. Could
you please drop it from your queue?

> thanks,

>

> greg k-h


Best,
Sumit.
Greg KH April 19, 2017, 12:58 p.m. | #3
On Mon, Apr 17, 2017 at 08:19:07PM +0530, Sumit Semwal wrote:
> Hi Greg,

> 

> On 16 April 2017 at 13:30, Greg KH <gregkh@linuxfoundation.org> wrote:

> >

> > Why should this go into a stable kernel? What does it fix?

> >

> You're right; it doesn't need to get into stable kernel; my bad. Could

> you please drop it from your queue?


Now dropped, but now the other hv patches don't apply.  Can you try to
figure out what is really needed for the hv patches in this series, if
any?

thanks,

greg k-h
Sumit Semwal April 20, 2017, 5:34 a.m. | #4
Hi Greg,


On 19 April 2017 at 18:28, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Mon, Apr 17, 2017 at 08:19:07PM +0530, Sumit Semwal wrote:

>> Hi Greg,

>>

>> On 16 April 2017 at 13:30, Greg KH <gregkh@linuxfoundation.org> wrote:

>> >

>> > Why should this go into a stable kernel? What does it fix?

>> >

>> You're right; it doesn't need to get into stable kernel; my bad. Could

>> you please drop it from your queue?

>

> Now dropped, but now the other hv patches don't apply.  Can you try to

> figure out what is really needed for the hv patches in this series, if

> any?


So this patch just changed how list_* were getting used, by
initializing the list header unconditionally. After dropping this, the
immediately following patch needed a little reorganising. The rest of
the patches will apply as-is, but for easy applicaiton, I'll just send
out all the hv patches as a series right now.
>

> thanks,

>

> greg k-h


Best,
Sumit.

Patch

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 1ef37c7..fb1e3df 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -223,8 +223,7 @@  EXPORT_SYMBOL_GPL(vmbus_open);
  * create_gpadl_header - Creates a gpadl for the specified buffer
  */
 static int create_gpadl_header(void *kbuffer, u32 size,
-					 struct vmbus_channel_msginfo **msginfo,
-					 u32 *messagecount)
+			       struct vmbus_channel_msginfo **msginfo)
 {
 	int i;
 	int pagecount;
@@ -268,7 +267,6 @@  static int create_gpadl_header(void *kbuffer, u32 size,
 			gpadl_header->range[0].pfn_array[i] = slow_virt_to_phys(
 				kbuffer + PAGE_SIZE * i) >> PAGE_SHIFT;
 		*msginfo = msgheader;
-		*messagecount = 1;
 
 		pfnsum = pfncount;
 		pfnleft = pagecount - pfncount;
@@ -308,7 +306,6 @@  static int create_gpadl_header(void *kbuffer, u32 size,
 			}
 
 			msgbody->msgsize = msgsize;
-			(*messagecount)++;
 			gpadl_body =
 				(struct vmbus_channel_gpadl_body *)msgbody->msg;
 
@@ -337,6 +334,8 @@  static int create_gpadl_header(void *kbuffer, u32 size,
 		msgheader = kzalloc(msgsize, GFP_KERNEL);
 		if (msgheader == NULL)
 			goto nomem;
+
+		INIT_LIST_HEAD(&msgheader->submsglist);
 		msgheader->msgsize = msgsize;
 
 		gpadl_header = (struct vmbus_channel_gpadl_header *)
@@ -351,7 +350,6 @@  static int create_gpadl_header(void *kbuffer, u32 size,
 				kbuffer + PAGE_SIZE * i) >> PAGE_SHIFT;
 
 		*msginfo = msgheader;
-		*messagecount = 1;
 	}
 
 	return 0;
@@ -376,7 +374,6 @@  int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
 	struct vmbus_channel_gpadl_body *gpadl_body;
 	struct vmbus_channel_msginfo *msginfo = NULL;
 	struct vmbus_channel_msginfo *submsginfo;
-	u32 msgcount;
 	struct list_head *curr;
 	u32 next_gpadl_handle;
 	unsigned long flags;
@@ -385,7 +382,7 @@  int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
 	next_gpadl_handle =
 		(atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1);
 
-	ret = create_gpadl_header(kbuffer, size, &msginfo, &msgcount);
+	ret = create_gpadl_header(kbuffer, size, &msginfo);
 	if (ret)
 		return ret;
 
@@ -408,24 +405,21 @@  int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
 	if (ret != 0)
 		goto cleanup;
 
-	if (msgcount > 1) {
-		list_for_each(curr, &msginfo->submsglist) {
+	list_for_each(curr, &msginfo->submsglist) {
+		submsginfo = (struct vmbus_channel_msginfo *)curr;
+		gpadl_body =
+			(struct vmbus_channel_gpadl_body *)submsginfo->msg;
 
-			submsginfo = (struct vmbus_channel_msginfo *)curr;
-			gpadl_body =
-			     (struct vmbus_channel_gpadl_body *)submsginfo->msg;
+		gpadl_body->header.msgtype =
+			CHANNELMSG_GPADL_BODY;
+		gpadl_body->gpadl = next_gpadl_handle;
 
-			gpadl_body->header.msgtype =
-				CHANNELMSG_GPADL_BODY;
-			gpadl_body->gpadl = next_gpadl_handle;
+		ret = vmbus_post_msg(gpadl_body,
+				     submsginfo->msgsize -
+				     sizeof(*submsginfo));
+		if (ret != 0)
+			goto cleanup;
 
-			ret = vmbus_post_msg(gpadl_body,
-					       submsginfo->msgsize -
-					       sizeof(*submsginfo));
-			if (ret != 0)
-				goto cleanup;
-
-		}
 	}
 	wait_for_completion(&msginfo->waitevent);