* Re: [PATCH 41/46] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init()
From: Dan Carpenter @ 2011-08-30 10:29 UTC (permalink / raw)
To: Greg KH
Cc: K. Y. Srinivasan, devel, Haiyang Zhang, gregkh, linux-kernel,
virtualization
In-Reply-To: <20110829180842.GA23618@kroah.com>
> err3:
> free_irq(irq, hv_acpi_dev);
> err2:
> bus_unregister(&hv_bus);
> err1:
> hv_cleanup();
Also here is an oldbie trick. You could use multiples of ten like
err30, err20, and err10. That way if you can add more error handling
in the middle without changing the numbering. I knew my GW-BASIC
experience would come in handy one day. :)
The better way to label things is based on what happens when you get
there: err_irq, err_unregister, err_cleanup.
Some people label things based on where the goto is, but that breaks
down if there is more than one goto. It doesn't really make sense to
name the destination after the starting point.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size
From: Rusty Russell @ 2011-08-30 0:12 UTC (permalink / raw)
To: Wang Sheng-Hui; +Cc: wanlong.gao, virtualization, linux-kernel, mst
In-Reply-To: <4E5B460F.7070707@gmail.com>
On Mon, 29 Aug 2011 15:55:59 +0800, Wang Sheng-Hui <shhuiw@gmail.com> wrote:
> Patch regerated to use sizeof(__u16) instead of 16 in vring_init.
> Please check it.
Applied.
Thanks,
Rusty.
^ permalink raw reply
* Re: [PATCH 0000/0046] Staging: hv: Driver cleanup
From: Greg KH @ 2011-08-29 18:15 UTC (permalink / raw)
To: K. Y. Srinivasan; +Cc: gregkh, linux-kernel, devel, virtualization
In-Reply-To: <1314469874-7017-1-git-send-email-kys@microsoft.com>
On Sat, Aug 27, 2011 at 11:31:14AM -0700, K. Y. Srinivasan wrote:
> Further cleanup of the hv drivers.
>
> 1) Cleanup reference counting.
>
> 2) Handle all block devices using the storvsc driver. I have modified
> the implementation here based on Christoph's feedback on my earlier
> implementation.
>
> 3) Fix bugs.
>
> 4) Accomodate some host side scsi emulation bugs.
>
> 5) In case of scsi errors off-line the device.
>
> This patch-set further reduces the size of Hyper-V drivers - the code is
> about 10% smaller. This reduction is mainly because we have eliminated the
> blkvsc driver.
If my tracking is correct, I applied everything in this series except
the following patches
[PATCH 37/46] Staging: hv: vmbus: Check for events before messages
[PATCH 41/46] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init()
[PATCH 46/46] Staging: hv: Update the TODO file
Please fix up patches 37 and 41 and resend, and for 46, that should be
totally different as mentioned, but you might want to send a patch that
at the least adds your name to the file at the bottom, as you wanted.
Also note that the MAINTAINERS file should also be adjusted to match.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 46/46] Staging: hv: Update the TODO file
From: Greg KH @ 2011-08-29 18:12 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang
In-Reply-To: <1314469905-7058-46-git-send-email-kys@microsoft.com>
On Sat, Aug 27, 2011 at 11:31:45AM -0700, K. Y. Srinivasan wrote:
> Update the TODO file.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> drivers/staging/hv/TODO | 24 ++++++++++++++++++++----
> 1 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/hv/TODO b/drivers/staging/hv/TODO
> index 582fd4a..23c74ed 100644
> --- a/drivers/staging/hv/TODO
> +++ b/drivers/staging/hv/TODO
> @@ -1,14 +1,30 @@
> TODO:
> - - fix remaining checkpatch warnings and errors
> - audit the vmbus to verify it is working properly with the
> driver model
> - - see if the vmbus can be merged with the other virtual busses
> - in the kernel
> +
> + STATUS (July 19, 2011):
> + All outstanding issues (known to us) with regards
> + to conforming to Linux Driver Model have been addressed.
The TODO file is not a place to have a "conversation" about what is and
is not completed by having status reports.
Let's discuss this through email, and if we all agree, then the TODO
entries can be removed.
greg k-h
^ permalink raw reply
* Re: [PATCH 44/46] Staging: hv: vmbus: Fix checkpatch warnings in connection.c
From: Greg KH @ 2011-08-29 18:09 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: devel, Haiyang Zhang, gregkh, linux-kernel, virtualization
In-Reply-To: <1314469905-7058-44-git-send-email-kys@microsoft.com>
On Sat, Aug 27, 2011 at 11:31:43AM -0700, K. Y. Srinivasan wrote:
> Fix checkpatch warnings in connection.c.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> drivers/staging/hv/connection.c | 13 +++++++------
> 1 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/hv/connection.c b/drivers/staging/hv/connection.c
> index ca92ca3..9e99c04 100644
> --- a/drivers/staging/hv/connection.c
> +++ b/drivers/staging/hv/connection.c
> @@ -220,11 +220,11 @@ static void process_chn_event(u32 relid)
> channel = relid2channel(relid);
>
> spin_lock_irqsave(&channel->inbound_lock, flags);
> - if (channel && (channel->onchannel_callback != NULL)) {
> + if (channel && (channel->onchannel_callback != NULL))
> channel->onchannel_callback(channel->channel_callback_context);
> - } else {
I agree with Joe here, if channel really was NULL, you just oopsed.
I'll apply this one, but please send me a follow-on one fixing this bug.
greg k-h
^ permalink raw reply
* Re: [PATCH 41/46] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init()
From: Greg KH @ 2011-08-29 18:08 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: devel, Haiyang Zhang, gregkh, linux-kernel, virtualization
In-Reply-To: <1314469905-7058-41-git-send-email-kys@microsoft.com>
On Sat, Aug 27, 2011 at 11:31:40AM -0700, K. Y. Srinivasan wrote:
> Fix a bug in error handling in vmbus_bus_init().
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> drivers/staging/hv/vmbus_drv.c | 21 ++++++++++-----------
> 1 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
> index 02edb11..4d1b123 100644
> --- a/drivers/staging/hv/vmbus_drv.c
> +++ b/drivers/staging/hv/vmbus_drv.c
> @@ -492,7 +492,7 @@ static int vmbus_bus_init(int irq)
>
> ret = bus_register(&hv_bus);
> if (ret)
> - return ret;
> + goto err1;
>
> ret = request_irq(irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
> driver_name, hv_acpi_dev);
> @@ -500,10 +500,7 @@ static int vmbus_bus_init(int irq)
> if (ret != 0) {
> pr_err("Unable to request IRQ %d\n",
> irq);
> -
> - bus_unregister(&hv_bus);
> -
> - return ret;
> + goto err2;
> }
>
> vector = IRQ0_VECTOR + irq;
> @@ -514,16 +511,18 @@ static int vmbus_bus_init(int irq)
> */
> on_each_cpu(hv_synic_init, (void *)&vector, 1);
> ret = vmbus_connect();
> - if (ret) {
> - free_irq(irq, hv_acpi_dev);
> - bus_unregister(&hv_bus);
> - return ret;
> - }
> -
> + if (ret)
> + goto err3;
>
> vmbus_request_offers();
>
> return 0;
> +
> +err3: free_irq(irq, hv_acpi_dev);
> +err2: bus_unregister(&hv_bus);
> +err1: hv_cleanup();
The traditional way to write this is:
err3:
free_irq(irq, hv_acpi_dev);
err2:
bus_unregister(&hv_bus);
err1:
hv_cleanup();
Care to fix this up and resend it?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 37/46] Staging: hv: vmbus: Check for events before messages
From: Greg KH @ 2011-08-29 18:05 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: devel, Haiyang Zhang, gregkh, linux-kernel, virtualization
In-Reply-To: <1314469905-7058-37-git-send-email-kys@microsoft.com>
On Sat, Aug 27, 2011 at 11:31:36AM -0700, K. Y. Srinivasan wrote:
> Conform to Windows specification by checking for events before messages.
What specification?
Care to provide a comment in the code that you are doing this in this
explicit order because of some rule that the hypervisor imposes on us?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 35/46] Staging: hv: Fix a bug in vmbus_match()
From: Greg KH @ 2011-08-29 18:00 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang
In-Reply-To: <1314469905-7058-35-git-send-email-kys@microsoft.com>
On Sat, Aug 27, 2011 at 11:31:34AM -0700, K. Y. Srinivasan wrote:
> The recent checkin that add a private pointer to hv_vmbus_device_id
> introduced this bug in vmbus_match; fix it.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> drivers/staging/hv/vmbus_drv.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
> index be62b62..51002c0 100644
> --- a/drivers/staging/hv/vmbus_drv.c
> +++ b/drivers/staging/hv/vmbus_drv.c
> @@ -273,7 +273,7 @@ static int vmbus_match(struct device *device, struct device_driver *driver)
>
> for (; !is_null_guid(id_array->guid); id_array++)
> if (!memcmp(&id_array->guid, &hv_dev->dev_type.b,
> - sizeof(struct hv_vmbus_device_id)))
> + sizeof(uuid_le)))
Ick, sorry about this, it was my fault.
greg k-h
^ permalink raw reply
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size
From: Wang Sheng-Hui @ 2011-08-29 7:55 UTC (permalink / raw)
To: Rusty Russell; +Cc: wanlong.gao, virtualization, linux-kernel, mst
In-Reply-To: <87fwkl2ajr.fsf@rustcorp.com.au>
On 2011年08月29日 10:53, Rusty Russell wrote:
> On Sat, 27 Aug 2011 17:52:02 +0800, Wang Sheng-Hui <shhuiw@gmail.com> wrote:
>> On 2011年08月27日 17:34, Wang Sheng-Hui wrote:
>> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
>> index 4a32cb6..300af76 100644
>> --- a/include/linux/virtio_ring.h
>> +++ b/include/linux/virtio_ring.h
>> @@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
>> vr->num = num;
>> vr->desc = p;
>> vr->avail = p + num*sizeof(struct vring_desc);
>> - vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
>> - & ~(align - 1));
>> + vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
>> + + align-1) & ~(align - 1));
>> }
>
> This + 16 should be + sizeof(__u16), right? It's just the
> used_event_idx which has been added:
>
> * __u16 available[num];
> * __u16 used_event_idx;
> *
> * // Padding to the next align boundary.
> * char pad[];
> *
> * [USED]
>
>> static inline unsigned vring_size(unsigned int num, unsigned long align)
>> {
>> - return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
>> + return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
>> + align - 1) & ~(align - 1))
>> + sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
>
> This is correct.
>
> And, yes, since align is always 4096, it's currently just a cleanup, but
> it makes things much less confusing!
>
> Thanks,
> Rusty.
Patch regerated to use sizeof(__u16) instead of 16 in vring_init.
Please check it.
[PATCH] virtio: modify vring_init and vring_size to take account of the layout containing *_event_idx
The patch is against 3.1-rc3.
Based on the layout description in the comments, take account of
the *_event_idx in functions vring_init and vring_size.
Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
---
include/linux/virtio_ring.h | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 4a32cb6..2a731bd 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
vr->num = num;
vr->desc = p;
vr->avail = p + num*sizeof(struct vring_desc);
- vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
- & ~(align - 1));
+ vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + sizeof(__16)
+ + align-1) & ~(align - 1));
}
static inline unsigned vring_size(unsigned int num, unsigned long align)
{
- return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
+ return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
+ align - 1) & ~(align - 1))
+ sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
}
--
1.7.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size
From: Wang Sheng-Hui @ 2011-08-29 6:51 UTC (permalink / raw)
To: Rusty Russell; +Cc: wanlong.gao, virtualization, linux-kernel, mst
In-Reply-To: <87fwkl2ajr.fsf@rustcorp.com.au>
On 2011年08月29日 10:53, Rusty Russell wrote:
> On Sat, 27 Aug 2011 17:52:02 +0800, Wang Sheng-Hui <shhuiw@gmail.com> wrote:
>> On 2011年08月27日 17:34, Wang Sheng-Hui wrote:
>> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
>> index 4a32cb6..300af76 100644
>> --- a/include/linux/virtio_ring.h
>> +++ b/include/linux/virtio_ring.h
>> @@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
>> vr->num = num;
>> vr->desc = p;
>> vr->avail = p + num*sizeof(struct vring_desc);
>> - vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
>> - & ~(align - 1));
>> + vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
>> + + align-1) & ~(align - 1));
>> }
>
> This + 16 should be + sizeof(__u16), right? It's just the
> used_event_idx which has been added:
Yes.
>
> * __u16 available[num];
> * __u16 used_event_idx;
> *
> * // Padding to the next align boundary.
> * char pad[];
> *
> * [USED]
>
>> static inline unsigned vring_size(unsigned int num, unsigned long align)
>> {
>> - return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
>> + return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
>> + align - 1) & ~(align - 1))
>> + sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
>
> This is correct.
>
> And, yes, since align is always 4096, it's currently just a cleanup, but
> it makes things much less confusing!
>
> Thanks,
> Rusty.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size
From: Rusty Russell @ 2011-08-29 2:53 UTC (permalink / raw)
To: Wang Sheng-Hui, wanlong.gao; +Cc: virtualization, linux-kernel, mst
In-Reply-To: <4E58BE42.8030700@gmail.com>
On Sat, 27 Aug 2011 17:52:02 +0800, Wang Sheng-Hui <shhuiw@gmail.com> wrote:
> On 2011年08月27日 17:34, Wang Sheng-Hui wrote:
> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
> index 4a32cb6..300af76 100644
> --- a/include/linux/virtio_ring.h
> +++ b/include/linux/virtio_ring.h
> @@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
> vr->num = num;
> vr->desc = p;
> vr->avail = p + num*sizeof(struct vring_desc);
> - vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
> - & ~(align - 1));
> + vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
> + + align-1) & ~(align - 1));
> }
This + 16 should be + sizeof(__u16), right? It's just the
used_event_idx which has been added:
* __u16 available[num];
* __u16 used_event_idx;
*
* // Padding to the next align boundary.
* char pad[];
*
* [USED]
> static inline unsigned vring_size(unsigned int num, unsigned long align)
> {
> - return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
> + return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
> + align - 1) & ~(align - 1))
> + sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
This is correct.
And, yes, since align is always 4096, it's currently just a cleanup, but
it makes things much less confusing!
Thanks,
Rusty.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size
From: Wang Sheng-Hui @ 2011-08-28 12:48 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: wanlong.gao, linux-kernel, virtualization
In-Reply-To: <20110828113132.GB4875@redhat.com>
On 2011年08月28日 19:31, Michael S. Tsirkin wrote:
> On Sat, Aug 27, 2011 at 05:52:02PM +0800, Wang Sheng-Hui wrote:
>> On 2011年08月27日 17:34, Wang Sheng-Hui wrote:
>>> On 2011年08月27日 10:49, Wanlong Gao wrote:
>>>> On Sat, 2011-08-27 at 09:07 +0800, Wang Sheng-Hui wrote:
>>>>> The patch is against 3.1-rc3.
>>>>>
>>>>> struct vring_used has two __u16 fields plus array of struct vring_used_elem.
>>>>> Current vring_size counts the __u16 fields to 3. Fix it to 2 in the patch.
>>>>>
>>>>> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
>>>>> ---
>>>>> include/linux/virtio_ring.h | 2 +-
>>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
>>>>> index 4a32cb6..fcda152 100644
>>>>> --- a/include/linux/virtio_ring.h
>>>>> +++ b/include/linux/virtio_ring.h
>>>>> @@ -143,7 +143,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
>>>>> {
>>>>> return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
>>>>> + align - 1) & ~(align - 1))
>>>>> - + sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
>>>>> + + sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num;
>>>>> }
>>>>>
>>>>> /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */
>>>>
>>>> Hi:
>>>> I'm not deep into it, but I think you can see this:
>>>> http://marc.info/?l=git-commits-head&m=130687915816130&w=2
>>>>
>>>> Thanks
>>>> -Wanlong Gao
>>>>
>>>
>>> Thanks.
>>>
>>> I checked the comments, and think we should patch vring_init
>>> and vring_size according to the layout description in the comments.
>>> Right?
>>
>> New patch generated. Please check it. Thanks,
>
>
>> [PATCH] virtio: modify vring_init and vring_size to take account of the layout containing *_event_idx
>>
>> The patch is against 3.1-rc3.
>>
>> Based on the layout description in the comments, take account of
>> the *_event_idx in functions vring_init and vring_size.
>>
>> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
>
> Is this a bugfix or just a cleanup?
> Alignment makes us get the same values with and without, right?
It's just a cleanup.
For normal case, the value 16 is too small compared with
the alignment, I think.
>
>> ---
>> include/linux/virtio_ring.h | 6 +++---
>> 1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
>> index 4a32cb6..300af76 100644
>> --- a/include/linux/virtio_ring.h
>> +++ b/include/linux/virtio_ring.h
>> @@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
>> vr->num = num;
>> vr->desc = p;
>> vr->avail = p + num*sizeof(struct vring_desc);
>> - vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
>> - & ~(align - 1));
>> + vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
>> + + align-1) & ~(align - 1));
>> }
>>
>> static inline unsigned vring_size(unsigned int num, unsigned long align)
>> {
>> - return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
>> + return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
>> + align - 1) & ~(align - 1))
>> + sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
>> }
>> --
>> 1.7.1
>>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size
From: Michael S. Tsirkin @ 2011-08-28 11:31 UTC (permalink / raw)
To: Wang Sheng-Hui; +Cc: wanlong.gao, linux-kernel, virtualization
In-Reply-To: <4E58BE42.8030700@gmail.com>
On Sat, Aug 27, 2011 at 05:52:02PM +0800, Wang Sheng-Hui wrote:
> On 2011年08月27日 17:34, Wang Sheng-Hui wrote:
> > On 2011年08月27日 10:49, Wanlong Gao wrote:
> >> On Sat, 2011-08-27 at 09:07 +0800, Wang Sheng-Hui wrote:
> >>> The patch is against 3.1-rc3.
> >>>
> >>> struct vring_used has two __u16 fields plus array of struct vring_used_elem.
> >>> Current vring_size counts the __u16 fields to 3. Fix it to 2 in the patch.
> >>>
> >>> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
> >>> ---
> >>> include/linux/virtio_ring.h | 2 +-
> >>> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
> >>> index 4a32cb6..fcda152 100644
> >>> --- a/include/linux/virtio_ring.h
> >>> +++ b/include/linux/virtio_ring.h
> >>> @@ -143,7 +143,7 @@ static inline unsigned vring_size(unsigned int num, unsigned long align)
> >>> {
> >>> return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
> >>> + align - 1) & ~(align - 1))
> >>> - + sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
> >>> + + sizeof(__u16) * 2 + sizeof(struct vring_used_elem) * num;
> >>> }
> >>>
> >>> /* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */
> >>
> >> Hi:
> >> I'm not deep into it, but I think you can see this:
> >> http://marc.info/?l=git-commits-head&m=130687915816130&w=2
> >>
> >> Thanks
> >> -Wanlong Gao
> >>
> >
> > Thanks.
> >
> > I checked the comments, and think we should patch vring_init
> > and vring_size according to the layout description in the comments.
> > Right?
>
> New patch generated. Please check it. Thanks,
> [PATCH] virtio: modify vring_init and vring_size to take account of the layout containing *_event_idx
>
> The patch is against 3.1-rc3.
>
> Based on the layout description in the comments, take account of
> the *_event_idx in functions vring_init and vring_size.
>
> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
Is this a bugfix or just a cleanup?
Alignment makes us get the same values with and without, right?
> ---
> include/linux/virtio_ring.h | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
> index 4a32cb6..300af76 100644
> --- a/include/linux/virtio_ring.h
> +++ b/include/linux/virtio_ring.h
> @@ -135,13 +135,13 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
> vr->num = num;
> vr->desc = p;
> vr->avail = p + num*sizeof(struct vring_desc);
> - vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + align-1)
> - & ~(align - 1));
> + vr->used = (void *)(((unsigned long)&vr->avail->ring[num] + 16
> + + align-1) & ~(align - 1));
> }
>
> static inline unsigned vring_size(unsigned int num, unsigned long align)
> {
> - return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (2 + num)
> + return ((sizeof(struct vring_desc) * num + sizeof(__u16) * (3 + num)
> + align - 1) & ~(align - 1))
> + sizeof(__u16) * 3 + sizeof(struct vring_used_elem) * num;
> }
> --
> 1.7.1
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 44/46] Staging: hv: vmbus: Fix checkpatch warnings in connection.c
From: Joe Perches @ 2011-08-28 6:56 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: devel, Haiyang Zhang, gregkh, linux-kernel, virtualization
In-Reply-To: <1314469905-7058-44-git-send-email-kys@microsoft.com>
On Sat, 2011-08-27 at 11:31 -0700, K. Y. Srinivasan wrote:
> Fix checkpatch warnings in connection.c.
[]
> diff --git a/drivers/staging/hv/connection.c b/drivers/staging/hv/connection.c
[]
> @@ -220,11 +220,11 @@ static void process_chn_event(u32 relid)
> channel = relid2channel(relid);
>
> spin_lock_irqsave(&channel->inbound_lock, flags);
> - if (channel && (channel->onchannel_callback != NULL)) {
> + if (channel && (channel->onchannel_callback != NULL))
Useless test for channel or bad placement for spin_lock.
channel has already been dereferenced by the spin_lock.
^ permalink raw reply
* [PATCH 46/46] Staging: hv: Update the TODO file
From: K. Y. Srinivasan @ 2011-08-27 18:31 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang
In-Reply-To: <1314469905-7058-1-git-send-email-kys@microsoft.com>
Update the TODO file.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/TODO | 24 ++++++++++++++++++++----
1 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/hv/TODO b/drivers/staging/hv/TODO
index 582fd4a..23c74ed 100644
--- a/drivers/staging/hv/TODO
+++ b/drivers/staging/hv/TODO
@@ -1,14 +1,30 @@
TODO:
- - fix remaining checkpatch warnings and errors
- audit the vmbus to verify it is working properly with the
driver model
- - see if the vmbus can be merged with the other virtual busses
- in the kernel
+
+ STATUS (July 19, 2011):
+ All outstanding issues (known to us) with regards
+ to conforming to Linux Driver Model have been addressed.
+
- audit the network driver
- checking for carrier inside open is wrong, network device API
confusion??
+
+ STATUS (July 19, 2011):
+ The network driver has been reviewed by the community.
+ We have addressed the various issues that were raised in this
+ review.
+
- audit the block driver
- audit the scsi driver
+ STATUS (July 19, 2011):
+ One of the longstanding comment on this body of code
+ has been to merge the block and scsi driver. This has been done
+ and patches have been submitted. We have also addressed all the
+ review comments on this body of code.
+
+
Please send patches for this code to Greg Kroah-Hartman <gregkh@suse.de>,
-Hank Janssen <hjanssen@microsoft.com>, and Haiyang Zhang <haiyangz@microsoft.com>.
+Hank Janssen <hjanssen@microsoft.com>, and Haiyang Zhang <haiyangz@microsoft.com>,
+K. Y. Srinivasan <kys@microsoft.com>.
--
1.7.4.1
^ permalink raw reply related
* [PATCH 45/46] Staging: hv: mousevsc: Fix checkpatch errors and warnings
From: K. Y. Srinivasan @ 2011-08-27 18:31 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang
In-Reply-To: <1314469905-7058-1-git-send-email-kys@microsoft.com>
Fix checkpatch errors and warnings.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/hv_mouse.c | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
index 090736a..dbb04ee 100644
--- a/drivers/staging/hv/hv_mouse.c
+++ b/drivers/staging/hv/hv_mouse.c
@@ -335,7 +335,8 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
input_device->hid_desc = kzalloc(desc->bLength, GFP_KERNEL);
if (!input_device->hid_desc) {
- pr_err("unable to allocate hid descriptor - size %d", desc->bLength);
+ pr_err("unable to allocate hid descriptor - size %d",
+ desc->bLength);
goto cleanup;
}
@@ -598,12 +599,12 @@ static int mousevsc_connect_to_vsp(struct hv_device *device)
pr_info("synthhid protocol request...");
ret = vmbus_sendpacket(device->channel, request,
- sizeof(struct pipe_prt_msg) -
- sizeof(unsigned char) +
- sizeof(struct synthhid_protocol_request),
- (unsigned long)request,
- VM_PKT_DATA_INBAND,
- VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
+ sizeof(struct pipe_prt_msg) -
+ sizeof(unsigned char) +
+ sizeof(struct synthhid_protocol_request),
+ (unsigned long)request,
+ VM_PKT_DATA_INBAND,
+ VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
if (ret != 0) {
pr_err("unable to send synthhid protocol request.");
goto cleanup;
--
1.7.4.1
^ permalink raw reply related
* [PATCH 44/46] Staging: hv: vmbus: Fix checkpatch warnings in connection.c
From: K. Y. Srinivasan @ 2011-08-27 18:31 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang
In-Reply-To: <1314469905-7058-1-git-send-email-kys@microsoft.com>
Fix checkpatch warnings in connection.c.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/connection.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/hv/connection.c b/drivers/staging/hv/connection.c
index ca92ca3..9e99c04 100644
--- a/drivers/staging/hv/connection.c
+++ b/drivers/staging/hv/connection.c
@@ -220,11 +220,11 @@ static void process_chn_event(u32 relid)
channel = relid2channel(relid);
spin_lock_irqsave(&channel->inbound_lock, flags);
- if (channel && (channel->onchannel_callback != NULL)) {
+ if (channel && (channel->onchannel_callback != NULL))
channel->onchannel_callback(channel->channel_callback_context);
- } else {
+ else
pr_err("channel not found for relid - %u\n", relid);
- }
+
spin_unlock_irqrestore(&channel->inbound_lock, flags);
}
@@ -246,16 +246,17 @@ void vmbus_on_event(unsigned long data)
if (!recv_int_page[dword])
continue;
for (bit = 0; bit < 32; bit++) {
- if (sync_test_and_clear_bit(bit, (unsigned long *)&recv_int_page[dword])) {
+ if (sync_test_and_clear_bit(bit,
+ (unsigned long *)&recv_int_page[dword])) {
relid = (dword << 5) + bit;
- if (relid == 0) {
+ if (relid == 0)
/*
* Special case - vmbus
* channel protocol msg
*/
continue;
- }
+
process_chn_event(relid);
}
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 43/46] Staging: hv: vmbus: Fix a checkpatch warning in ring_buffer.c
From: K. Y. Srinivasan @ 2011-08-27 18:31 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang
In-Reply-To: <1314469905-7058-1-git-send-email-kys@microsoft.com>
Fix a checkpatch warning in ring_buffer.c
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/ring_buffer.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/hv/ring_buffer.c b/drivers/staging/hv/ring_buffer.c
index 9212699..70e2e66 100644
--- a/drivers/staging/hv/ring_buffer.c
+++ b/drivers/staging/hv/ring_buffer.c
@@ -34,7 +34,8 @@
/* Amount of space to write to */
-#define BYTES_AVAIL_TO_WRITE(r, w, z) ((w) >= (r)) ? ((z) - ((w) - (r))) : ((r) - (w))
+#define BYTES_AVAIL_TO_WRITE(r, w, z) \
+ ((w) >= (r)) ? ((z) - ((w) - (r))) : ((r) - (w))
/*
--
1.7.4.1
^ permalink raw reply related
* [PATCH 42/46] Staging: hv: vmbus: Get rid of an unnecessary check in vmbus_connect()
From: K. Y. Srinivasan @ 2011-08-27 18:31 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization
Cc: K. Y. Srinivasan, Haiyang Zhang
In-Reply-To: <1314469905-7058-1-git-send-email-kys@microsoft.com>
Get rid of an unnecessary check in vmbus_connect().
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/connection.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/hv/connection.c b/drivers/staging/hv/connection.c
index 6aab802..ca92ca3 100644
--- a/drivers/staging/hv/connection.c
+++ b/drivers/staging/hv/connection.c
@@ -50,10 +50,6 @@ int vmbus_connect(void)
struct vmbus_channel_initiate_contact *msg;
unsigned long flags;
- /* Make sure we are not connecting or connected */
- if (vmbus_connection.conn_state != DISCONNECTED)
- return -EISCONN;
-
/* Initialize the vmbus connection */
vmbus_connection.conn_state = CONNECTING;
vmbus_connection.work_queue = create_workqueue("hv_vmbus_con");
--
1.7.4.1
^ permalink raw reply related
* [PATCH 41/46] Staging: hv: vmbus: Fix a bug in error handling in vmbus_bus_init()
From: K. Y. Srinivasan @ 2011-08-27 18:31 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang
In-Reply-To: <1314469905-7058-1-git-send-email-kys@microsoft.com>
Fix a bug in error handling in vmbus_bus_init().
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/vmbus_drv.c | 21 ++++++++++-----------
1 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 02edb11..4d1b123 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -492,7 +492,7 @@ static int vmbus_bus_init(int irq)
ret = bus_register(&hv_bus);
if (ret)
- return ret;
+ goto err1;
ret = request_irq(irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
driver_name, hv_acpi_dev);
@@ -500,10 +500,7 @@ static int vmbus_bus_init(int irq)
if (ret != 0) {
pr_err("Unable to request IRQ %d\n",
irq);
-
- bus_unregister(&hv_bus);
-
- return ret;
+ goto err2;
}
vector = IRQ0_VECTOR + irq;
@@ -514,16 +511,18 @@ static int vmbus_bus_init(int irq)
*/
on_each_cpu(hv_synic_init, (void *)&vector, 1);
ret = vmbus_connect();
- if (ret) {
- free_irq(irq, hv_acpi_dev);
- bus_unregister(&hv_bus);
- return ret;
- }
-
+ if (ret)
+ goto err3;
vmbus_request_offers();
return 0;
+
+err3: free_irq(irq, hv_acpi_dev);
+err2: bus_unregister(&hv_bus);
+err1: hv_cleanup();
+
+ return ret;
}
/**
--
1.7.4.1
^ permalink raw reply related
* [PATCH 40/46] Staging: hv: vmbus: Get rid of some dated/redundant comments
From: K. Y. Srinivasan @ 2011-08-27 18:31 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang
In-Reply-To: <1314469905-7058-1-git-send-email-kys@microsoft.com>
Get rid of some dated/redundant comments in vmbus_drv.c
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/vmbus_drv.c | 11 +----------
1 files changed, 1 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index d50fa89..02edb11 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -391,9 +391,6 @@ static void vmbus_onmessage_work(struct work_struct *work)
kfree(ctx);
}
-/*
- * vmbus_on_msg_dpc - DPC routine to handle messages from the hypervisior
- */
static void vmbus_on_msg_dpc(unsigned long data)
{
int cpu = smp_processor_id();
@@ -490,16 +487,13 @@ static int vmbus_bus_init(int irq)
return ret;
}
- /* Initialize the bus context */
tasklet_init(&msg_dpc, vmbus_on_msg_dpc, 0);
tasklet_init(&event_dpc, vmbus_on_event, 0);
- /* Now, register the bus with LDM */
ret = bus_register(&hv_bus);
if (ret)
return ret;
- /* Get the interrupt resource */
ret = request_irq(irq, vmbus_isr, IRQF_SAMPLE_RANDOM,
driver_name, hv_acpi_dev);
@@ -588,7 +582,6 @@ struct hv_device *vmbus_child_device_create(uuid_le *type,
{
struct hv_device *child_device_obj;
- /* Allocate the new child device */
child_device_obj = kzalloc(sizeof(struct hv_device), GFP_KERNEL);
if (!child_device_obj) {
pr_err("Unable to allocate device object for child device\n");
@@ -613,12 +606,10 @@ int vmbus_child_device_register(struct hv_device *child_device_obj)
static atomic_t device_num = ATOMIC_INIT(0);
- /* Set the device name. Otherwise, device_register() will fail. */
dev_set_name(&child_device_obj->device, "vmbus_0_%d",
atomic_inc_return(&device_num));
- /* The new device belongs to this bus */
- child_device_obj->device.bus = &hv_bus; /* device->dev.bus; */
+ child_device_obj->device.bus = &hv_bus;
child_device_obj->device.parent = &hv_acpi_dev->dev;
child_device_obj->device.release = vmbus_device_release;
--
1.7.4.1
^ permalink raw reply related
* [PATCH 39/46] Staging: hv: vmbus: Fixup indentation in vmbus_acpi_add()
From: K. Y. Srinivasan @ 2011-08-27 18:31 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang
In-Reply-To: <1314469905-7058-1-git-send-email-kys@microsoft.com>
Fixup indentation in vmbus_acpi_add().
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/vmbus_drv.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 4e07d4f..d50fa89 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -678,9 +678,8 @@ static int vmbus_acpi_add(struct acpi_device *device)
hv_acpi_dev = device;
- result =
- acpi_walk_resources(device->handle, METHOD_NAME__CRS,
- vmbus_walk_resources, &irq);
+ result = acpi_walk_resources(device->handle, METHOD_NAME__CRS,
+ vmbus_walk_resources, &irq);
if (ACPI_FAILURE(result)) {
complete(&probe_event);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 38/46] Staging: hv: vmbus: Do not enable auto eoi
From: K. Y. Srinivasan @ 2011-08-27 18:31 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang
In-Reply-To: <1314469905-7058-1-git-send-email-kys@microsoft.com>
Linux interrupt handling code generates the eoi; don't enable auto eoi.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/hv.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/hv/hv.c b/drivers/staging/hv/hv.c
index 736794e..06f1e15 100644
--- a/drivers/staging/hv/hv.c
+++ b/drivers/staging/hv/hv.c
@@ -369,7 +369,7 @@ void hv_synic_init(void *irqarg)
shared_sint.as_uint64 = 0;
shared_sint.vector = irq_vector; /* HV_SHARED_SINT_IDT_VECTOR + 0x20; */
shared_sint.masked = false;
- shared_sint.auto_eoi = true;
+ shared_sint.auto_eoi = false;
wrmsrl(HV_X64_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 37/46] Staging: hv: vmbus: Check for events before messages
From: K. Y. Srinivasan @ 2011-08-27 18:31 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang
In-Reply-To: <1314469905-7058-1-git-send-email-kys@microsoft.com>
Conform to Windows specification by checking for events before messages.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/vmbus_drv.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 5dcc8c3..4e07d4f 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -445,15 +445,6 @@ static irqreturn_t vmbus_isr(int irq, void *dev_id)
union hv_synic_event_flags *event;
bool handled = false;
- page_addr = hv_context.synic_message_page[cpu];
- msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
-
- /* Check if there are actual msgs to be process */
- if (msg->header.message_type != HVMSG_NONE) {
- handled = true;
- tasklet_schedule(&msg_dpc);
- }
-
page_addr = hv_context.synic_event_page[cpu];
event = (union hv_synic_event_flags *)page_addr + VMBUS_MESSAGE_SINT;
@@ -463,6 +454,15 @@ static irqreturn_t vmbus_isr(int irq, void *dev_id)
tasklet_schedule(&event_dpc);
}
+ page_addr = hv_context.synic_message_page[cpu];
+ msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
+
+ /* Check if there are actual msgs to be processed */
+ if (msg->header.message_type != HVMSG_NONE) {
+ handled = true;
+ tasklet_schedule(&msg_dpc);
+ }
+
if (handled)
return IRQ_HANDLED;
else
--
1.7.4.1
^ permalink raw reply related
* [PATCH 36/46] Staging: hv: vmbus: Get rid of vmbus_on_isr() by inlining the code
From: K. Y. Srinivasan @ 2011-08-27 18:31 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang
In-Reply-To: <1314469905-7058-1-git-send-email-kys@microsoft.com>
Get rid of vmbus_on_isr() by inlining the code.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/vmbus_drv.c | 41 +++++++++++----------------------------
1 files changed, 12 insertions(+), 29 deletions(-)
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index 51002c0..5dcc8c3 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -437,53 +437,36 @@ static void vmbus_on_msg_dpc(unsigned long data)
}
}
-/*
- * vmbus_on_isr - ISR routine
- */
-static int vmbus_on_isr(void)
+static irqreturn_t vmbus_isr(int irq, void *dev_id)
{
- int ret = 0;
int cpu = smp_processor_id();
void *page_addr;
struct hv_message *msg;
union hv_synic_event_flags *event;
+ bool handled = false;
page_addr = hv_context.synic_message_page[cpu];
msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
/* Check if there are actual msgs to be process */
- if (msg->header.message_type != HVMSG_NONE)
- ret |= 0x1;
+ if (msg->header.message_type != HVMSG_NONE) {
+ handled = true;
+ tasklet_schedule(&msg_dpc);
+ }
page_addr = hv_context.synic_event_page[cpu];
event = (union hv_synic_event_flags *)page_addr + VMBUS_MESSAGE_SINT;
/* Since we are a child, we only need to check bit 0 */
- if (sync_test_and_clear_bit(0, (unsigned long *) &event->flags32[0]))
- ret |= 0x2;
-
- return ret;
-}
-
-
-static irqreturn_t vmbus_isr(int irq, void *dev_id)
-{
- int ret;
-
- ret = vmbus_on_isr();
-
- /* Schedules a dpc if necessary */
- if (ret > 0) {
- if (test_bit(0, (unsigned long *)&ret))
- tasklet_schedule(&msg_dpc);
-
- if (test_bit(1, (unsigned long *)&ret))
- tasklet_schedule(&event_dpc);
+ if (sync_test_and_clear_bit(0, (unsigned long *) &event->flags32[0])) {
+ handled = true;
+ tasklet_schedule(&event_dpc);
+ }
+ if (handled)
return IRQ_HANDLED;
- } else {
+ else
return IRQ_NONE;
- }
}
/*
--
1.7.4.1
^ permalink raw reply related
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