What is struct pci_driver.owner for?

What is struct pci_driver.owner for?

[Roland Dreier]

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.

Re: What is struct pci_driver.owner for?

[Jeff Garzik]

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

Re: What is struct pci_driver.owner for?

[Greg KH]

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	

Re: What is struct pci_driver.owner for?

[Russell King]

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

Re: What is struct pci_driver.owner for?

[Roland Dreier]

    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)

Re: What is struct pci_driver.owner for?

[Jiri Slaby]

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         www.fi.muni.cz/~xslaby
~\-/~      jirislaby@gmail.com      ~\-/~
B67499670407CE62ACC8 22A032CC55C339D47A7E

Re: What is struct pci_driver.owner for?

[Roland Dreier]

    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.

Re: What is struct pci_driver.owner for?

[Jiri Slaby]

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         www.fi.muni.cz/~xslaby
~\-/~      jirislaby@gmail.com      ~\-/~
B67499670407CE62ACC8 22A032CC55C339D47A7E