Re: [PATCH 15/16 v6] PCI: document the SR-IOV sysfs entries

2008-11-18 Thread Greg KH
On Mon, Nov 17, 2008 at 04:09:49PM +0800, Yu Zhao wrote:
 On Fri, Nov 14, 2008 at 08:55:38AM +0800, Greg KH wrote:
  On Thu, Nov 13, 2008 at 02:50:24PM +0800, Yu Zhao wrote:
   On Fri, Nov 07, 2008 at 11:18:37AM +0800, Greg KH wrote:
On Fri, Nov 07, 2008 at 11:01:29AM +0800, Zhao, Yu wrote:
 Currently PCI subsystem has /sys/.../{vendor,device,...} bundled to 
 the 
 main PCI device (I suppose this means the entries are created by 
 'device_add')

 And after the PCI device is announced, 
 /sys/.../{config,resourceX,rom,vpd,iov,...} get created depending on 
 if 
 these features are supported.

And that's a bug.  Let's not continue to make the same bug here as well.

 Making dynamic entries tie to the main PCI device would require PCI 
 subsystem to allocate different 'bus_type' for the devices, right?

No, it would just mean they need to be all added before the device is
fully registered with the driver core.
   
   I looked into the PCI and driver core code again, but didn't figured out 
   how
   to do it.
   
   A 'pci_dev' is added by pci_bus_add_device() via device_add(), which 
   creates
   sysfs entries according to 'dev_attrs' in the 'pci_bus_type'. If we want 
   those
   dynamic entries to appear before the uevent is triggered, we have to 
   bundle
   them into the 'dev_attrs'. Is this right way for the dynamic entries? Or I
   missed something?
  
  Yes, that is correct.
  
  Or you can add attributes before device_add() is called to the device,
  which is probably much easier to do, right?
  
  There are also conditional attributes, which get only displayed if
  some kind of condition is met, I think you want to use those.
 
 The problem is the sysfs directory of the PCI device is created by the
 kobject_add() in the device_add() as follows. And the static entries
 bundled with the 'pci_bus_type' are created by the bus_add_device().
 Between the kobject_add() and the kobject_uevent(), we don't have any
 other choice to add the dynamic entries.
 
 In device_add():
 
   error = kobject_add(dev-kobj, dev-kobj.parent, %s, dev-bus_id);
   ...
   error = bus_add_device(dev);
   ...
   kobject_uevent(dev-kobj, KOBJ_ADD);
 
 
 So looks like the only way is to make the dynamic entries bundled with
 the 'pci_bus_type', which means they would become static no matter the
 device supports the entries (i.e. corresponding capabilities) or not.

No, this can work, other busses do this.  There are conditional
attributes that only get enabled if specific things happen, and you can
add attributes before device_add() is called.  See the scsi code for
examples of both of these options.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 15/16 v6] PCI: document the SR-IOV sysfs entries

2008-11-18 Thread Kay Sievers
On Mon, Nov 17, 2008 at 09:09, Yu Zhao [EMAIL PROTECTED] wrote:
 On Fri, Nov 14, 2008 at 08:55:38AM +0800, Greg KH wrote:

 There are also conditional attributes, which get only displayed if
 some kind of condition is met, I think you want to use those.

 The problem is the sysfs directory of the PCI device is created by the
 kobject_add() in the device_add() as follows. And the static entries
 bundled with the 'pci_bus_type' are created by the bus_add_device().
 Between the kobject_add() and the kobject_uevent(), we don't have any
 other choice to add the dynamic entries.

 In device_add():

error = kobject_add(dev-kobj, dev-kobj.parent, %s, dev-bus_id);
...
error = bus_add_device(dev);
...
kobject_uevent(dev-kobj, KOBJ_ADD);


 So looks like the only way is to make the dynamic entries bundled with
 the 'pci_bus_type', which means they would become static no matter the
 device supports the entries (i.e. corresponding capabilities) or not.

There is device_add_attrs() which is just called between the calls you
mention above. Like Greg said, it can add groups, and groups have an
is_visible() callback, which can be used to conditionally create
attributes out of of predefined list.

Kay
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 15/16 v6] PCI: document the SR-IOV sysfs entries

2008-11-17 Thread Yu Zhao
On Fri, Nov 14, 2008 at 08:55:38AM +0800, Greg KH wrote:
 On Thu, Nov 13, 2008 at 02:50:24PM +0800, Yu Zhao wrote:
  On Fri, Nov 07, 2008 at 11:18:37AM +0800, Greg KH wrote:
   On Fri, Nov 07, 2008 at 11:01:29AM +0800, Zhao, Yu wrote:
Currently PCI subsystem has /sys/.../{vendor,device,...} bundled to the 
main PCI device (I suppose this means the entries are created by 
'device_add')
   
And after the PCI device is announced, 
/sys/.../{config,resourceX,rom,vpd,iov,...} get created depending on if 
these features are supported.
   
   And that's a bug.  Let's not continue to make the same bug here as well.
   
Making dynamic entries tie to the main PCI device would require PCI 
subsystem to allocate different 'bus_type' for the devices, right?
   
   No, it would just mean they need to be all added before the device is
   fully registered with the driver core.
  
  I looked into the PCI and driver core code again, but didn't figured out how
  to do it.
  
  A 'pci_dev' is added by pci_bus_add_device() via device_add(), which creates
  sysfs entries according to 'dev_attrs' in the 'pci_bus_type'. If we want 
  those
  dynamic entries to appear before the uevent is triggered, we have to bundle
  them into the 'dev_attrs'. Is this right way for the dynamic entries? Or I
  missed something?
 
 Yes, that is correct.
 
 Or you can add attributes before device_add() is called to the device,
 which is probably much easier to do, right?
 
 There are also conditional attributes, which get only displayed if
 some kind of condition is met, I think you want to use those.

The problem is the sysfs directory of the PCI device is created by the
kobject_add() in the device_add() as follows. And the static entries
bundled with the 'pci_bus_type' are created by the bus_add_device().
Between the kobject_add() and the kobject_uevent(), we don't have any
other choice to add the dynamic entries.

In device_add():

error = kobject_add(dev-kobj, dev-kobj.parent, %s, dev-bus_id);
...
error = bus_add_device(dev);
...
kobject_uevent(dev-kobj, KOBJ_ADD);


So looks like the only way is to make the dynamic entries bundled with
the 'pci_bus_type', which means they would become static no matter the
device supports the entries (i.e. corresponding capabilities) or not.

Thanks,
Yu
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 15/16 v6] PCI: document the SR-IOV sysfs entries

2008-11-13 Thread Greg KH
On Thu, Nov 13, 2008 at 02:50:24PM +0800, Yu Zhao wrote:
 On Fri, Nov 07, 2008 at 11:18:37AM +0800, Greg KH wrote:
  On Fri, Nov 07, 2008 at 11:01:29AM +0800, Zhao, Yu wrote:
   Greg KH wrote:
   On Wed, Nov 05, 2008 at 08:33:18PM -0800, Greg KH wrote:
   On Wed, Oct 22, 2008 at 04:45:15PM +0800, Yu Zhao wrote:
Documentation/ABI/testing/sysfs-bus-pci |   33 
   +++
1 files changed, 33 insertions(+), 0 deletions(-)
  
   diff --git a/Documentation/ABI/testing/sysfs-bus-pci 
   b/Documentation/ABI/testing/sysfs-bus-pci
   index ceddcff..41cce8f 100644
   --- a/Documentation/ABI/testing/sysfs-bus-pci
   +++ b/Documentation/ABI/testing/sysfs-bus-pci
   @@ -9,3 +9,36 @@ Description:
  that some devices may have malformatted data.  If the
  underlying VPD has a writable section then the
  corresponding section of this file will be writable.
   +
   +What: /sys/bus/pci/devices/.../iov/enable
   Are you sure this is still the correct location with your change to
   struct device?
   Nevermind, this is correct.
   But the bigger problem is that userspace doesn't know when these
   attributes show up.  So tools like udev and HAL and others can't look
   for them as they never get notified, and they don't even know if they
   should be looking for them or not.
   Is there any way to tie these attributes to the main pci device so
   that they get created before the device is announced to the world?
   Doing that would solve this issue.
   thanks,
   greg k-h
  
   Currently PCI subsystem has /sys/.../{vendor,device,...} bundled to the 
   main PCI device (I suppose this means the entries are created by 
   'device_add')
  
   And after the PCI device is announced, 
   /sys/.../{config,resourceX,rom,vpd,iov,...} get created depending on if 
   these features are supported.
  
  And that's a bug.  Let's not continue to make the same bug here as well.
  
   Making dynamic entries tie to the main PCI device would require PCI 
   subsystem to allocate different 'bus_type' for the devices, right?
  
  No, it would just mean they need to be all added before the device is
  fully registered with the driver core.
 
 I looked into the PCI and driver core code again, but didn't figured out how
 to do it.
 
 A 'pci_dev' is added by pci_bus_add_device() via device_add(), which creates
 sysfs entries according to 'dev_attrs' in the 'pci_bus_type'. If we want those
 dynamic entries to appear before the uevent is triggered, we have to bundle
 them into the 'dev_attrs'. Is this right way for the dynamic entries? Or I
 missed something?

Yes, that is correct.

Or you can add attributes before device_add() is called to the device,
which is probably much easier to do, right?

There are also conditional attributes, which get only displayed if
some kind of condition is met, I think you want to use those.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 15/16 v6] PCI: document the SR-IOV sysfs entries

2008-11-12 Thread Yu Zhao
On Fri, Nov 07, 2008 at 11:18:37AM +0800, Greg KH wrote:
 On Fri, Nov 07, 2008 at 11:01:29AM +0800, Zhao, Yu wrote:
  Greg KH wrote:
  On Wed, Nov 05, 2008 at 08:33:18PM -0800, Greg KH wrote:
  On Wed, Oct 22, 2008 at 04:45:15PM +0800, Yu Zhao wrote:
   Documentation/ABI/testing/sysfs-bus-pci |   33 
  +++
   1 files changed, 33 insertions(+), 0 deletions(-)
 
  diff --git a/Documentation/ABI/testing/sysfs-bus-pci 
  b/Documentation/ABI/testing/sysfs-bus-pci
  index ceddcff..41cce8f 100644
  --- a/Documentation/ABI/testing/sysfs-bus-pci
  +++ b/Documentation/ABI/testing/sysfs-bus-pci
  @@ -9,3 +9,36 @@ Description:
   that some devices may have malformatted data.  If the
   underlying VPD has a writable section then the
   corresponding section of this file will be writable.
  +
  +What:   /sys/bus/pci/devices/.../iov/enable
  Are you sure this is still the correct location with your change to
  struct device?
  Nevermind, this is correct.
  But the bigger problem is that userspace doesn't know when these
  attributes show up.  So tools like udev and HAL and others can't look
  for them as they never get notified, and they don't even know if they
  should be looking for them or not.
  Is there any way to tie these attributes to the main pci device so
  that they get created before the device is announced to the world?
  Doing that would solve this issue.
  thanks,
  greg k-h
 
  Currently PCI subsystem has /sys/.../{vendor,device,...} bundled to the 
  main PCI device (I suppose this means the entries are created by 
  'device_add')
 
  And after the PCI device is announced, 
  /sys/.../{config,resourceX,rom,vpd,iov,...} get created depending on if 
  these features are supported.
 
 And that's a bug.  Let's not continue to make the same bug here as well.
 
  Making dynamic entries tie to the main PCI device would require PCI 
  subsystem to allocate different 'bus_type' for the devices, right?
 
 No, it would just mean they need to be all added before the device is
 fully registered with the driver core.

I looked into the PCI and driver core code again, but didn't figured out how
to do it.

A 'pci_dev' is added by pci_bus_add_device() via device_add(), which creates
sysfs entries according to 'dev_attrs' in the 'pci_bus_type'. If we want those
dynamic entries to appear before the uevent is triggered, we have to bundle
them into the 'dev_attrs'. Is this right way for the dynamic entries? Or I
missed something?

Thanks,
Yu
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 15/16 v6] PCI: document the SR-IOV sysfs entries

2008-11-06 Thread Zhao, Yu

Greg KH wrote:

On Wed, Nov 05, 2008 at 08:33:18PM -0800, Greg KH wrote:

On Wed, Oct 22, 2008 at 04:45:15PM +0800, Yu Zhao wrote:

 Documentation/ABI/testing/sysfs-bus-pci |   33 +++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci 
b/Documentation/ABI/testing/sysfs-bus-pci
index ceddcff..41cce8f 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -9,3 +9,36 @@ Description:
that some devices may have malformatted data.  If the
underlying VPD has a writable section then the
corresponding section of this file will be writable.
+
+What:  /sys/bus/pci/devices/.../iov/enable

Are you sure this is still the correct location with your change to
struct device?


Nevermind, this is correct.

But the bigger problem is that userspace doesn't know when these
attributes show up.  So tools like udev and HAL and others can't look
for them as they never get notified, and they don't even know if they
should be looking for them or not.

Is there any way to tie these attributes to the main pci device so
that they get created before the device is announced to the world?
Doing that would solve this issue.

thanks,

greg k-h


Currently PCI subsystem has /sys/.../{vendor,device,...} bundled to the 
main PCI device (I suppose this means the entries are created by 
'device_add')


