Re: [RFC] PCI: enable and disable sriov support via sysfs at per device level

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/02/2012 11:10 PM, Greg Kroah-Hartman wrote:
On Tue, Oct 02, 2012 at 05:06:34PM -0400, Don Dutile wrote:
On 10/02/2012 04:39 PM, Greg Kroah-Hartman wrote:
On Tue, Oct 02, 2012 at 04:23:40PM -0400, Don Dutile wrote:
Greg: Why not use the above functions?

What "above functions"?  You make a lot of calls in the code :)

You are creating an attribute group, and then manually walking through
it to create/destroy the file?  That's not good, use the in-kernel
functions to do that automatically for you.

Also, you need to do this _before_ the device shows up to userspace,
otherwise you have a race that you just lost with udev and the like.

thanks,

greg k-h

So, why is it ok to create the 'reset' file after the device shows up?

It's not, that should be fixed, patches are always welcome.

Like reset, SRIOV is optional.  The sysfs files are designed to only
be used by userspace utils to enable/disable VFs after the system is up.

Or should the design allow a udev rule to be created that if one
of these new sriov files exists, then perform a set of read-max-vf&
enable n-vf ops ?

Yes it should.  Never create sysfs files after the device is present in
the system, otherwise userspace never knows about it.

thanks,

greg k-h

ok, thanks for feedback and bit of education ....
still looking for more on this 'visible' tagging though.

so, I guess Bjorn ought to add 'cleaning up pci-sysfs' to
his PCI to-do list...

maybe once i figure out the proper sriov sysfs code, I
can duplicate that effort in the reset,vga & pm-capabilities
section.

- Don
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux