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

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

[K. Y. Srinivasan]

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

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 include/linux/hyperv.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 330ec44..ab7359f 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -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)
-- 
1.7.4.1

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

[Greg KH]

On Thu, Mar 06, 2014 at 11:15:08PM -0800, K. Y. Srinivasan wrote:
> Increase the maximum number of pfns we can handle is a single vmbus packet.

Is that a correct sentance?


> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  include/linux/hyperv.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 330ec44..ab7359f 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -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

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

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Friday, March 7, 2014 12:19 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; jasowang@redhat.com
> Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Include the limit on the number
> of pfns we can handle
> 
> On Thu, Mar 06, 2014 at 11:15:08PM -0800, K. Y. Srinivasan wrote:
> > 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.
> 
> 
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> >  include/linux/hyperv.h |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index
> > 330ec44..ab7359f 100644
> > --- a/include/linux/hyperv.h
> > +++ b/include/linux/hyperv.h
> > @@ -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?
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.
> 
> 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 
> 
> greg k-h

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

[Dan Carpenter]

On Thu, Mar 06, 2014 at 11:15:08PM -0800, K. Y. Srinivasan wrote:
> Increase the maximum number of pfns we can handle is a single vmbus packet.
                                                    ^^ in
> 

What are the user visible effects of this patch?

regards,
dan carpenter

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

[KY Srinivasan]

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Friday, March 7, 2014 1:46 PM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com
> Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Include the limit on the number
> of pfns we can handle
> 
> On Thu, Mar 06, 2014 at 11:15:08PM -0800, K. Y. Srinivasan wrote:
> > 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.

> >
> 
> 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

> 
> regards,
> dan carpenter

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

[Dan Carpenter]

On Fri, Mar 07, 2014 at 09:07:42AM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> > Sent: Friday, March 7, 2014 1:46 PM
> > To: KY Srinivasan
> > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> > jasowang@redhat.com
> > Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Include the limit on the number
> > of pfns we can handle
> > 
> > On Thu, Mar 06, 2014 at 11:15:08PM -0800, K. Y. Srinivasan wrote:
> > > 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.

> > >
> > 
> > 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

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

[KY Srinivasan]

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Friday, March 7, 2014 2:51 PM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com
> Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Include the limit on the number
> of pfns we can handle
> 
> On Fri, Mar 07, 2014 at 09:07:42AM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> > > Sent: Friday, March 7, 2014 1:46 PM
> > > To: KY Srinivasan
> > > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> > > jasowang@redhat.com
> > > Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Include the limit on
> > > the number of pfns we can handle
> > >
> > > On Thu, Mar 06, 2014 at 11:15:08PM -0800, K. Y. Srinivasan wrote:
> > > > 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.
> 
> > > >
> > >
> > > 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.

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
> 
> regards,
> dan carpenter