I just noticed that at some point, struct pci_driver grew a .owner
member. However, only a handful of drivers set it:
$ grep -r -A10 pci_driver drivers/ | grep owner
drivers/block/sx8.c- .owner = THIS_MODULE,
drivers/ieee1394/pcilynx.c- .owner = THIS_MODULE,
drivers/net/spider_net.c- .owner = THIS_MODULE,
drivers/video/imsttfb.c- .owner = THIS_MODULE,
drivers/video/kyro/fbdev.c- .owner = THIS_MODULE,
drivers/video/tridentfb.c- .owner = THIS_MODULE,
Should all drivers be setting .owner = THIS_MODULE? Is this a good
kernel janitors task?
- R.
Roland Dreier wrote:
> I just noticed that at some point, struct pci_driver grew a .owner
> member. However, only a handful of drivers set it:
>
> $ grep -r -A10 pci_driver drivers/ | grep owner
> drivers/block/sx8.c- .owner = THIS_MODULE,
> drivers/ieee1394/pcilynx.c- .owner = THIS_MODULE,
> drivers/net/spider_net.c- .owner = THIS_MODULE,
> drivers/video/imsttfb.c- .owner = THIS_MODULE,
> drivers/video/kyro/fbdev.c- .owner = THIS_MODULE,
> drivers/video/tridentfb.c- .owner = THIS_MODULE,
>
> Should all drivers be setting .owner = THIS_MODULE? Is this a good
> kernel janitors task?
In theory its for module refcounting. With so many PCI drivers and so
few pci_driver::owner users, it makes me wonder how needed it is.
If it is needed (I've done no analysis, even though I am sx8.c author),
then it should be applied uniformly.
Jeff
On Tue, Oct 18, 2005 at 04:53:36PM -0400, Jeff Garzik wrote:
> Roland Dreier wrote:
> >I just noticed that at some point, struct pci_driver grew a .owner
> >member. However, only a handful of drivers set it:
> >
> > $ grep -r -A10 pci_driver drivers/ | grep owner
> > drivers/block/sx8.c- .owner = THIS_MODULE,
> > drivers/ieee1394/pcilynx.c- .owner = THIS_MODULE,
> > drivers/net/spider_net.c- .owner = THIS_MODULE,
> > drivers/video/imsttfb.c- .owner = THIS_MODULE,
> > drivers/video/kyro/fbdev.c- .owner = THIS_MODULE,
> > drivers/video/tridentfb.c- .owner = THIS_MODULE,
> >
> >Should all drivers be setting .owner = THIS_MODULE? Is this a good
> >kernel janitors task?
>
> In theory its for module refcounting. With so many PCI drivers and so
> few pci_driver::owner users, it makes me wonder how needed it is.
It might in the future be needed for refcounting, I originally added it
when I thought it was needed.
But what it really does today is create the symlink from the driver to
the module that is contained in it, in sysfs. Which is very invaluable
for people who want to know these things (installer programs, etc.)
For example:
$ tree /sys/bus/pci/drivers/uhci_hcd/
/sys/bus/pci/drivers/uhci_hcd/
|-- 0000:00:1d.0 -> ../../../../devices/pci0000:00/0000:00:1d.0
|-- 0000:00:1d.1 -> ../../../../devices/pci0000:00/0000:00:1d.1
|-- 0000:00:1d.2 -> ../../../../devices/pci0000:00/0000:00:1d.2
|-- 0000:00:1d.3 -> ../../../../devices/pci0000:00/0000:00:1d.3
|-- bind
|-- module -> ../../../../module/uhci_hcd
|-- new_id
`-- unbind
That "module" symlink is created only if the .owner field is set.
That's why people are going through and adding it to all of the drivers
in the system.
Hope this helps,
greg k-h
On Tue, Oct 18, 2005 at 01:59:08PM -0700, Greg KH wrote:
> On Tue, Oct 18, 2005 at 04:53:36PM -0400, Jeff Garzik wrote:
> > Roland Dreier wrote:
> > >I just noticed that at some point, struct pci_driver grew a .owner
> > >member. However, only a handful of drivers set it:
> > >
> > > $ grep -r -A10 pci_driver drivers/ | grep owner
> > > drivers/block/sx8.c- .owner = THIS_MODULE,
> > > drivers/ieee1394/pcilynx.c- .owner = THIS_MODULE,
> > > drivers/net/spider_net.c- .owner = THIS_MODULE,
> > > drivers/video/imsttfb.c- .owner = THIS_MODULE,
> > > drivers/video/kyro/fbdev.c- .owner = THIS_MODULE,
> > > drivers/video/tridentfb.c- .owner = THIS_MODULE,
> > >
> > >Should all drivers be setting .owner = THIS_MODULE? Is this a good
> > >kernel janitors task?
> >
> > In theory its for module refcounting. With so many PCI drivers and so
> > few pci_driver::owner users, it makes me wonder how needed it is.
>
> It might in the future be needed for refcounting, I originally added it
> when I thought it was needed.
Note that both pci_driver and pci_driver.driver both have an "owner"
field. One should go - there's no point needlessly duplicating stuff
like this.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
Greg> But what it really does today is create the symlink from the
Greg> driver to the module that is contained in it, in sysfs.
Greg> Which is very invaluable for people who want to know these
Greg> things (installer programs, etc.)
Greg> That "module" symlink is created only if the .owner field is
Greg> set. That's why people are going through and adding it to
Greg> all of the drivers in the system.
OK, I'll make my own small contribution and push this for 2.6.15:
diff --git a/drivers/infiniband/hw/mthca/mthca_main.c b/drivers/infiniband/hw/mthca/mthca_main.c
--- a/drivers/infiniband/hw/mthca/mthca_main.c
+++ b/drivers/infiniband/hw/mthca/mthca_main.c
@@ -1196,6 +1196,7 @@ MODULE_DEVICE_TABLE(pci, mthca_pci_table
static struct pci_driver mthca_driver = {
.name = DRV_NAME,
+ .owner = THIS_MODULE,
.id_table = mthca_pci_table,
.probe = mthca_init_one,
.remove = __devexit_p(mthca_remove_one)
Roland Dreier napsal(a):
> Greg> But what it really does today is create the symlink from the
> Greg> driver to the module that is contained in it, in sysfs.
> Greg> Which is very invaluable for people who want to know these
> Greg> things (installer programs, etc.)
>
> Greg> That "module" symlink is created only if the .owner field is
> Greg> set. That's why people are going through and adding it to
> Greg> all of the drivers in the system.
>
>OK, I'll make my own small contribution and push this for 2.6.15:
>
>
Ok, but read Documentation/SubmittingPatches. At least Signed-off-by is
missing.
>diff --git a/drivers/infiniband/hw/mthca/mthca_main.c b/drivers/infiniband/hw/mthca/mthca_main.c
>--- a/drivers/infiniband/hw/mthca/mthca_main.c
>+++ b/drivers/infiniband/hw/mthca/mthca_main.c
>@@ -1196,6 +1196,7 @@ MODULE_DEVICE_TABLE(pci, mthca_pci_table
>
> static struct pci_driver mthca_driver = {
> .name = DRV_NAME,
>+ .owner = THIS_MODULE,
> .id_table = mthca_pci_table,
> .probe = mthca_init_one,
> .remove = __devexit_p(mthca_remove_one)
>
>
regards,
--
Jiri Slaby http://www.fi.muni.cz/~xslaby
~\-/~ [email protected] ~\-/~
B67499670407CE62ACC8 22A032CC55C339D47A7E
Jiri> Ok, but read Documentation/SubmittingPatches. At least
Jiri> Signed-off-by is missing.
Thanks, but that wasn't really a patch for anyone to accept. It was
just a note about what I just put into my git tree, which I'll ask
Linus to pull from directly when 2.6.15 opens. Of course in the real
commit message I put a nice description and Signed-off-by: line.
- R.
Roland Dreier napsal(a):
> Jiri> Ok, but read Documentation/SubmittingPatches. At least
> Jiri> Signed-off-by is missing.
>
>Thanks, but that wasn't really a patch for anyone to accept. It was
>just a note about what I just put into my git tree, which I'll ask
>Linus to pull from directly when 2.6.15 opens. Of course in the real
>commit message I put a nice description and Signed-off-by: line.
>
>
Oh, sorry.
--
Jiri Slaby http://www.fi.muni.cz/~xslaby
~\-/~ [email protected] ~\-/~
B67499670407CE62ACC8 22A032CC55C339D47A7E