* [PATCH 2/5] staging: hv: Fixed lockup problem with bounce_buffer scatter list
From: Hank Janssen @ 2010-08-05 19:29 UTC (permalink / raw)
To: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org'
Cc: Haiyang Zhang, 'gregkh@suse.de'
From: Hank Janssen <hjanssen@microsoft.com>
Fixed lockup problem with bounce_buffer scatter list which caused
crashes in heavy loads. Under heavy loads with many clients we this
problem appear, it causes a kernel Panic.
Signed-off-by:Hank Janssen <hjanssen@microsoft.com>
Signed-off-by:Haiyang Zhang <haiyangz@microsoft.com>
Cc: stable <stable@kernel.org>
---
drivers/staging/hv/storvsc_drv.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 3b9ccb0..169d701 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -615,6 +615,7 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
unsigned int request_size = 0;
int i;
struct scatterlist *sgl;
+ unsigned int sg_count = 0;
DPRINT_DBG(STORVSC_DRV, "scmnd %p dir %d, use_sg %d buf %p len %d "
"queue depth %d tagged %d", scmnd, scmnd->sc_data_direction, @@ -697,6 +698,7 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
request->DataBuffer.Length = scsi_bufflen(scmnd);
if (scsi_sg_count(scmnd)) {
sgl = (struct scatterlist *)scsi_sglist(scmnd);
+ sg_count = scsi_sg_count(scmnd);
/* check if we need to bounce the sgl */
if (do_bounce_buffer(sgl, scsi_sg_count(scmnd)) != -1) { @@ -731,15 +733,16 @@ static int storvsc_queuecommand(struct scsi_cmnd *scmnd,
scsi_sg_count(scmnd));
sgl = cmd_request->bounce_sgl;
+ sg_count = cmd_request->bounce_sgl_count;
}
request->DataBuffer.Offset = sgl[0].offset;
- for (i = 0; i < scsi_sg_count(scmnd); i++) {
+ for (i = 0; i < sg_count; i++) {
DPRINT_DBG(STORVSC_DRV, "sgl[%d] len %d offset %d\n",
i, sgl[i].length, sgl[i].offset);
- request->DataBuffer.PfnArray[i] =
- page_to_pfn(sg_page((&sgl[i])));
+ request->DataBuffer.PfnArray[i] =
+ page_to_pfn(sg_page((&sgl[i])));
}
} else if (scsi_sglist(scmnd)) {
/* ASSERT(scsi_bufflen(scmnd) <= PAGE_SIZE); */
--
1.6.0.2
^ permalink raw reply related
* [PATCH 1/5] staging: hv: Fixed bounce kmap problem by using correct index
From: Hank Janssen @ 2010-08-05 19:29 UTC (permalink / raw)
To: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org'
Cc: 'gregkh@suse.de', Haiyang Zhang
From: Hank Janssen <hjanssen@microsoft.com>
Fixed bounce offset kmap problem by using correct index.
The symptom of the problem is that in some NAS appliances this problem
represents Itself by a unresponsive VM under a load with many clients writing
small files.
Signed-off-by:Hank Janssen <hjanssen@microsoft.com>
Signed-off-by:Haiyang Zhang <haiyangz@microsoft.com>
Cc: stable <stable@kernel.org>
---
drivers/staging/hv/storvsc_drv.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 075b61b..3b9ccb0 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -495,7 +495,7 @@ static unsigned int copy_to_bounce_buffer(struct scatterlist *orig_sgl,
/* ASSERT(orig_sgl[i].offset + orig_sgl[i].length <= PAGE_SIZE); */
- if (j == 0)
+ if (bounce_addr == 0)
bounce_addr = (unsigned long)kmap_atomic(sg_page((&bounce_sgl[j])), KM_IRQ0);
while (srclen) {
@@ -556,7 +556,7 @@ static unsigned int copy_from_bounce_buffer(struct scatterlist *orig_sgl,
destlen = orig_sgl[i].length;
/* ASSERT(orig_sgl[i].offset + orig_sgl[i].length <= PAGE_SIZE); */
- if (j == 0)
+ if (bounce_addr == 0)
bounce_addr = (unsigned long)kmap_atomic(sg_page((&bounce_sgl[j])), KM_IRQ0);
while (destlen) {
--
1.6.0.2
^ permalink raw reply related
* Using virtio as a physical (wire-level) transport
From: Ira W. Snyder @ 2010-08-04 23:04 UTC (permalink / raw)
To: Michael S. Tsirkin, Rusty Russell; +Cc: netdev, Zang Roy, virtualization
Hello Michael, Rusty,
I'm trying to figure out how to use virtio-net and vhost-net to
communicate over a physical transport (PCI bus) instead of shared
memory (for example, qemu/kvm guest).
We've talked about this several times in the past, and I currently have
some time to devote to this again. I'm trying to figure out if virtio is
still a viable solution, or if it has been evolved such that it is
unusable for this application.
I am trying to create a generic system to allow the type of
communications described below. I would like to create something that
can be easily ported to any slave computer which meets the following
requirements:
1) it is a PCI slave (agent) (it acts like any other PCI card)
2) it has an inter-processor communications mechanism
3) it has a DMA engine
There is a reasonable amount of demand for such a system. I get
inquiries about the prototype code I posted to linux-netdev at least
once a month. This sort of system is used regularly in the
telecommunications industry, among others.
Here is a quick drawing of the system I work with. Please forgive my
poor ascii art skills.
+-----------------+
| master computer |
| | +-------------------+
| PCI slot #1 | <-- physical connection --> | slave computer #1 |
| virtio-net if#1 | | vhost-net if#1 |
| | +-------------------+
| |
| | +-------------------+
| PCI slot #2 | <-- physical connection --> | slave computer #2 |
| virtio-net if#2 | | vhost-net if#2 |
| | +-------------------+
| |
| | +-------------------+
| PCI slot #n | <-- physical connection --> | slave computer #n |
| virtio-net if#n | | vhost-net if#n |
| | +-------------------+
+-----------------+
The reason for using vhost-net on the "slave" side is because vhost-net
is the component that performs data copies. In most cases, the slave
computers are non-x86 and have DMA controllers. DMA is an absolute
necessity when copying data across the PCI bus.
Do you think virtio is a viable solution to solve this problem? If not,
can you suggest anything else?
Another reason I ask this question is that I have previously invested
several months implementing a similar solution, only to have it outright
rejected for "not being the right way". If you don't think something
like this has any hope, I'd rather not waste another month of my life.
If you can think of a solution that is likely to be "the right way", I'd
rather you told me before I implement any code.
Making my life harder since the last time I tried this, mainline commit
7c5e9ed0c (virtio_ring: remove a level of indirection) has removed the
possibility of using an alternative virtqueue implementation. The commit
message suggests that you might be willing to add this capability back.
Would this be an option?
Thanks for your time,
Ira
^ permalink raw reply
* Re: [PATCH 1/6] staging: hv: Fixed bounce kmap problem by using correct index
From: Greg KH @ 2010-08-04 19:28 UTC (permalink / raw)
To: Hank Janssen
Cc: linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org,
virtualization@lists.osdl.org, Haiyang Zhang
In-Reply-To: <81D58C97-B9A7-4217-9912-7EE6A3FE7DD0@microsoft.com>
On Wed, Aug 04, 2010 at 06:50:24PM +0000, Hank Janssen wrote:
> On Aug 4, 2010, at 11:26, Greg KH <gregkh@suse.de> wrote:
> > On Wed, Aug 04, 2010 at 05:17:21PM +0000, Hank Janssen wrote:
> >>>
> >>
> >> I will re-submit all 5 patches and CC the stable mailing list on it as well.
> >
> > 5? I see 6 of them here :)
> >
> > thanks,
> >
> > greg k-h
> >
>
> True but anise you rejected the todo patch there will only be 5.
> Unless you are willing to take the todo patch. :)
Doh, you are right, nevermind :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 1/6] staging: hv: Fixed bounce kmap problem by using correct index
From: Hank Janssen @ 2010-08-04 18:50 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org,
virtualization@lists.osdl.org, Haiyang Zhang
In-Reply-To: <20100804182318.GB29046@suse.de>
On Aug 4, 2010, at 11:26, Greg KH <gregkh@suse.de> wrote:
> On Wed, Aug 04, 2010 at 05:17:21PM +0000, Hank Janssen wrote:
>>>
>>
>> I will re-submit all 5 patches and CC the stable mailing list on it as well.
>
> 5? I see 6 of them here :)
>
> thanks,
>
> greg k-h
>
True but anise you rejected the todo patch there will only be 5.
Unless you are willing to take the todo patch. :)
Hank.
^ permalink raw reply
* Re: [PATCH 1/6] staging: hv: Fixed bounce kmap problem by using correct index
From: Greg KH @ 2010-08-04 18:23 UTC (permalink / raw)
To: Hank Janssen
Cc: 'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang,
'linux-kernel@vger.kernel.org'
In-Reply-To: <8AFC7968D54FB448A30D8F38F259C56223FE8EF5@TK5EX14MBXC114.redmond.corp.microsoft.com>
On Wed, Aug 04, 2010 at 05:17:21PM +0000, Hank Janssen wrote:
>
> >And then Greg spoke:
> >> And then Greg spoke:
> > >>>On Tue, Aug 03, 2010 at 05:31:51PM +0000, Hank Janssen wrote:
> > >>> From: Hank Janssen <hjanssen@microsoft.com>
> > >>>
> > >> >Fixed bounce offset kmap problem by using correct index
> > >>
>
> SNIP
>
> > > I actually goofed up in my previous code and used an incorrect index
> > > number For this part of the code. It should have shown up in more
> > > situations than what I have seen oddly enough.
> >
> >Please provide this type of information in the changelog section.
> >
> >If you want something backported to older stable kernels, add a:
> > Cc: stable <stable@kernel.org>
>
> I will re-submit all 5 patches and CC the stable mailing list on it as well.
5? I see 6 of them here :)
thanks,
greg k-h
^ permalink raw reply
* RE: [PATCH 1/6] staging: hv: Fixed bounce kmap problem by using correct index
From: Hank Janssen @ 2010-08-04 17:17 UTC (permalink / raw)
To: Greg KH
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <20100804170406.GA28217@suse.de>
>And then Greg spoke:
>> And then Greg spoke:
> >>>On Tue, Aug 03, 2010 at 05:31:51PM +0000, Hank Janssen wrote:
> >>> From: Hank Janssen <hjanssen@microsoft.com>
> >>>
> >> >Fixed bounce offset kmap problem by using correct index
> >>
SNIP
> > I actually goofed up in my previous code and used an incorrect index
> > number For this part of the code. It should have shown up in more
> > situations than what I have seen oddly enough.
>
>Please provide this type of information in the changelog section.
>
>If you want something backported to older stable kernels, add a:
> Cc: stable <stable@kernel.org>
I will re-submit all 5 patches and CC the stable mailing list on it as well.
Thanks!
Hank.
^ permalink raw reply
* Re: [PATCH 5/6] staging: hv: Increased storvsc ringbuffer and max_io_requests
From: Greg KH @ 2010-08-04 17:08 UTC (permalink / raw)
To: Hank Janssen
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <8AFC7968D54FB448A30D8F38F259C56223FE59CB@TK5EX14MBXC114.redmond.corp.microsoft.com>
On Wed, Aug 04, 2010 at 03:23:09AM +0000, Hank Janssen wrote:
>
> >>And than Greg KH spoke these words:
> >>On Tue, Aug 03, 2010 at 05:31:55PM +0000, Hank Janssen wrote:
> > >From: Hank Janssen <hjanssen@microsoft.com>
> > >
> > >Increased storvsc ringbuffer and max_io_requests. This now more
> > >closely mimics the numbers on Hyper-V.
> >
> >What do you mean by this? The max provided by the hypervisor?
>
> Max_IO is set to double from what it was before, Hyper-V allows it and
> We have had appliance builder requests to see if it was a problem to increase
> The number.
>
> Ringbuffer size for storvsc is now increased because I have seen
> A few buffer problems on extremely busy systems. They where
> Set pretty low before. And since max_io_requests is increased I
> Really needed to increase the buffer as well.
>
> >> And will allow more IO requests to take place for the SCSI driver.
> >
> >Which means what, better throughput? Or just more outstanding requests and the same speed overall?
>
> It means better throughput of high load systems, else you get buffer
> Contention.
All good, care to included this in the changelog comment when you resend
this whole series?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 3/6] staging: hv: Fixed the value of the 64bit-hole inside ring buffer
From: Greg KH @ 2010-08-04 17:05 UTC (permalink / raw)
To: Hank Janssen
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <8AFC7968D54FB448A30D8F38F259C56223FE7ADB@TK5EX14MBXC114.redmond.corp.microsoft.com>
On Wed, Aug 04, 2010 at 04:25:40AM +0000, Hank Janssen wrote:
>
> >>On Tue, Aug 03, 2010 at 05:31:57PM +0000, Hank Janssen wrote:
> >> From: Haiyang Zhang <haiyangz@microsoft.com>
> >>
> >> Fixed the value of the 64bit-hole inside ring buffer, this caused a problem on Hyper-V
> >>when running checked Windows builds.
> >
> >What was the problem, and why would this fix it? Was it a bug in this code,
> >by virtue that the code was incorrect in the client and the checked builds properly warned
> >of this, or something else?
>
> Checked builds of Windows are used internally and given to external system builders at times.
> They are builds that for example that all elements in a structure follow the definition of that
> Structure. The bug this fixed was for a field that we did not fill in at all (Because we do
> Not use it on the Linux side), and the checked build of windows gives errors on it internally
> to the Windows logs.
>
> This fixes that error.
Wonderful, please provide this information in the changelog comment.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 2/6] staging: hv: Fixed lockup problem with bounce_buffer scatter list
From: Greg KH @ 2010-08-04 17:05 UTC (permalink / raw)
To: Hank Janssen
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <8AFC7968D54FB448A30D8F38F259C56223FE7AF6@TK5EX14MBXC114.redmond.corp.microsoft.com>
On Wed, Aug 04, 2010 at 04:32:17AM +0000, Hank Janssen wrote:
> And then Greg spoke:
> >> On Tue, Aug 03, 2010 at 05:31:53PM +0000, Hank Janssen wrote:
> >> From: Hank Janssen <hjanssen@microsoft.com>
> > >
> > >Fixed lockup problem with bounce_buffer scatter list which caused crashes in heavy loads.
> >
> >Same questions for you as patch 1/6.
> >
> >Also, please wrap your changelog comments at 72 columns.
>
> Under heavy loads with many clients we this problem appears, it causes a kernel
> Panic.
Great, please provide this info.
> Yes, this should be backported to all stable kernel releases that have the
> Hyper-V drivers In them.
Again, add a cc: stable <stable@kernel.org> to the patch please.
> >> Signed-off-by:Hank Janssen <hjanssen@microsoft.com>
> >> Signed-off-by:Haiyang Zhang <haiyangz@microsoft.com>
> >
> >What's with the trailing space?
>
> Same problem as wrapping my comments at 72 chars, I just upgraded to the latest
> Evil outlook 2010, and my settings in office 2007 did not transfer so I have to
> Set up all the stuff that fixes these problems :(
>
> Do you want me to re-submit with correct wrapping?
Yes. But you also have trailing spaces in some of these patches as
well, which need to be addressed.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 1/6] staging: hv: Fixed bounce kmap problem by using correct index
From: Greg KH @ 2010-08-04 17:04 UTC (permalink / raw)
To: Hank Janssen
Cc: 'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang,
'linux-kernel@vger.kernel.org'
In-Reply-To: <8AFC7968D54FB448A30D8F38F259C56223FE8B19@TK5EX14MBXC114.redmond.corp.microsoft.com>
On Wed, Aug 04, 2010 at 04:35:06AM +0000, Hank Janssen wrote:
>
> And then Greg spoke:
> >>On Tue, Aug 03, 2010 at 05:31:51PM +0000, Hank Janssen wrote:
> >> From: Hank Janssen <hjanssen@microsoft.com>
> >>
> > >Fixed bounce offset kmap problem by using correct index
> >
> >Is this a bugfix that needs to go to previous kernel releases? If so, which ones? What is the symptom of the problem here?
>
> Yes, this should be backported to all stable kernel releases that have the Hyper-V drivers In them.
>
> The symptom of the problem is that in some NAS appliances this problem represents
> Itself by a unresponsive VM under a load with many clients writing small files.
>
> I actually goofed up in my previous code and used an incorrect index number
> For this part of the code. It should have shown up in more situations than what
> I have seen oddly enough.
Please provide this type of information in the changelog section.
If you want something backported to older stable kernels, add a:
Cc: stable <stable@kernel.org>
to the signed-off-by area of the patch and it will be automatically sent
to the stable alias when it gets included in Linus's tree so I know to
pick it up then.
Care to resend all of these patches based on these comments?
thanks,
greg k-h
^ permalink raw reply
* RE: [PATCH 1/6] staging: hv: Fixed bounce kmap problem by using correct index
From: Hank Janssen @ 2010-08-04 4:35 UTC (permalink / raw)
To: Greg KH
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <20100803173732.GA1455@suse.de>
And then Greg spoke:
>>On Tue, Aug 03, 2010 at 05:31:51PM +0000, Hank Janssen wrote:
>> From: Hank Janssen <hjanssen@microsoft.com>
>>
> >Fixed bounce offset kmap problem by using correct index
>
>Is this a bugfix that needs to go to previous kernel releases? If so, which ones? What is the symptom of the problem here?
Yes, this should be backported to all stable kernel releases that have the Hyper-V drivers In them.
The symptom of the problem is that in some NAS appliances this problem represents
Itself by a unresponsive VM under a load with many clients writing small files.
I actually goofed up in my previous code and used an incorrect index number
For this part of the code. It should have shown up in more situations than what
I have seen oddly enough.
Hank.
^ permalink raw reply
* RE: [PATCH 2/6] staging: hv: Fixed lockup problem with bounce_buffer scatter list
From: Hank Janssen @ 2010-08-04 4:32 UTC (permalink / raw)
To: Greg KH
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <20100803173857.GB1455@suse.de>
And then Greg spoke:
>> On Tue, Aug 03, 2010 at 05:31:53PM +0000, Hank Janssen wrote:
>> From: Hank Janssen <hjanssen@microsoft.com>
> >
> >Fixed lockup problem with bounce_buffer scatter list which caused crashes in heavy loads.
>
>Same questions for you as patch 1/6.
>
>Also, please wrap your changelog comments at 72 columns.
Under heavy loads with many clients we this problem appears, it causes a kernel
Panic.
Yes, this should be backported to all stable kernel releases that have the
Hyper-V drivers In them.
>> Signed-off-by:Hank Janssen <hjanssen@microsoft.com>
>> Signed-off-by:Haiyang Zhang <haiyangz@microsoft.com>
>
>What's with the trailing space?
Same problem as wrapping my comments at 72 chars, I just upgraded to the latest
Evil outlook 2010, and my settings in office 2007 did not transfer so I have to
Set up all the stuff that fixes these problems :(
Do you want me to re-submit with correct wrapping?
Thanks,
Hank.
^ permalink raw reply
* RE: [PATCH 6/6] staging: hv: Gracefully handle SCSI resets
From: Hank Janssen @ 2010-08-04 4:28 UTC (permalink / raw)
To: Greg KH
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <20100803174722.GG1455@suse.de>
And then Greg spoke;
>>On Tue, Aug 03, 2010 at 05:31:56PM +0000, Hank Janssen wrote:
>> From: Hank Janssen <hjanssen@microsoft.com>
>>
>> If we get a SCSI host bus reset we now gracefully handle it, and we take the device offline.
>> This before sometimes caused hangs.
>
>Is this a problem for all older versions as well? If so, should it be backported to the -stable kernel releases?
Yes, this should be backported to all stable kernel releases that have the Hyper-V drivers
In them. This fixes a VM freeze.
All patches I submitted in the batch of 6 (Except for the TODO patch) fix bugs. This one
Fixes the worst one.
Thanks,
Hank.
^ permalink raw reply
* RE: [PATCH 3/6] staging: hv: Fixed the value of the 64bit-hole inside ring buffer
From: Hank Janssen @ 2010-08-04 4:25 UTC (permalink / raw)
To: Greg KH
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <20100803174405.GF1455@suse.de>
>>On Tue, Aug 03, 2010 at 05:31:57PM +0000, Hank Janssen wrote:
>> From: Haiyang Zhang <haiyangz@microsoft.com>
>>
>> Fixed the value of the 64bit-hole inside ring buffer, this caused a problem on Hyper-V
>>when running checked Windows builds.
>
>What was the problem, and why would this fix it? Was it a bug in this code,
>by virtue that the code was incorrect in the client and the checked builds properly warned
>of this, or something else?
Checked builds of Windows are used internally and given to external system builders at times.
They are builds that for example that all elements in a structure follow the definition of that
Structure. The bug this fixed was for a field that we did not fill in at all (Because we do
Not use it on the Linux side), and the checked build of windows gives errors on it internally
to the Windows logs.
This fixes that error.
Thanks,
Hank.
^ permalink raw reply
* RE: [PATCH 5/6] staging: hv: Increased storvsc ringbuffer and max_io_requests
From: Hank Janssen @ 2010-08-04 3:23 UTC (permalink / raw)
To: Greg KH
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <20100803174005.GC1455@suse.de>
>>And than Greg KH spoke these words:
>>On Tue, Aug 03, 2010 at 05:31:55PM +0000, Hank Janssen wrote:
> >From: Hank Janssen <hjanssen@microsoft.com>
> >
> >Increased storvsc ringbuffer and max_io_requests. This now more
> >closely mimics the numbers on Hyper-V.
>
>What do you mean by this? The max provided by the hypervisor?
Max_IO is set to double from what it was before, Hyper-V allows it and
We have had appliance builder requests to see if it was a problem to increase
The number.
Ringbuffer size for storvsc is now increased because I have seen
A few buffer problems on extremely busy systems. They where
Set pretty low before. And since max_io_requests is increased I
Really needed to increase the buffer as well.
>> And will allow more IO requests to take place for the SCSI driver.
>
>Which means what, better throughput? Or just more outstanding requests and the same speed overall?
It means better throughput of high load systems, else you get buffer
Contention.
Thanks,
Hank.
^ permalink raw reply
* [PATCH 1/1] staging: hv: Fix missing functions for net_device_ops (with detailed comments)
From: Haiyang Zhang @ 2010-08-03 19:15 UTC (permalink / raw)
To: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
"'virtualization@lists.osdl.org'" <virtualiz>
From: Haiyang Zhang <haiyangz@microsoft.com>
Fix missing functions for net_device_ops.
It's a bug when porting Linux IC from 2.6.27 to 2.6.32. In 2.6.27,
the default functions for Ethernet, like eth_change_mtu(), were assigned
by ether_setup(). But in 2.6.32, these function pointers moved to
net_device_ops structure and no longer be assigned in ether_setup(). So we
need to set these functions in our driver code. It will ensure the MTU won't
be set beyond 1500. Otherwise, this can cause an error on the server side,
because the HyperV linux driver doesn't support jumbo frame yet.
Requesting it to be backported to kernel 2.6.32 and later, which are all
versions containing Hyper-V drivers.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
drivers/staging/hv/netvsc_drv.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index 56e1157..64a0114 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -327,6 +327,9 @@ static const struct net_device_ops device_ops = {
.ndo_stop = netvsc_close,
.ndo_start_xmit = netvsc_start_xmit,
.ndo_set_multicast_list = netvsc_set_multicast_list,
+ .ndo_change_mtu = eth_change_mtu,
+ .ndo_validate_addr = eth_validate_addr,
+ .ndo_set_mac_address = eth_mac_addr,
};
static int netvsc_probe(struct device *device)
--
1.6.3.2
^ permalink raw reply related
* Re: [PATCH 1/1] staging: hv: Fix missing functions for net_device_ops (resubmit)
From: Greg KH @ 2010-08-03 18:43 UTC (permalink / raw)
To: Haiyang Zhang
Cc: Greg KH, 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Hank Janssen
In-Reply-To: <1FB5E1D5CA062146B38059374562DF728380C2F4@TK5EX14MBXC126.redmond.corp.microsoft.com>
On Tue, Aug 03, 2010 at 06:21:50PM +0000, Haiyang Zhang wrote:
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Monday, August 02, 2010 9:30 PM
> > Why put the subject in the body of the email? This forces me to edit
> > it again :(
>
> Sure, I'll remove it.
thanks.
> > > functions. We need to put them into net_device_ops. For example,
> > > eth_change_mtu prevents setting MTU beyond 1500. This is necessary
> > > because we don't support jumbo frame at this time.
> >
> > What does this mean? Is it a bug? Just not going as fast as possible?
> > Something else?
>
> Yes, it's a bug when porting Linux IC from 2.6.27 to 2.6.32. In 2.6.27,
> the default functions for Ethernet, like eth_change_mtu(), were assigned
> by ether_setup(). But in 2.6.32, these function pointers moved to
> net_device_ops structure and no longer be assigned in ether_setup(). So we
> need to set these functions in our driver code. It will ensure the MTU won't
> be set beyong 1500. Otherwise, this can cause an error on the server side,
> because the HyperV linux driver doesn't support jumbo frame yet.
Wonderful, why not put this type of full information in the patch itself
so that people can understand and rememeber what is going on when they
look at it in a few months time?
> > > It affects kernel 2.6.32 and later, which are all versions
> > > containing Hyper-V drivers.
> >
> > So is this fixing a bug that needs to be backported to these older
> > kernels (i.e. the staging tree?) Is this something that is causing
> > people problems?
>
> Yes it should be backported. The bug can cause error on guest and host.
Than please ask for it to be backported in the future.
thanks,
greg k-h
^ permalink raw reply
* [PATCH 1/1] staging: hv: Fix missing functions for net_device_ops (submit again)
From: Haiyang Zhang @ 2010-08-03 18:25 UTC (permalink / raw)
To: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
"'virtualization@lists.osdl.org'" <virtualiz>
From: Haiyang Zhang <haiyangz@microsoft.com>
Fix missing functions for net_device_ops.
Unlike in older kernels, ether_setup() no longer sets default
functions. We need to put them into net_device_ops. For example,
eth_change_mtu prevents setting MTU beyond 1500. This is necessary
because we don't support jumbo frame at this time.
It affects kernel 2.6.32 and later, which are all versions
containing Hyper-V drivers.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
---
drivers/staging/hv/netvsc_drv.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/staging/hv/netvsc_drv.c b/drivers/staging/hv/netvsc_drv.c
index 56e1157..64a0114 100644
--- a/drivers/staging/hv/netvsc_drv.c
+++ b/drivers/staging/hv/netvsc_drv.c
@@ -327,6 +327,9 @@ static const struct net_device_ops device_ops = {
.ndo_stop = netvsc_close,
.ndo_start_xmit = netvsc_start_xmit,
.ndo_set_multicast_list = netvsc_set_multicast_list,
+ .ndo_change_mtu = eth_change_mtu,
+ .ndo_validate_addr = eth_validate_addr,
+ .ndo_set_mac_address = eth_mac_addr,
};
static int netvsc_probe(struct device *device)
--
1.6.3.2
^ permalink raw reply related
* RE: [PATCH 1/1] staging: hv: Fix missing functions for net_device_ops (resubmit)
From: Haiyang Zhang @ 2010-08-03 18:21 UTC (permalink / raw)
To: Greg KH
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', 'gregkh@suse.de',
Hank Janssen
In-Reply-To: <20100803013013.GC3814@kroah.com>
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Monday, August 02, 2010 9:30 PM
> Why put the subject in the body of the email? This forces me to edit
> it again :(
Sure, I'll remove it.
> > functions. We need to put them into net_device_ops. For example,
> > eth_change_mtu prevents setting MTU beyond 1500. This is necessary
> > because we don't support jumbo frame at this time.
>
> What does this mean? Is it a bug? Just not going as fast as possible?
> Something else?
Yes, it's a bug when porting Linux IC from 2.6.27 to 2.6.32. In 2.6.27,
the default functions for Ethernet, like eth_change_mtu(), were assigned
by ether_setup(). But in 2.6.32, these function pointers moved to
net_device_ops structure and no longer be assigned in ether_setup(). So we
need to set these functions in our driver code. It will ensure the MTU won't
be set beyong 1500. Otherwise, this can cause an error on the server side,
because the HyperV linux driver doesn't support jumbo frame yet.
> > It affects kernel 2.6.32 and later, which are all versions
> > containing Hyper-V drivers.
>
> So is this fixing a bug that needs to be backported to these older
> kernels (i.e. the staging tree?) Is this something that is causing
> people problems?
Yes it should be backported. The bug can cause error on guest and host.
Thanks,
- Haiyang
^ permalink raw reply
* Re: [PATCH 6/6] staging: hv: Gracefully handle SCSI resets
From: Greg KH @ 2010-08-03 17:47 UTC (permalink / raw)
To: Hank Janssen
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <8AFC7968D54FB448A30D8F38F259C56223FD1D3C@TK5EX14MBXC114.redmond.corp.microsoft.com>
On Tue, Aug 03, 2010 at 05:31:56PM +0000, Hank Janssen wrote:
> From: Hank Janssen <hjanssen@microsoft.com>
>
> If we get a SCSI host bus reset we now gracefully handle it, and we take the device offline.
> This before sometimes caused hangs.
Is this a problem for all older versions as well? If so, should it be
backported to the -stable kernel releases?
>
> Signed-off-by:Hank Janssen <hjanssen@microsoft.com>
> Signed-off-by:Haiyang Zhang <haiyangz@microsoft.com>
>
>
> ---
> drivers/staging/hv/storvsc.c | 36 +++++++++++++++++++++++++++++++++++-
> 1 files changed, 35 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/staging/hv/storvsc.c b/drivers/staging/hv/storvsc.c index 6bd2ff1..5f222cf 100644
> --- a/drivers/staging/hv/storvsc.c
> +++ b/drivers/staging/hv/storvsc.c
> @@ -48,7 +48,9 @@ struct storvsc_device {
>
> /* 0 indicates the device is being destroyed */
> atomic_t RefCount;
> -
> +
Trailing whitespace :(
> + int reset;
Can't this be a bool?
> + spinlock_t lock;
> atomic_t NumOutstandingRequests;
>
> /*
> @@ -93,6 +95,9 @@ static inline struct storvsc_device *AllocStorDevice(struct hv_device *Device)
> atomic_cmpxchg(&storDevice->RefCount, 0, 2);
>
> storDevice->Device = Device;
> + storDevice->reset = 0;
> + spin_lock_init(&storDevice->lock);
> +
> Device->Extension = storDevice;
>
> return storDevice;
> @@ -101,6 +106,7 @@ static inline struct storvsc_device *AllocStorDevice(struct hv_device *Device) static inline void FreeStorDevice(struct storvsc_device *Device) {
> /* ASSERT(atomic_read(&Device->RefCount) == 0); */
> + /*kfree(Device->lock);*/
Why add a commented out line? Especially one that is incorrect? :)
> kfree(Device);
> }
>
> @@ -108,13 +114,24 @@ static inline void FreeStorDevice(struct storvsc_device *Device) static inline struct storvsc_device *GetStorDevice(struct hv_device *Device) {
> struct storvsc_device *storDevice;
> + unsigned long flags;
>
> storDevice = (struct storvsc_device *)Device->Extension;
> +
> + spin_lock_irqsave(&storDevice->lock, flags);
> +
> + if (storDevice->reset == 1) {
> + spin_unlock_irqrestore(&storDevice->lock, flags);
> + return NULL;
Don't return here, jump to the end of the function and return there.
That way you only have one lock/unlock pair and it's much easier to
maintain and audit over time that you got everything correct.
> + }
> +
> if (storDevice && atomic_read(&storDevice->RefCount) > 1)
> atomic_inc(&storDevice->RefCount);
> else
> storDevice = NULL;
>
> + spin_unlock_irqrestore(&storDevice->lock, flags);
> +
> return storDevice;
> }
>
> @@ -122,13 +139,19 @@ static inline struct storvsc_device *GetStorDevice(struct hv_device *Device) static inline struct storvsc_device *MustGetStorDevice(struct hv_device *Device) {
> struct storvsc_device *storDevice;
> + unsigned long flags;
>
> storDevice = (struct storvsc_device *)Device->Extension;
> +
> + spin_lock_irqsave(&storDevice->lock, flags);
> +
> if (storDevice && atomic_read(&storDevice->RefCount))
> atomic_inc(&storDevice->RefCount);
> else
> storDevice = NULL;
>
> + spin_unlock_irqrestore(&storDevice->lock, flags);
> +
> return storDevice;
> }
>
> @@ -614,6 +637,7 @@ int StorVscOnHostReset(struct hv_device *Device)
> struct storvsc_device *storDevice;
> struct storvsc_request_extension *request;
> struct vstor_packet *vstorPacket;
> + unsigned long flags;
> int ret;
>
> DPRINT_INFO(STORVSC, "resetting host adapter..."); @@ -625,6 +649,16 @@ int StorVscOnHostReset(struct hv_device *Device)
> return -1;
> }
>
> + spin_lock_irqsave(&storDevice->lock, flags);
> + storDevice->reset = 1;
> + spin_unlock_irqrestore(&storDevice->lock, flags);
> +
> + /*
> + * Wait for traffic in transit to complete
> + */
> + while (atomic_read(&storDevice->NumOutstandingRequests))
> + udelay(1000);
What's ever going to get us out of this loop? You need a fall-back in
case this read never succeeds.
And why an atomic value if you have a lock protecting it? That's major
overkill and is probably not needed.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 3/6] staging: hv: Fixed the value of the 64bit-hole inside ring buffer
From: Greg KH @ 2010-08-03 17:44 UTC (permalink / raw)
To: Hank Janssen
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <8AFC7968D54FB448A30D8F38F259C56223FD1D45@TK5EX14MBXC114.redmond.corp.microsoft.com>
On Tue, Aug 03, 2010 at 05:31:57PM +0000, Hank Janssen wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> Fixed the value of the 64bit-hole inside ring buffer, this caused a problem on Hyper-V when running checked Windows builds.
What was the problem, and why would this fix it? Was it a bug in this
code, by virtue that the code was incorrect in the client and the
checked builds properly warned of this, or something else?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 4/6] staging: hv: Removed TODO for VMBUS, it mirrors HYPER-V behavior to closely
From: Greg KH @ 2010-08-03 17:42 UTC (permalink / raw)
To: Hank Janssen
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <8AFC7968D54FB448A30D8F38F259C56223FD1D2F@TK5EX14MBXC114.redmond.corp.microsoft.com>
On Tue, Aug 03, 2010 at 05:31:54PM +0000, Hank Janssen wrote:
> From: Hank Janssen <hjanssen@microsoft.com>
>
> Removed TODO for VMBUS, it mirrors HYPER-V behaviour to closely and does not really share any
> usable functionality of the Linux kernel facilities. There is not really an easy way to combine functionality.
Are you sure about that? Right now, the code doesn't look like any
other virtual bus code, but that's because it needs to be fixed up and
work properly (not with the two layers of indirection that was added a
long time ago before the code was opened up.)
I think once the code is cleaned up properly, you might find that it can
be combined much easier than you think...
So no, I'll not allow this TODO item to be removed until after the vbus
code is fixed up, and you can convince me that it really is unique
enough to remain as-is.
Oh, and again, please properly line-wrap your changelog comments.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 5/6] staging: hv: Increased storvsc ringbuffer and max_io_requests
From: Greg KH @ 2010-08-03 17:40 UTC (permalink / raw)
To: Hank Janssen
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <8AFC7968D54FB448A30D8F38F259C56223FD1D36@TK5EX14MBXC114.redmond.corp.microsoft.com>
On Tue, Aug 03, 2010 at 05:31:55PM +0000, Hank Janssen wrote:
> From: Hank Janssen <hjanssen@microsoft.com>
>
> Increased storvsc ringbuffer and max_io_requests. This now more closely mimics
> the numbers on Hyper-V. And will allow more IO requests to take place for the SCSI driver.
>
> Signed-off-by:Hank Janssen <hjanssen@microsoft.com>
> Signed-off-by:Haiyang Zhang <haiyangz@microsoft.com>
Again with the trailing spaces :(
^ permalink raw reply
* Re: [PATCH 5/6] staging: hv: Increased storvsc ringbuffer and max_io_requests
From: Greg KH @ 2010-08-03 17:40 UTC (permalink / raw)
To: Hank Janssen
Cc: 'linux-kernel@vger.kernel.org',
'devel@driverdev.osuosl.org',
'virtualization@lists.osdl.org', Haiyang Zhang
In-Reply-To: <8AFC7968D54FB448A30D8F38F259C56223FD1D36@TK5EX14MBXC114.redmond.corp.microsoft.com>
On Tue, Aug 03, 2010 at 05:31:55PM +0000, Hank Janssen wrote:
> From: Hank Janssen <hjanssen@microsoft.com>
>
> Increased storvsc ringbuffer and max_io_requests. This now more closely mimics
> the numbers on Hyper-V.
What do you mean by this? The max provided by the hypervisor?
> And will allow more IO requests to take place for the SCSI driver.
Which means what, better throughput? Or just more outstanding requests
and the same speed overall?
thanks,
greg k-h
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox