* RE: [PATCH 12/59] Staging: hv: vmbus: Cleanup vmbus_uevent() code
From: KY Srinivasan @ 2011-08-25 22:14 UTC (permalink / raw)
To: Greg KH
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang
In-Reply-To: <20110825205930.GA10883@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Thursday, August 25, 2011 5:00 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang
> Subject: Re: [PATCH 12/59] Staging: hv: vmbus: Cleanup vmbus_uevent() code
>
> On Thu, Aug 25, 2011 at 09:48:38AM -0700, K. Y. Srinivasan wrote:
> > Now generate appropriate uevent based on the modalias string. As part of this,
> > cleanup the existing uevent code.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> > drivers/staging/hv/vmbus_drv.c | 60 ++++++++--------------------------------
> > 1 files changed, 12 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
> > index b651968..a6e7dc5 100644
> > --- a/drivers/staging/hv/vmbus_drv.c
> > +++ b/drivers/staging/hv/vmbus_drv.c
> > @@ -237,58 +237,22 @@ static struct device_attribute vmbus_device_attrs[] =
> {
> > * This routine is invoked when a device is added or removed on the vmbus to
> > * generate a uevent to udev in the userspace. The udev will then look at its
> > * rule and the uevent generated here to load the appropriate driver
> > + *
> > + * The alias string will be of the form vmbus:guid where guid is the string
> > + * representation of the device guid (each byte of the guid will be
> > + * represented with two hex characters.
> > */
> > static int vmbus_uevent(struct device *device, struct kobj_uevent_env *env)
> > {
> > struct hv_device *dev = device_to_hv_device(device);
> > - int ret;
> > -
> > - ret = add_uevent_var(env, "VMBUS_DEVICE_CLASS_GUID={"
> > - "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
> > - "%02x%02x%02x%02x%02x%02x%02x%02x}",
> > - dev->dev_type.b[3],
> > - dev->dev_type.b[2],
> > - dev->dev_type.b[1],
> > - dev->dev_type.b[0],
> > - dev->dev_type.b[5],
> > - dev->dev_type.b[4],
> > - dev->dev_type.b[7],
> > - dev->dev_type.b[6],
> > - dev->dev_type.b[8],
> > - dev->dev_type.b[9],
> > - dev->dev_type.b[10],
> > - dev->dev_type.b[11],
> > - dev->dev_type.b[12],
> > - dev->dev_type.b[13],
> > - dev->dev_type.b[14],
> > - dev->dev_type.b[15]);
> > -
> > - if (ret)
> > - return ret;
> > + int i, ret;
> > + char alias_name[((sizeof(struct hv_vmbus_device_id) + 1)) * 2];
> >
> > - ret = add_uevent_var(env, "VMBUS_DEVICE_DEVICE_GUID={"
> > - "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
> > - "%02x%02x%02x%02x%02x%02x%02x%02x}",
> > - dev->dev_instance.b[3],
> > - dev->dev_instance.b[2],
> > - dev->dev_instance.b[1],
> > - dev->dev_instance.b[0],
> > - dev->dev_instance.b[5],
> > - dev->dev_instance.b[4],
> > - dev->dev_instance.b[7],
> > - dev->dev_instance.b[6],
> > - dev->dev_instance.b[8],
> > - dev->dev_instance.b[9],
> > - dev->dev_instance.b[10],
> > - dev->dev_instance.b[11],
> > - dev->dev_instance.b[12],
> > - dev->dev_instance.b[13],
> > - dev->dev_instance.b[14],
> > - dev->dev_instance.b[15]);
> > - if (ret)
> > - return ret;
> > + for (i = 0; i < (sizeof(struct hv_vmbus_device_id) * 2); i += 2)
> > + sprintf(&alias_name[i], "%02x", dev->dev_type.b[i/2]);
>
> I have to edit this to get it to work properly with the fact that I
> added the driver_data field to hv_vmbus_device_id.
>
> Arguably, one could say that this patch was always broken as you were
> assuming the size of an individual field was the same size as the whole
> structure, which I don't think is always the case, or at least it's not
> a safe thing to assume :)
Perhaps I could have sized it based on guid size since that is what is going
to be the alias.
Regards,
K. Y
^ permalink raw reply
* RE: [PATCH 12/59] Staging: hv: vmbus: Cleanup vmbus_uevent() code
From: KY Srinivasan @ 2011-08-25 22:20 UTC (permalink / raw)
To: Greg KH
Cc: devel@linuxdriverproject.org, Haiyang Zhang, gregkh@suse.de,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org
In-Reply-To: <20110825211514.GD26773@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Thursday, August 25, 2011 5:15 PM
> To: KY Srinivasan
> Cc: devel@linuxdriverproject.org; Haiyang Zhang; gregkh@suse.de; linux-
> kernel@vger.kernel.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 12/59] Staging: hv: vmbus: Cleanup vmbus_uevent() code
>
> On Thu, Aug 25, 2011 at 01:59:30PM -0700, Greg KH wrote:
> > On Thu, Aug 25, 2011 at 09:48:38AM -0700, K. Y. Srinivasan wrote:
> > > Now generate appropriate uevent based on the modalias string. As part of
> this,
> > > cleanup the existing uevent code.
> > >
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > ---
> > > drivers/staging/hv/vmbus_drv.c | 60 ++++++++--------------------------------
> > > 1 files changed, 12 insertions(+), 48 deletions(-)
> > >
> > > diff --git a/drivers/staging/hv/vmbus_drv.c
> b/drivers/staging/hv/vmbus_drv.c
> > > index b651968..a6e7dc5 100644
> > > --- a/drivers/staging/hv/vmbus_drv.c
> > > +++ b/drivers/staging/hv/vmbus_drv.c
> > > @@ -237,58 +237,22 @@ static struct device_attribute vmbus_device_attrs[]
> = {
> > > * This routine is invoked when a device is added or removed on the vmbus
> to
> > > * generate a uevent to udev in the userspace. The udev will then look at its
> > > * rule and the uevent generated here to load the appropriate driver
> > > + *
> > > + * The alias string will be of the form vmbus:guid where guid is the string
> > > + * representation of the device guid (each byte of the guid will be
> > > + * represented with two hex characters.
> > > */
> > > static int vmbus_uevent(struct device *device, struct kobj_uevent_env
> *env)
> > > {
> > > struct hv_device *dev = device_to_hv_device(device);
> > > - int ret;
> > > -
> > > - ret = add_uevent_var(env, "VMBUS_DEVICE_CLASS_GUID={"
> > > - "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
> > > - "%02x%02x%02x%02x%02x%02x%02x%02x}",
> > > - dev->dev_type.b[3],
> > > - dev->dev_type.b[2],
> > > - dev->dev_type.b[1],
> > > - dev->dev_type.b[0],
> > > - dev->dev_type.b[5],
> > > - dev->dev_type.b[4],
> > > - dev->dev_type.b[7],
> > > - dev->dev_type.b[6],
> > > - dev->dev_type.b[8],
> > > - dev->dev_type.b[9],
> > > - dev->dev_type.b[10],
> > > - dev->dev_type.b[11],
> > > - dev->dev_type.b[12],
> > > - dev->dev_type.b[13],
> > > - dev->dev_type.b[14],
> > > - dev->dev_type.b[15]);
> > > -
> > > - if (ret)
> > > - return ret;
> > > + int i, ret;
> > > + char alias_name[((sizeof(struct hv_vmbus_device_id) + 1)) * 2];
> > >
> > > - ret = add_uevent_var(env, "VMBUS_DEVICE_DEVICE_GUID={"
> > > - "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
> > > - "%02x%02x%02x%02x%02x%02x%02x%02x}",
> > > - dev->dev_instance.b[3],
> > > - dev->dev_instance.b[2],
> > > - dev->dev_instance.b[1],
> > > - dev->dev_instance.b[0],
> > > - dev->dev_instance.b[5],
> > > - dev->dev_instance.b[4],
> > > - dev->dev_instance.b[7],
> > > - dev->dev_instance.b[6],
> > > - dev->dev_instance.b[8],
> > > - dev->dev_instance.b[9],
> > > - dev->dev_instance.b[10],
> > > - dev->dev_instance.b[11],
> > > - dev->dev_instance.b[12],
> > > - dev->dev_instance.b[13],
> > > - dev->dev_instance.b[14],
> > > - dev->dev_instance.b[15]);
> > > - if (ret)
> > > - return ret;
> > > + for (i = 0; i < (sizeof(struct hv_vmbus_device_id) * 2); i += 2)
> > > + sprintf(&alias_name[i], "%02x", dev->dev_type.b[i/2]);
> >
> > I have to edit this to get it to work properly with the fact that I
> > added the driver_data field to hv_vmbus_device_id.
>
> You should have a copy of the patch I applied in your inbox now, can you
> verify I didn't mess it up?
Greg,
I don't think I got this mail. Could you resend the mail.
Regards,
K. Y
^ permalink raw reply
* Re: [PATCH 12/59] Staging: hv: vmbus: Cleanup vmbus_uevent() code
From: Greg KH @ 2011-08-25 22:26 UTC (permalink / raw)
To: KY Srinivasan
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang
In-Reply-To: <6E21E5352C11B742B20C142EB499E048081B356B@TK5EX14MBXC126.redmond.corp.microsoft.com>
On Thu, Aug 25, 2011 at 10:14:05PM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Thursday, August 25, 2011 5:00 PM
> > To: KY Srinivasan
> > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang
> > Subject: Re: [PATCH 12/59] Staging: hv: vmbus: Cleanup vmbus_uevent() code
> >
> > On Thu, Aug 25, 2011 at 09:48:38AM -0700, K. Y. Srinivasan wrote:
> > > Now generate appropriate uevent based on the modalias string. As part of this,
> > > cleanup the existing uevent code.
> > >
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > ---
> > > drivers/staging/hv/vmbus_drv.c | 60 ++++++++--------------------------------
> > > 1 files changed, 12 insertions(+), 48 deletions(-)
> > >
> > > diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
> > > index b651968..a6e7dc5 100644
> > > --- a/drivers/staging/hv/vmbus_drv.c
> > > +++ b/drivers/staging/hv/vmbus_drv.c
> > > @@ -237,58 +237,22 @@ static struct device_attribute vmbus_device_attrs[] =
> > {
> > > * This routine is invoked when a device is added or removed on the vmbus to
> > > * generate a uevent to udev in the userspace. The udev will then look at its
> > > * rule and the uevent generated here to load the appropriate driver
> > > + *
> > > + * The alias string will be of the form vmbus:guid where guid is the string
> > > + * representation of the device guid (each byte of the guid will be
> > > + * represented with two hex characters.
> > > */
> > > static int vmbus_uevent(struct device *device, struct kobj_uevent_env *env)
> > > {
> > > struct hv_device *dev = device_to_hv_device(device);
> > > - int ret;
> > > -
> > > - ret = add_uevent_var(env, "VMBUS_DEVICE_CLASS_GUID={"
> > > - "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
> > > - "%02x%02x%02x%02x%02x%02x%02x%02x}",
> > > - dev->dev_type.b[3],
> > > - dev->dev_type.b[2],
> > > - dev->dev_type.b[1],
> > > - dev->dev_type.b[0],
> > > - dev->dev_type.b[5],
> > > - dev->dev_type.b[4],
> > > - dev->dev_type.b[7],
> > > - dev->dev_type.b[6],
> > > - dev->dev_type.b[8],
> > > - dev->dev_type.b[9],
> > > - dev->dev_type.b[10],
> > > - dev->dev_type.b[11],
> > > - dev->dev_type.b[12],
> > > - dev->dev_type.b[13],
> > > - dev->dev_type.b[14],
> > > - dev->dev_type.b[15]);
> > > -
> > > - if (ret)
> > > - return ret;
> > > + int i, ret;
> > > + char alias_name[((sizeof(struct hv_vmbus_device_id) + 1)) * 2];
> > >
> > > - ret = add_uevent_var(env, "VMBUS_DEVICE_DEVICE_GUID={"
> > > - "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
> > > - "%02x%02x%02x%02x%02x%02x%02x%02x}",
> > > - dev->dev_instance.b[3],
> > > - dev->dev_instance.b[2],
> > > - dev->dev_instance.b[1],
> > > - dev->dev_instance.b[0],
> > > - dev->dev_instance.b[5],
> > > - dev->dev_instance.b[4],
> > > - dev->dev_instance.b[7],
> > > - dev->dev_instance.b[6],
> > > - dev->dev_instance.b[8],
> > > - dev->dev_instance.b[9],
> > > - dev->dev_instance.b[10],
> > > - dev->dev_instance.b[11],
> > > - dev->dev_instance.b[12],
> > > - dev->dev_instance.b[13],
> > > - dev->dev_instance.b[14],
> > > - dev->dev_instance.b[15]);
> > > - if (ret)
> > > - return ret;
> > > + for (i = 0; i < (sizeof(struct hv_vmbus_device_id) * 2); i += 2)
> > > + sprintf(&alias_name[i], "%02x", dev->dev_type.b[i/2]);
> >
> > I have to edit this to get it to work properly with the fact that I
> > added the driver_data field to hv_vmbus_device_id.
> >
> > Arguably, one could say that this patch was always broken as you were
> > assuming the size of an individual field was the same size as the whole
> > structure, which I don't think is always the case, or at least it's not
> > a safe thing to assume :)
>
> Perhaps I could have sized it based on guid size since that is what is going
> to be the alias.
That's what I ended up doing :)
greg k-h
^ permalink raw reply
* RE: [PATCH 22/59] Staging: hv: vmbus: Get rid of the unused name field in struct hv_driver
From: KY Srinivasan @ 2011-08-25 22:27 UTC (permalink / raw)
To: Greg KH
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang
In-Reply-To: <20110825212820.GA6770@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Thursday, August 25, 2011 5:28 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang
> Subject: Re: [PATCH 22/59] Staging: hv: vmbus: Get rid of the unused name field
> in struct hv_driver
>
> On Thu, Aug 25, 2011 at 02:24:28PM -0700, Greg KH wrote:
> > On Thu, Aug 25, 2011 at 09:48:48AM -0700, K. Y. Srinivasan wrote:
> > > Get rid of the unused "name" field in struct hv_driver.
> > >
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > ---
> > > drivers/staging/hv/hyperv.h | 2 --
> > > 1 files changed, 0 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/hv/hyperv.h b/drivers/staging/hv/hyperv.h
> > > index b8199f4..60ead66 100644
> > > --- a/drivers/staging/hv/hyperv.h
> > > +++ b/drivers/staging/hv/hyperv.h
> > > @@ -802,8 +802,6 @@ struct hv_device_info {
> > >
> > > /* Base driver object */
> > > struct hv_driver {
> > > - const char *name;
> >
> > Wait, why is this unused? What are you going to use as your name for
> > the driver in sysfs then? The module name?
> >
> > As much as I love seeing things deleted, I really think you need this
> > field.
> >
> > Ah, yeah, I see why you think it's unneeded, crud like this in the
> > drivers:
> >
> > drv->driver.name = driver_name;
> >
> > No vmbus driver should ever have to touch the base struct driver on it's
> > own at all. Your vmbus core should properly handle telling the driver
> > core what the name of the driver is.
> >
> > As an example, see the __pci_register_driver() function, the first thing
> > that code does is set the name based on the name of the larger
> > pci_driver structure passed to it.
> >
> > Man, if you want something done right, you have to do it yourself, let
> > me go make these changes so you don't have to do any new work at this
> > point in time, hopefully your other patches will apply...
>
> What, vmbus_child_driver_register() takes a struct driver *?
>
> No wonder things are so messed up here, and why you got confused. Let
> me pound on this for a bit to see if I can get it cleaned up to be more
> "sane"...
Greg,
This was existing code and I realized it did not conform to the Linux
Driver Model. I fixed it in the patch set I had sent earlier (110/117).
Yesterday I only got up to 74 of the 117 patches that you commented on.
I was planning to send the rest today. If it is ok with you, please don't fix it here
As it would break everything else. This is existing code that I am fixing in the patch set
I will send you shortly.
Regards,
K. Y
>
> greg k-h
^ permalink raw reply
* Re: [PATCH 12/59] Staging: hv: vmbus: Cleanup vmbus_uevent() code
From: Greg KH @ 2011-08-25 22:28 UTC (permalink / raw)
To: KY Srinivasan
Cc: devel@linuxdriverproject.org, Haiyang Zhang, gregkh@suse.de,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org
In-Reply-To: <6E21E5352C11B742B20C142EB499E048081B359F@TK5EX14MBXC126.redmond.corp.microsoft.com>
On Thu, Aug 25, 2011 at 10:20:32PM +0000, KY Srinivasan wrote:
> > > I have to edit this to get it to work properly with the fact that I
> > > added the driver_data field to hv_vmbus_device_id.
> >
> > You should have a copy of the patch I applied in your inbox now, can you
> > verify I didn't mess it up?
>
> Greg,
>
> I don't think I got this mail. Could you resend the mail.
Sure, here it is:
From: "K. Y. Srinivasan" <kys@microsoft.com>
Date: Thu, 25 Aug 2011 09:48:38 -0700
Subject: [PATCH] Staging: hv: vmbus: Cleanup vmbus_uevent() code
Status: RO
Content-Length: 2871
Lines: 82
Now generate appropriate uevent based on the modalias string. As part of this,
cleanup the existing uevent code.
[gregkh - fixed code to handle driver_data portion of struct
hv_vmbus_device_id]
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
index b651968..afb16704 100644
--- a/drivers/staging/hv/vmbus_drv.c
+++ b/drivers/staging/hv/vmbus_drv.c
@@ -237,58 +237,22 @@ static struct device_attribute vmbus_device_attrs[] = {
* This routine is invoked when a device is added or removed on the vmbus to
* generate a uevent to udev in the userspace. The udev will then look at its
* rule and the uevent generated here to load the appropriate driver
+ *
+ * The alias string will be of the form vmbus:guid where guid is the string
+ * representation of the device guid (each byte of the guid will be
+ * represented with two hex characters.
*/
static int vmbus_uevent(struct device *device, struct kobj_uevent_env *env)
{
struct hv_device *dev = device_to_hv_device(device);
- int ret;
-
- ret = add_uevent_var(env, "VMBUS_DEVICE_CLASS_GUID={"
- "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
- "%02x%02x%02x%02x%02x%02x%02x%02x}",
- dev->dev_type.b[3],
- dev->dev_type.b[2],
- dev->dev_type.b[1],
- dev->dev_type.b[0],
- dev->dev_type.b[5],
- dev->dev_type.b[4],
- dev->dev_type.b[7],
- dev->dev_type.b[6],
- dev->dev_type.b[8],
- dev->dev_type.b[9],
- dev->dev_type.b[10],
- dev->dev_type.b[11],
- dev->dev_type.b[12],
- dev->dev_type.b[13],
- dev->dev_type.b[14],
- dev->dev_type.b[15]);
-
- if (ret)
- return ret;
+ int i, ret;
+ char alias_name[((sizeof((struct hv_vmbus_device_id *)0)->guid) + 1) * 2];
- ret = add_uevent_var(env, "VMBUS_DEVICE_DEVICE_GUID={"
- "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
- "%02x%02x%02x%02x%02x%02x%02x%02x}",
- dev->dev_instance.b[3],
- dev->dev_instance.b[2],
- dev->dev_instance.b[1],
- dev->dev_instance.b[0],
- dev->dev_instance.b[5],
- dev->dev_instance.b[4],
- dev->dev_instance.b[7],
- dev->dev_instance.b[6],
- dev->dev_instance.b[8],
- dev->dev_instance.b[9],
- dev->dev_instance.b[10],
- dev->dev_instance.b[11],
- dev->dev_instance.b[12],
- dev->dev_instance.b[13],
- dev->dev_instance.b[14],
- dev->dev_instance.b[15]);
- if (ret)
- return ret;
+ for (i = 0; i < ((sizeof((struct hv_vmbus_device_id *)0)->guid) * 2); i += 2)
+ sprintf(&alias_name[i], "%02x", dev->dev_type.b[i/2]);
- return 0;
+ ret = add_uevent_var(env, "MODALIAS=vmbus:%s", alias_name);
+ return ret;
}
^ permalink raw reply related
* Re: [PATCH 22/59] Staging: hv: vmbus: Get rid of the unused name field in struct hv_driver
From: Greg KH @ 2011-08-25 22:35 UTC (permalink / raw)
To: KY Srinivasan
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang
In-Reply-To: <6E21E5352C11B742B20C142EB499E048081B360E@TK5EX14MBXC126.redmond.corp.microsoft.com>
On Thu, Aug 25, 2011 at 10:27:40PM +0000, KY Srinivasan wrote:
> This was existing code and I realized it did not conform to the Linux
> Driver Model. I fixed it in the patch set I had sent earlier (110/117).
> Yesterday I only got up to 74 of the 117 patches that you commented on.
> I was planning to send the rest today. If it is ok with you, please don't fix it here
> As it would break everything else. This is existing code that I am fixing in the patch set
> I will send you shortly.
Oops, as you see, I applied it :)
And I applied the rest of your series, so you might want to re-test and
resync your next series of patches based on what I have in the
staging-next tree now.
thanks,
greg k-h
^ permalink raw reply
* RE: [PATCH 12/59] Staging: hv: vmbus: Cleanup vmbus_uevent() code
From: KY Srinivasan @ 2011-08-25 22:46 UTC (permalink / raw)
To: Greg KH
Cc: devel@linuxdriverproject.org, Haiyang Zhang, gregkh@suse.de,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org
In-Reply-To: <20110825222819.GA13787@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Thursday, August 25, 2011 6:28 PM
> To: KY Srinivasan
> Cc: devel@linuxdriverproject.org; Haiyang Zhang; gregkh@suse.de; linux-
> kernel@vger.kernel.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 12/59] Staging: hv: vmbus: Cleanup vmbus_uevent() code
>
> On Thu, Aug 25, 2011 at 10:20:32PM +0000, KY Srinivasan wrote:
> > > > I have to edit this to get it to work properly with the fact that I
> > > > added the driver_data field to hv_vmbus_device_id.
> > >
> > > You should have a copy of the patch I applied in your inbox now, can you
> > > verify I didn't mess it up?
> >
> > Greg,
> >
> > I don't think I got this mail. Could you resend the mail.
>
> Sure, here it is:
>
>
> From: "K. Y. Srinivasan" <kys@microsoft.com>
> Date: Thu, 25 Aug 2011 09:48:38 -0700
> Subject: [PATCH] Staging: hv: vmbus: Cleanup vmbus_uevent() code
> Status: RO
> Content-Length: 2871
> Lines: 82
>
> Now generate appropriate uevent based on the modalias string. As part of this,
> cleanup the existing uevent code.
>
> [gregkh - fixed code to handle driver_data portion of struct
> hv_vmbus_device_id]
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
>
> diff --git a/drivers/staging/hv/vmbus_drv.c b/drivers/staging/hv/vmbus_drv.c
> index b651968..afb16704 100644
> --- a/drivers/staging/hv/vmbus_drv.c
> +++ b/drivers/staging/hv/vmbus_drv.c
> @@ -237,58 +237,22 @@ static struct device_attribute vmbus_device_attrs[] = {
> * This routine is invoked when a device is added or removed on the vmbus to
> * generate a uevent to udev in the userspace. The udev will then look at its
> * rule and the uevent generated here to load the appropriate driver
> + *
> + * The alias string will be of the form vmbus:guid where guid is the string
> + * representation of the device guid (each byte of the guid will be
> + * represented with two hex characters.
> */
> static int vmbus_uevent(struct device *device, struct kobj_uevent_env *env)
> {
> struct hv_device *dev = device_to_hv_device(device);
> - int ret;
> -
> - ret = add_uevent_var(env, "VMBUS_DEVICE_CLASS_GUID={"
> - "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
> - "%02x%02x%02x%02x%02x%02x%02x%02x}",
> - dev->dev_type.b[3],
> - dev->dev_type.b[2],
> - dev->dev_type.b[1],
> - dev->dev_type.b[0],
> - dev->dev_type.b[5],
> - dev->dev_type.b[4],
> - dev->dev_type.b[7],
> - dev->dev_type.b[6],
> - dev->dev_type.b[8],
> - dev->dev_type.b[9],
> - dev->dev_type.b[10],
> - dev->dev_type.b[11],
> - dev->dev_type.b[12],
> - dev->dev_type.b[13],
> - dev->dev_type.b[14],
> - dev->dev_type.b[15]);
> -
> - if (ret)
> - return ret;
> + int i, ret;
> + char alias_name[((sizeof((struct hv_vmbus_device_id *)0)->guid) + 1) *
> 2];
>
> - ret = add_uevent_var(env, "VMBUS_DEVICE_DEVICE_GUID={"
> - "%02x%02x%02x%02x-%02x%02x-%02x%02x-"
> - "%02x%02x%02x%02x%02x%02x%02x%02x}",
> - dev->dev_instance.b[3],
> - dev->dev_instance.b[2],
> - dev->dev_instance.b[1],
> - dev->dev_instance.b[0],
> - dev->dev_instance.b[5],
> - dev->dev_instance.b[4],
> - dev->dev_instance.b[7],
> - dev->dev_instance.b[6],
> - dev->dev_instance.b[8],
> - dev->dev_instance.b[9],
> - dev->dev_instance.b[10],
> - dev->dev_instance.b[11],
> - dev->dev_instance.b[12],
> - dev->dev_instance.b[13],
> - dev->dev_instance.b[14],
> - dev->dev_instance.b[15]);
> - if (ret)
> - return ret;
> + for (i = 0; i < ((sizeof((struct hv_vmbus_device_id *)0)->guid) * 2); i += 2)
> + sprintf(&alias_name[i], "%02x", dev->dev_type.b[i/2]);
>
> - return 0;
> + ret = add_uevent_var(env, "MODALIAS=vmbus:%s", alias_name);
> + return ret;
> }
>
>
Looks good.
Thank you.
K. Y
^ permalink raw reply
* Re: [PATCH] virtio.h: correct comment for struct virtio_driver
From: Rusty Russell @ 2011-08-25 23:46 UTC (permalink / raw)
To: Wang Sheng-Hui, mst, virtualization, linux-kernel
In-Reply-To: <4E564845.3020209@gmail.com>
On Thu, 25 Aug 2011 21:04:05 +0800, Wang Sheng-Hui <shhuiw@gmail.com> wrote:
> The patch is against 3.0.
>
> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
Thanks, applied!
Cheers,
Rusty.
^ permalink raw reply
* [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size
From: Wang Sheng-Hui @ 2011-08-27 1:07 UTC (permalink / raw)
To: rusty, mst, virtualization, linux-kernel
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 */
--
1.7.1
crossover@crossover-plinux:~/dev/patchwork$
^ permalink raw reply related
* Re: [PATCH] virtio: fix size computation according to the definition of struct vring_used in vring_size
From: Wanlong Gao @ 2011-08-27 2:49 UTC (permalink / raw)
To: Wang Sheng-Hui; +Cc: virtualization, linux-kernel, mst
In-Reply-To: <4E584365.1010806@gmail.com>
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
^ 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-27 9:34 UTC (permalink / raw)
To: wanlong.gao; +Cc: virtualization, linux-kernel, mst
In-Reply-To: <1314413399.1913.1.camel@Allen>
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?
_______________________________________________
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-27 9:52 UTC (permalink / raw)
To: wanlong.gao; +Cc: virtualization, linux-kernel, mst
In-Reply-To: <4E58BA41.6050503@gmail.com>
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>
---
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 related
* [PATCH 01/46] Staging: hv: storvsc: Inline free_stor_device()
From: K. Y. Srinivasan @ 2011-08-27 18:31 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, virtualization; +Cc: Haiyang Zhang
In-Reply-To: <1314469874-7017-1-git-send-email-kys@microsoft.com>
Inline the code for free_stor_device() and get rid of the function.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/storvsc.c | 8 ++------
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/hv/storvsc.c b/drivers/staging/hv/storvsc.c
index 827b6a3..8c62829 100644
--- a/drivers/staging/hv/storvsc.c
+++ b/drivers/staging/hv/storvsc.c
@@ -51,10 +51,6 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)
return stor_device;
}
-static inline void free_stor_device(struct storvsc_device *device)
-{
- kfree(device);
-}
/* Get the stordevice object iff exists and its refcount > 0 */
static inline struct storvsc_device *must_get_stor_device(
@@ -394,7 +390,7 @@ int storvsc_dev_add(struct hv_device *device,
/* Send it back up */
ret = storvsc_connect_to_vsp(device, device_info->ring_buffer_size);
if (ret) {
- free_stor_device(stor_device);
+ kfree(stor_device);
return ret;
}
device_info->path_id = stor_device->path_id;
@@ -422,7 +418,7 @@ int storvsc_dev_remove(struct hv_device *device)
/* Close the channel */
vmbus_close(device->channel);
- free_stor_device(stor_device);
+ kfree(stor_device);
return 0;
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 02/46] Staging: hv: storvsc: Do not aquire an unnecessary reference on stor_device
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>
On entry into storvsc_on_io_completion() we have already acquired a reference
on the stor_device; there is no need to acquire an additional reference here.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/storvsc.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/hv/storvsc.c b/drivers/staging/hv/storvsc.c
index 8c62829..cd38cd6 100644
--- a/drivers/staging/hv/storvsc.c
+++ b/drivers/staging/hv/storvsc.c
@@ -232,9 +232,7 @@ static void storvsc_on_io_completion(struct hv_device *device,
struct storvsc_device *stor_device;
struct vstor_packet *stor_pkt;
- stor_device = must_get_stor_device(device);
- if (!stor_device)
- return;
+ stor_device = (struct storvsc_device *)device->ext;
stor_pkt = &request->vstor_packet;
@@ -279,7 +277,6 @@ static void storvsc_on_io_completion(struct hv_device *device,
wake_up(&stor_device->waiting_to_drain);
- put_stor_device(device);
}
static void storvsc_on_receive(struct hv_device *device,
--
1.7.4.1
^ permalink raw reply related
* [PATCH 03/46] Staging: hv: storvsc: Rename must_get_stor_device()
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>
In preparation for cleaning up how we manage reference counts on the stor
device, clearly distinguish why we are attempting to acquire a reference.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/storvsc.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/hv/storvsc.c b/drivers/staging/hv/storvsc.c
index cd38cd6..89708b1 100644
--- a/drivers/staging/hv/storvsc.c
+++ b/drivers/staging/hv/storvsc.c
@@ -41,7 +41,7 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)
return NULL;
/* Set to 2 to allow both inbound and outbound traffics */
- /* (ie get_stor_device() and must_get_stor_device()) to proceed. */
+ /* (ie get_stor_device() and get_in_stor_device()) to proceed. */
atomic_cmpxchg(&stor_device->ref_count, 0, 2);
init_waitqueue_head(&stor_device->waiting_to_drain);
@@ -53,7 +53,7 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)
/* Get the stordevice object iff exists and its refcount > 0 */
-static inline struct storvsc_device *must_get_stor_device(
+static inline struct storvsc_device *get_in_stor_device(
struct hv_device *device)
{
struct storvsc_device *stor_device;
@@ -305,7 +305,7 @@ static void storvsc_on_channel_callback(void *context)
int ret;
- stor_device = must_get_stor_device(device);
+ stor_device = get_in_stor_device(device);
if (!stor_device)
return;
--
1.7.4.1
^ permalink raw reply related
* [PATCH 04/46] Staging: hv: storvsc: Rename get_stor_device()
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>
In preparation for cleaning up how we manage reference counts on the stor
device, clearly distinguish why we are attempting to acquire a reference.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/hyperv_storage.h | 3 ++-
drivers/staging/hv/storvsc.c | 8 ++++----
drivers/staging/hv/storvsc_drv.c | 2 +-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/hv/hyperv_storage.h b/drivers/staging/hv/hyperv_storage.h
index a01f9a0..a224413 100644
--- a/drivers/staging/hv/hyperv_storage.h
+++ b/drivers/staging/hv/hyperv_storage.h
@@ -288,7 +288,8 @@ struct storvsc_device {
/* Get the stordevice object iff exists and its refcount > 1 */
-static inline struct storvsc_device *get_stor_device(struct hv_device *device)
+static inline struct storvsc_device *get_out_stor_device(
+ struct hv_device *device)
{
struct storvsc_device *stor_device;
diff --git a/drivers/staging/hv/storvsc.c b/drivers/staging/hv/storvsc.c
index 89708b1..313a3f8 100644
--- a/drivers/staging/hv/storvsc.c
+++ b/drivers/staging/hv/storvsc.c
@@ -41,7 +41,7 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)
return NULL;
/* Set to 2 to allow both inbound and outbound traffics */
- /* (ie get_stor_device() and get_in_stor_device()) to proceed. */
+ /* (ie get_out_stor_device() and get_in_stor_device()) to proceed. */
atomic_cmpxchg(&stor_device->ref_count, 0, 2);
init_waitqueue_head(&stor_device->waiting_to_drain);
@@ -67,7 +67,7 @@ static inline struct storvsc_device *get_in_stor_device(
return stor_device;
}
-/* Drop ref count to 1 to effectively disable get_stor_device() */
+/* Drop ref count to 1 to effectively disable get_out_stor_device() */
static inline struct storvsc_device *release_stor_device(
struct hv_device *device)
{
@@ -105,7 +105,7 @@ static int storvsc_channel_init(struct hv_device *device)
struct vstor_packet *vstor_packet;
int ret, t;
- stor_device = get_stor_device(device);
+ stor_device = get_out_stor_device(device);
if (!stor_device)
return -ENODEV;
@@ -427,7 +427,7 @@ int storvsc_do_io(struct hv_device *device,
int ret = 0;
vstor_packet = &request->vstor_packet;
- stor_device = get_stor_device(device);
+ stor_device = get_out_stor_device(device);
if (!stor_device)
return -ENODEV;
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index faa8d57..5b2004f 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -344,7 +344,7 @@ static int storvsc_host_reset(struct hv_device *device)
int ret, t;
- stor_device = get_stor_device(device);
+ stor_device = get_out_stor_device(device);
if (!stor_device)
return -ENODEV;
--
1.7.4.1
^ permalink raw reply related
* [PATCH 05/46] Staging: hv: storvsc: Cleanup alloc_stor_device()
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>
Cleanup alloc_stor_device(), we can set the ref_count directly.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/storvsc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/hv/storvsc.c b/drivers/staging/hv/storvsc.c
index 313a3f8..48bd8da 100644
--- a/drivers/staging/hv/storvsc.c
+++ b/drivers/staging/hv/storvsc.c
@@ -42,7 +42,7 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)
/* Set to 2 to allow both inbound and outbound traffics */
/* (ie get_out_stor_device() and get_in_stor_device()) to proceed. */
- atomic_cmpxchg(&stor_device->ref_count, 0, 2);
+ atomic_set(&stor_device->ref_count, 2);
init_waitqueue_head(&stor_device->waiting_to_drain);
stor_device->device = device;
--
1.7.4.1
^ permalink raw reply related
* [PATCH 06/46] Staging: hv: storvsc: Introduce state to manage the lifecycle of stor device
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>
Introduce state to manage the lifecycle of stor device. This would be the
basis for managing the references on the stor object.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/hyperv_storage.h | 2 +-
drivers/staging/hv/storvsc.c | 8 +++++++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/hv/hyperv_storage.h b/drivers/staging/hv/hyperv_storage.h
index a224413..d93bf93 100644
--- a/drivers/staging/hv/hyperv_storage.h
+++ b/drivers/staging/hv/hyperv_storage.h
@@ -266,7 +266,7 @@ struct storvsc_device {
/* 0 indicates the device is being destroyed */
atomic_t ref_count;
-
+ bool destroy;
bool drain_notify;
atomic_t num_outstanding_req;
diff --git a/drivers/staging/hv/storvsc.c b/drivers/staging/hv/storvsc.c
index 48bd8da..0f8c609 100644
--- a/drivers/staging/hv/storvsc.c
+++ b/drivers/staging/hv/storvsc.c
@@ -43,7 +43,7 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)
/* Set to 2 to allow both inbound and outbound traffics */
/* (ie get_out_stor_device() and get_in_stor_device()) to proceed. */
atomic_set(&stor_device->ref_count, 2);
-
+ stor_device->destroy = false;
init_waitqueue_head(&stor_device->waiting_to_drain);
stor_device->device = device;
device->ext = stor_device;
@@ -399,9 +399,15 @@ int storvsc_dev_add(struct hv_device *device,
int storvsc_dev_remove(struct hv_device *device)
{
struct storvsc_device *stor_device;
+ unsigned long flags;
+
stor_device = release_stor_device(device);
+ spin_lock_irqsave(&device->channel->inbound_lock, flags);
+ stor_device->destroy = true;
+ spin_unlock_irqrestore(&device->channel->inbound_lock, flags);
+
/*
* At this point, all outbound traffic should be disable. We
* only allow inbound traffic (responses) to proceed so that
--
1.7.4.1
^ permalink raw reply related
* [PATCH 07/46] Staging: hv: storvsc: Prevent outgoing traffic when stor dev is being destroyed
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>
Prevent outgoing traffic when stor dev is destroyed.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/hyperv_storage.h | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/hv/hyperv_storage.h b/drivers/staging/hv/hyperv_storage.h
index d93bf93..1a59ca0 100644
--- a/drivers/staging/hv/hyperv_storage.h
+++ b/drivers/staging/hv/hyperv_storage.h
@@ -294,7 +294,8 @@ static inline struct storvsc_device *get_out_stor_device(
struct storvsc_device *stor_device;
stor_device = (struct storvsc_device *)device->ext;
- if (stor_device && atomic_read(&stor_device->ref_count) > 1)
+ if (stor_device && (atomic_read(&stor_device->ref_count) > 1) &&
+ !stor_device->destroy)
atomic_inc(&stor_device->ref_count);
else
stor_device = NULL;
--
1.7.4.1
^ permalink raw reply related
* [PATCH 08/46] Staging: hv: storvsc: Get rid of release_stor_device() 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 release_stor_device() by inlining the code.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/storvsc.c | 19 ++-----------------
1 files changed, 2 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/hv/storvsc.c b/drivers/staging/hv/storvsc.c
index 0f8c609..1976a34 100644
--- a/drivers/staging/hv/storvsc.c
+++ b/drivers/staging/hv/storvsc.c
@@ -67,21 +67,6 @@ static inline struct storvsc_device *get_in_stor_device(
return stor_device;
}
-/* Drop ref count to 1 to effectively disable get_out_stor_device() */
-static inline struct storvsc_device *release_stor_device(
- struct hv_device *device)
-{
- struct storvsc_device *stor_device;
-
- stor_device = (struct storvsc_device *)device->ext;
-
- /* Busy wait until the ref drop to 2, then set it to 1 */
- while (atomic_cmpxchg(&stor_device->ref_count, 2, 1) != 2)
- udelay(100);
-
- return stor_device;
-}
-
/* Drop ref count to 0. No one can use stor_device object. */
static inline struct storvsc_device *final_release_stor_device(
struct hv_device *device)
@@ -401,8 +386,8 @@ int storvsc_dev_remove(struct hv_device *device)
struct storvsc_device *stor_device;
unsigned long flags;
-
- stor_device = release_stor_device(device);
+ stor_device = (struct storvsc_device *)device->ext;
+ atomic_dec(&stor_device->ref_count);
spin_lock_irqsave(&device->channel->inbound_lock, flags);
stor_device->destroy = true;
--
1.7.4.1
^ permalink raw reply related
* [PATCH 09/46] Staging: hv: storvsc: Get rid of final_release_stor_device() by inlining 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 final_release_stor_device() by inlining code.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/storvsc.c | 23 ++++++-----------------
1 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/hv/storvsc.c b/drivers/staging/hv/storvsc.c
index 1976a34..3e9829f 100644
--- a/drivers/staging/hv/storvsc.c
+++ b/drivers/staging/hv/storvsc.c
@@ -67,22 +67,6 @@ static inline struct storvsc_device *get_in_stor_device(
return stor_device;
}
-/* Drop ref count to 0. No one can use stor_device object. */
-static inline struct storvsc_device *final_release_stor_device(
- struct hv_device *device)
-{
- struct storvsc_device *stor_device;
-
- stor_device = (struct storvsc_device *)device->ext;
-
- /* Busy wait until the ref drop to 1, then set it to 0 */
- while (atomic_cmpxchg(&stor_device->ref_count, 1, 0) != 1)
- udelay(100);
-
- device->ext = NULL;
- return stor_device;
-}
-
static int storvsc_channel_init(struct hv_device *device)
{
struct storvsc_device *stor_device;
@@ -401,7 +385,12 @@ int storvsc_dev_remove(struct hv_device *device)
storvsc_wait_to_drain(stor_device);
- stor_device = final_release_stor_device(device);
+ /*
+ * Since we have already drained, we don't need to busy wait
+ * as was done in final_release_stor_device()
+ */
+ atomic_set(&stor_device->ref_count, 0);
+ device->ext = NULL;
/* Close the channel */
vmbus_close(device->channel);
--
1.7.4.1
^ permalink raw reply related
* [PATCH 10/46] Staging: hv: storvsc: Get rid of the reference counting in struct storvsc_device
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 the reference counting in struct storvsc_device. We manage the lifecycle with
the following logic: If the device is marked for destruction, we dot allow any
outgoing traffic on the device. Incoming traffic is allowed only to drain pending
outgoing traffic. Note that while the upper level code in Linux deals with outstanding
I/Os, we may have situations on Hyper-V where some book keeping messages are sent out
that the upper level Linux code may not be aware of.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/hyperv_storage.h | 18 ++----------------
drivers/staging/hv/storvsc.c | 33 +++++++++++++++++++++------------
drivers/staging/hv/storvsc_drv.c | 1 -
3 files changed, 23 insertions(+), 29 deletions(-)
diff --git a/drivers/staging/hv/hyperv_storage.h b/drivers/staging/hv/hyperv_storage.h
index 1a59ca0..687cdc5 100644
--- a/drivers/staging/hv/hyperv_storage.h
+++ b/drivers/staging/hv/hyperv_storage.h
@@ -264,8 +264,6 @@ struct storvsc_major_info {
struct storvsc_device {
struct hv_device *device;
- /* 0 indicates the device is being destroyed */
- atomic_t ref_count;
bool destroy;
bool drain_notify;
atomic_t num_outstanding_req;
@@ -287,32 +285,20 @@ struct storvsc_device {
};
-/* Get the stordevice object iff exists and its refcount > 1 */
static inline struct storvsc_device *get_out_stor_device(
struct hv_device *device)
{
struct storvsc_device *stor_device;
stor_device = (struct storvsc_device *)device->ext;
- if (stor_device && (atomic_read(&stor_device->ref_count) > 1) &&
- !stor_device->destroy)
- atomic_inc(&stor_device->ref_count);
- else
+
+ if (stor_device && stor_device->destroy)
stor_device = NULL;
return stor_device;
}
-static inline void put_stor_device(struct hv_device *device)
-{
- struct storvsc_device *stor_device;
-
- stor_device = (struct storvsc_device *)device->ext;
-
- atomic_dec(&stor_device->ref_count);
-}
-
static inline void storvsc_wait_to_drain(struct storvsc_device *dev)
{
dev->drain_notify = true;
diff --git a/drivers/staging/hv/storvsc.c b/drivers/staging/hv/storvsc.c
index 3e9829f..fb7b3ca 100644
--- a/drivers/staging/hv/storvsc.c
+++ b/drivers/staging/hv/storvsc.c
@@ -40,9 +40,6 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)
if (!stor_device)
return NULL;
- /* Set to 2 to allow both inbound and outbound traffics */
- /* (ie get_out_stor_device() and get_in_stor_device()) to proceed. */
- atomic_set(&stor_device->ref_count, 2);
stor_device->destroy = false;
init_waitqueue_head(&stor_device->waiting_to_drain);
stor_device->device = device;
@@ -52,19 +49,31 @@ static inline struct storvsc_device *alloc_stor_device(struct hv_device *device)
}
-/* Get the stordevice object iff exists and its refcount > 0 */
static inline struct storvsc_device *get_in_stor_device(
struct hv_device *device)
{
struct storvsc_device *stor_device;
+ unsigned long flags;
+ spin_lock_irqsave(&device->channel->inbound_lock, flags);
stor_device = (struct storvsc_device *)device->ext;
- if (stor_device && atomic_read(&stor_device->ref_count))
- atomic_inc(&stor_device->ref_count);
- else
+
+ if (!stor_device)
+ goto get_in_err;
+
+ /*
+ * If the device is being destroyed; allow incoming
+ * traffic only to cleanup outstanding requests.
+ */
+
+ if (stor_device->destroy &&
+ (atomic_read(&stor_device->num_outstanding_req) == 0))
stor_device = NULL;
+get_in_err:
+ spin_unlock_irqrestore(&device->channel->inbound_lock, flags);
return stor_device;
+
}
static int storvsc_channel_init(struct hv_device *device)
@@ -190,7 +199,6 @@ static int storvsc_channel_init(struct hv_device *device)
cleanup:
- put_stor_device(device);
return ret;
}
@@ -303,7 +311,6 @@ static void storvsc_on_channel_callback(void *context)
}
} while (1);
- put_stor_device(device);
return;
}
@@ -371,7 +378,6 @@ int storvsc_dev_remove(struct hv_device *device)
unsigned long flags;
stor_device = (struct storvsc_device *)device->ext;
- atomic_dec(&stor_device->ref_count);
spin_lock_irqsave(&device->channel->inbound_lock, flags);
stor_device->destroy = true;
@@ -388,9 +394,13 @@ int storvsc_dev_remove(struct hv_device *device)
/*
* Since we have already drained, we don't need to busy wait
* as was done in final_release_stor_device()
+ * Note that we cannot set the ext pointer to NULL until
+ * we have drained - to drain the outgoing packets, we need to
+ * allow incoming packets.
*/
- atomic_set(&stor_device->ref_count, 0);
+ spin_lock_irqsave(&device->channel->inbound_lock, flags);
device->ext = NULL;
+ spin_unlock_irqrestore(&device->channel->inbound_lock, flags);
/* Close the channel */
vmbus_close(device->channel);
@@ -448,7 +458,6 @@ int storvsc_do_io(struct hv_device *device,
atomic_inc(&stor_device->num_outstanding_req);
- put_stor_device(device);
return ret;
}
diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
index 5b2004f..ae74f50 100644
--- a/drivers/staging/hv/storvsc_drv.c
+++ b/drivers/staging/hv/storvsc_drv.c
@@ -378,7 +378,6 @@ static int storvsc_host_reset(struct hv_device *device)
*/
cleanup:
- put_stor_device(device);
return ret;
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH 11/46] Staging: hv: netvsc: Inline the code for free_net_device()
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>
Inline the code for free_net_device().
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/netvsc.c | 12 ++----------
1 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/hv/netvsc.c b/drivers/staging/hv/netvsc.c
index b6e1fb9..75c6ed7 100644
--- a/drivers/staging/hv/netvsc.c
+++ b/drivers/staging/hv/netvsc.c
@@ -49,14 +49,6 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)
return net_device;
}
-static void free_net_device(struct netvsc_device *device)
-{
- WARN_ON(atomic_read(&device->refcnt) != 0);
- device->dev->ext = NULL;
- kfree(device);
-}
-
-
/* Get the net device object iff exists and its refcount > 1 */
static struct netvsc_device *get_outbound_net_device(struct hv_device *device)
{
@@ -438,7 +430,7 @@ int netvsc_device_remove(struct hv_device *device)
kfree(netvsc_packet);
}
- free_net_device(net_device);
+ kfree(net_device);
return 0;
}
@@ -980,7 +972,7 @@ cleanup:
release_outbound_net_device(device);
release_inbound_net_device(device);
- free_net_device(net_device);
+ kfree(net_device);
}
return ret;
--
1.7.4.1
^ permalink raw reply related
* [PATCH 12/46] Staging: hv: netvsc: Cleanup alloc_net_device()
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>
Cleanup alloc_net_device(); we can directly set the refcnt.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/netvsc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/hv/netvsc.c b/drivers/staging/hv/netvsc.c
index 75c6ed7..7722102 100644
--- a/drivers/staging/hv/netvsc.c
+++ b/drivers/staging/hv/netvsc.c
@@ -41,7 +41,7 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)
return NULL;
/* Set to 2 to allow both inbound and outbound traffic */
- atomic_cmpxchg(&net_device->refcnt, 0, 2);
+ atomic_set(&net_device->refcnt, 2);
net_device->dev = device;
device->ext = net_device;
--
1.7.4.1
^ permalink raw reply related
* [PATCH 13/46] Staging: hv: netvsc: Introduce state to manage the lifecycle of net device
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>
Introduce state to manage the lifecycle of net device.
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/staging/hv/hyperv_net.h | 1 +
drivers/staging/hv/netvsc.c | 6 ++++++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/staging/hv/hyperv_net.h b/drivers/staging/hv/hyperv_net.h
index 5782fea..0b347c1 100644
--- a/drivers/staging/hv/hyperv_net.h
+++ b/drivers/staging/hv/hyperv_net.h
@@ -371,6 +371,7 @@ struct netvsc_device {
atomic_t refcnt;
atomic_t num_outstanding_sends;
+ bool destroy;
/*
* List of free preallocated hv_netvsc_packet to represent receive
* packet
diff --git a/drivers/staging/hv/netvsc.c b/drivers/staging/hv/netvsc.c
index 7722102..8eb4039 100644
--- a/drivers/staging/hv/netvsc.c
+++ b/drivers/staging/hv/netvsc.c
@@ -43,6 +43,7 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)
/* Set to 2 to allow both inbound and outbound traffic */
atomic_set(&net_device->refcnt, 2);
+ net_device->destroy = false;
net_device->dev = device;
device->ext = net_device;
@@ -396,6 +397,7 @@ int netvsc_device_remove(struct hv_device *device)
{
struct netvsc_device *net_device;
struct hv_netvsc_packet *netvsc_packet, *pos;
+ unsigned long flags;
/* Stop outbound traffic ie sends and receives completions */
net_device = release_outbound_net_device(device);
@@ -404,6 +406,10 @@ int netvsc_device_remove(struct hv_device *device)
return -ENODEV;
}
+ spin_lock_irqsave(&device->channel->inbound_lock, flags);
+ net_device->destroy = true;
+ spin_unlock_irqrestore(&device->channel->inbound_lock, flags);
+
/* Wait for all send completions */
while (atomic_read(&net_device->num_outstanding_sends)) {
dev_err(&device->device,
--
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