Skip to contentSkip to conversation

Drivers: hv: vmbus: Include the limit on the number of pfns we can handle

PATCH In review K. Y. Srinivasan 3 participants 7 messages Mar 7, 2014 Hyper-V/Azure core and drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Drivers: hv: vmbus: Include the limit on the number of pfns we can handle

Increase the maximum number of pfns we can handle is a single vmbus packet.

---
 include/linux/hyperv.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/hyperv.h +1 -1
index 330ec44..ab7359f 100644
@@ -37,7 +37,7 @@
 #include <linux/mod_devicetable.h>
 
 
-#define MAX_PAGE_BUFFER_COUNT				19
+#define MAX_PAGE_BUFFER_COUNT				32
 #define MAX_MULTIPAGE_BUFFER_COUNT			32 /* 128K */
 
 #pragma pack(push, 1)
Signature · 1 line
1.7.4.1
#1 lore raw
Quoted 1 line · K. Y. Srinivasan

Increase the maximum number of pfns we can handle is a single vmbus packet.

Is that a correct sentance?

Quoted 11 lines
---
 include/linux/hyperv.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/hyperv.h +0 -0
index 330ec44..ab7359f 100644

include/linux/hyperv.h line 40

Quoted 5 lines
include/linux/hyperv.h +1 -1
@@ -37,7 +37,7 @@
 #include <linux/mod_devicetable.h>
 
 
-#define MAX_PAGE_BUFFER_COUNT				19
+#define MAX_PAGE_BUFFER_COUNT				32

What is the problem if this number isn't changed?

Again, I'm going to have to ask you, what kernel is this for?

Please remember this when sending patches that look like they might be fixing a bug, you don't want me to guess, as my "guess" usually is "delete the patch".

greg k-h

#2 lore raw
Quoted 11 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Friday, March 7, 2014 12:19 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Include the limit on the number of pfns we can handle

Quoted 5 lines
Quoted 1 line · K. Y. Srinivasan

Increase the maximum number of pfns we can handle is a single vmbus

packet.

Is that a correct sentance?

I will fix the typo in the sentence.

Quoted 16 lines
Quoted 16 lines
---
 include/linux/hyperv.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/hyperv.h index renamed from include/linux/hyperv.h +0 -0

330ec44..ab7359f 100644

include/linux/hyperv.h +0 -0
@@ -37,7 +37,7 @@
 #include <linux/mod_devicetable.h>
 
 
Quoted 4 lines

include/linux/hyperv.h line 40

Quoted 2 lines
include/linux/hyperv.h +1 -1
-#define MAX_PAGE_BUFFER_COUNT				19
+#define MAX_PAGE_BUFFER_COUNT				32

What is the problem if this number isn't changed?

On the networking side we may send frames that require more than 19 PFN entries. Number 32 is imposed by some limitations by the vmbus data structures. For some reason MAX_PAGE_BUFFER_COUNT was set to a lower value and I want to bump this up to what vmbus can support.

Quoted 6 lines

Again, I'm going to have to ask you, what kernel is this for?

Please remember this when sending patches that look like they might be fixing a bug, you don't want me to guess, as my "guess" usually is "delete the patch".

Sorry Greg; this is not a bug fix and can go into the *next kernel. I will resend this patch after fixing the changelog comment.

Regards,

K. Y

Quoted 2 lines

greg k-h

#3 lore raw
In reply to K. Y. Srinivasan
Quoted 1 line · K. Y. Srinivasan

Increase the maximum number of pfns we can handle is a single vmbus packet.

                                                    ^^ in
Quoted 1 line

What are the user visible effects of this patch?

regards,
dan carpenter

#4 lore raw
Quoted 12 lines

-----Original Message-----
From: Dan Carpenter [mailto:[email protected]]
Sent: Friday, March 7, 2014 1:46 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Include the limit on the number of pfns we can handle

Quoted 4 lines
Quoted 1 line · K. Y. Srinivasan

Increase the maximum number of pfns we can handle is a single vmbus

packet.

                                                    ^^ in

Thanks Dan. I am traveling in India and it shows. I have already sent a corrected patch.

Quoted 3 lines
Quoted 1 line

What are the user visible effects of this patch?

The current code will drop these packets that have more PFNs than the limit. Upping the limit will mitigate this problem.

Regards,

K. Y

Quoted 3 lines

regards,
dan carpenter

#5 lore raw
Quoted 12 lines · KY Srinivasan
Quoted 12 lines

-----Original Message-----
From: Dan Carpenter [mailto:[email protected]]
Sent: Friday, March 7, 2014 1:46 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Include the limit on the number of pfns we can handle

Quoted 7 lines
Quoted 4 lines · K. Y. Srinivasan
Quoted 1 line

Increase the maximum number of pfns we can handle is a single vmbus

packet.

                                                    ^^ in

Thanks Dan. I am traveling in India and it shows. I have already sent a corrected patch.

Yeah. I saw that. Thanks.

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

What are the user visible effects of this patch?

The current code will drop these packets that have more PFNs than the limit. Upping the limit will mitigate this problem.

Meanwhile, the revised patch says that it is not a bugfix. It sort of sounds like upping the limit does improve performance for some people?

I am confused now.

regards,
dan carpenter

#6 lore raw
Quoted 41 lines

-----Original Message-----
From: Dan Carpenter [mailto:[email protected]]
Sent: Friday, March 7, 2014 2:51 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Include the limit on the number of pfns we can handle

Quoted 12 lines · KY Srinivasan
Quoted 12 lines

-----Original Message-----
From: Dan Carpenter [mailto:[email protected]]
Sent: Friday, March 7, 2014 1:46 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Include the limit on the number of pfns we can handle

Quoted 7 lines
Quoted 5 lines · K. Y. Srinivasan
Quoted 2 lines

Increase the maximum number of pfns we can handle is a single vmbus

packet.

                                                    ^^ in

Thanks Dan. I am traveling in India and it shows. I have already sent a

corrected patch.

Quoted 1 line

Yeah. I saw that. Thanks.

Quoted 4 lines
Quoted 3 lines
Quoted 1 line

What are the user visible effects of this patch?

The current code will drop these packets that have more PFNs than the

Quoted 8 lines

limit.

Quoted 2 lines

Upping the limit will mitigate this problem.

Meanwhile, the revised patch says that it is not a bugfix. It sort of sounds like upping the limit does improve performance for some people?

I am confused now.

With the current network code this condition cannot occur since we neither support scatter/gather I/O nor segmentation offload. I have submitted patches that enable these features for our network driver. Once TSO is turned on, the network stack can send packets that may need more than the current limit of PFNs in a packet.

Hope this helps,

K. Y

Quoted 3 lines

regards,
dan carpenter