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 k...@microsoft.com
Signed-off-by: Haiyang Zhang haiya...@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 k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 22/59] Staging: hv: vmbus: Get rid of the unused name field in struct hv_driver
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 k...@microsoft.com
Signed-off-by: Haiyang Zhang haiya...@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...
greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
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:28:20PM -0700, Greg KH wrote:
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...
Ok, here's what I'm talking about. It properly hooks up the module
reference counting issues that you had yet to take care of as well (see,
you got that for free just by getting the logic correct, and the code is
even smaller than before overall, it's a win-win all around...)
The patch below works for me, I've committed it and will work out how to
apply the rest of your patch series now.
-
From: Greg Kroah-Hartman gre...@suse.de
Date: Thu, 25 Aug 2011 15:07:32 -0700
Subject: Staging: hv: fix up driver registering mess
Individual drivers should never be touching the 'struct device' field,
so if that is a requirement to pass to the vmbus core, you know
something is wrong.
This patch fixes that all up, and resolves the problem where the module
reference counting was not happening properly for the individual drivers
as well. Overall, it reduces the lines of code the individual drivers
have to have, which tells you that this is the correct thing to do.
Also, somehow the _GPL marking for the functions got removed on an older
patch. As the name of the function was changing, properly change the
_GPL marking as well at the same time.
Cc: K. Y. Srinivasan k...@microsoft.com
Cc: Haiyang Zhang haiya...@microsoft.com
Cc: Hank Janssen hjans...@microsoft.com
Signed-off-by: Greg Kroah-Hartman gre...@suse.de
---
drivers/staging/hv/blkvsc_drv.c | 17 ++
drivers/staging/hv/hv_mouse.c| 21 ++---
drivers/staging/hv/hv_util.c |9 ++-
drivers/staging/hv/hyperv.h |8 +-
drivers/staging/hv/hyperv_net.h |1 -
drivers/staging/hv/netvsc.c | 14
drivers/staging/hv/netvsc_drv.c | 19 ++--
drivers/staging/hv/storvsc_drv.c | 24 ++---
drivers/staging/hv/vmbus_drv.c | 43 ++---
9 files changed, 46 insertions(+), 110 deletions(-)
diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
index d170f24..07dc9ed 100644
--- a/drivers/staging/hv/blkvsc_drv.c
+++ b/drivers/staging/hv/blkvsc_drv.c
@@ -109,7 +109,6 @@ struct block_device_context {
int users;
};
-static const char *drv_name = blkvsc;
/*
* There is a circular dependency involving blkvsc_request_completion()
@@ -805,6 +804,7 @@ MODULE_DEVICE_TABLE(vmbus, id_table);
/* The one and only one */
static struct hv_driver blkvsc_drv = {
+ .name = blkvsc,
.id_table = id_table,
.probe = blkvsc_probe,
.remove = blkvsc_remove,
@@ -824,24 +824,13 @@ static const struct block_device_operations block_ops = {
*/
static int blkvsc_drv_init(void)
{
- struct hv_driver *drv = blkvsc_drv;
- int ret;
-
BUILD_BUG_ON(sizeof(sector_t) != 8);
-
- drv-driver.name = drv_name;
-
- /* The driver belongs to vmbus */
- ret = vmbus_child_driver_register(drv-driver);
-
- return ret;
+ return vmbus_driver_register(blkvsc_drv);
}
-
static void blkvsc_drv_exit(void)
{
-
- vmbus_child_driver_unregister(blkvsc_drv.driver);
+ vmbus_driver_unregister(blkvsc_drv);
}
/*
diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
index ebd1715..5727173 100644
--- a/drivers/staging/hv/hv_mouse.c
+++ b/drivers/staging/hv/hv_mouse.c
@@ -176,8 +176,6 @@ struct mousevsc_dev {
};
-static const char *driver_name = mousevsc;
-
static void deviceinfo_callback(struct hv_device *dev, struct
hv_input_dev_info *info);
static void inputreport_callback(struct hv_device *dev, void *packet, u32 len);
static void reportdesc_callback(struct hv_device *dev, void *packet, u32 len);
@@ -921,33 +919,20 @@ static const struct hv_vmbus_device_id id_table[] = {
/* MODULE_DEVICE_TABLE(vmbus, id_table); */
static struct hv_driver mousevsc_drv = {
+ .name = mousevsc,
.id_table = id_table,
.probe = mousevsc_probe,
.remove = mousevsc_remove,
};
-static void mousevsc_drv_exit(void)
-{
- vmbus_child_driver_unregister(mousevsc_drv.driver);
-}
-
static int __init mousevsc_init(void)
{
- struct hv_driver *drv = mousevsc_drv;
-
- DPRINT_INFO(INPUTVSC_DRV, Hyper-V Mouse driver initializing.);
-
- drv-driver.name = driver_name;
-
- /* The driver belongs to vmbus */
- vmbus_child_driver_register(drv-driver);
-
- return 0;
+ return vmbus_driver_register(mousevsc_drv);
}
static void __exit mousevsc_exit(void)
{
- mousevsc_drv_exit();
+
RE: [PATCH 22/59] Staging: hv: vmbus: Get rid of the unused name field in struct hv_driver
-Original Message-
From: Greg KH [mailto:g...@kroah.com]
Sent: Thursday, August 25, 2011 5:28 PM
To: KY Srinivasan
Cc: gre...@suse.de; linux-ker...@vger.kernel.org;
de...@linuxdriverproject.org; virtualizat...@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 k...@microsoft.com
Signed-off-by: Haiyang Zhang haiya...@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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 22/59] Staging: hv: vmbus: Get rid of the unused name field in struct hv_driver
On Thu, Aug 25, 2011 at 10:27:40PM +, 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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