And after the PCI device is announced, 
/sys/.../{config,resourceX,rom,vpd,iov,...} get created depending on if 
these features are supported.


Making dynamic entries tie to the main PCI device would require PCI 
subsystem to allocate different 'bus_type' for the devices, right?


Regards,
Yu
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 15/16 v6] PCI: document the SR-IOV sysfs entries

2008-11-06 Thread Greg KH
On Fri, Nov 07, 2008 at 11:01:29AM +0800, Zhao, Yu wrote:
 Greg KH wrote:
 On Wed, Nov 05, 2008 at 08:33:18PM -0800, Greg KH wrote:
 On Wed, Oct 22, 2008 at 04:45:15PM +0800, Yu Zhao wrote:
  Documentation/ABI/testing/sysfs-bus-pci |   33 
 +++
  1 files changed, 33 insertions(+), 0 deletions(-)

 diff --git a/Documentation/ABI/testing/sysfs-bus-pci 
 b/Documentation/ABI/testing/sysfs-bus-pci
 index ceddcff..41cce8f 100644
 --- a/Documentation/ABI/testing/sysfs-bus-pci
 +++ b/Documentation/ABI/testing/sysfs-bus-pci
 @@ -9,3 +9,36 @@ Description:
that some devices may have malformatted data.  If the
underlying VPD has a writable section then the
corresponding section of this file will be writable.
 +
 +What: /sys/bus/pci/devices/.../iov/enable
 Are you sure this is still the correct location with your change to
 struct device?
 Nevermind, this is correct.
 But the bigger problem is that userspace doesn't know when these
 attributes show up.  So tools like udev and HAL and others can't look
 for them as they never get notified, and they don't even know if they
 should be looking for them or not.
 Is there any way to tie these attributes to the main pci device so
 that they get created before the device is announced to the world?
 Doing that would solve this issue.
 thanks,
 greg k-h

 Currently PCI subsystem has /sys/.../{vendor,device,...} bundled to the 
 main PCI device (I suppose this means the entries are created by 
 'device_add')

 And after the PCI device is announced, 
 /sys/.../{config,resourceX,rom,vpd,iov,...} get created depending on if 
 these features are supported.

And that's a bug.  Let's not continue to make the same bug here as well.

 Making dynamic entries tie to the main PCI device would require PCI 
 subsystem to allocate different 'bus_type' for the devices, right?

No, it would just mean they need to be all added before the device is
fully registered with the driver core.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 15/16 v6] PCI: document the SR-IOV sysfs entries

2008-11-05 Thread Greg KH
On Wed, Nov 05, 2008 at 08:33:18PM -0800, Greg KH wrote:
 On Wed, Oct 22, 2008 at 04:45:15PM +0800, Yu Zhao wrote:
  Document the SR-IOV sysfs entries.
  
  Cc: Alex Chiang [EMAIL PROTECTED]
  Cc: Grant Grundler [EMAIL PROTECTED]
  Cc: Greg KH [EMAIL PROTECTED]
  Cc: Ingo Molnar [EMAIL PROTECTED]
  Cc: Jesse Barnes [EMAIL PROTECTED]
  Cc: Matthew Wilcox [EMAIL PROTECTED]
  Cc: Randy Dunlap [EMAIL PROTECTED]
  Cc: Roland Dreier [EMAIL PROTECTED]
  Signed-off-by: Yu Zhao [EMAIL PROTECTED]
  
  ---
   Documentation/ABI/testing/sysfs-bus-pci |   33 
  +++
   1 files changed, 33 insertions(+), 0 deletions(-)
  
  diff --git a/Documentation/ABI/testing/sysfs-bus-pci 
  b/Documentation/ABI/testing/sysfs-bus-pci
  index ceddcff..41cce8f 100644
  --- a/Documentation/ABI/testing/sysfs-bus-pci
  +++ b/Documentation/ABI/testing/sysfs-bus-pci
  @@ -9,3 +9,36 @@ Description:
  that some devices may have malformatted data.  If the
  underlying VPD has a writable section then the
  corresponding section of this file will be writable.
  +
  +What:  /sys/bus/pci/devices/.../iov/enable
 
 Are you sure this is still the correct location with your change to
 struct device?

Nevermind, this is correct.

But the bigger problem is that userspace doesn't know when these
attributes show up.  So tools like udev and HAL and others can't look
for them as they never get notified, and they don't even know if they
should be looking for them or not.

Is there any way to tie these attributes to the main pci device so
that they get created before the device is announced to the world?
Doing that would solve this issue.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html