On Thu, Dec 19, 2013 at 12:30:17PM -0800, Greg Kroah-Hartman wrote:
> From: Greg Kroah-Hartman <gre...@linuxfoundation.org>
>
> The PCI MSI sysfs code is a mess with kobjects for things that don't
> really need to be kobjects. This patch creates attributes dynamically
> for the MSI interrupts instead of using kobjects.
>
> Note, this removes a directory from the current MSI interrupt sysfs
> code:
>
> old MSI kobjects:
> pci_device
> └── msi_irqs
> └── 40
> └── mode
>
> new MSI attributes:
> pci_device
> └── msi_irqs
> └── 40
>
> As there was only one file "mode" with the kobject model, the interrupt
> number is now a file that returns the "mode" of the interrupt (msi vs.
> msix).
>
> Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Thanks, I added this to pci/msi for v3.14.
> ---
> v3: attribute creation properly tagged for lockdep.
> error handling if creation of attributes fails.
>
> v2: add documentation for sysfs attributes
>
> Documentation/ABI/testing/sysfs-bus-pci | 11 --
> drivers/pci/msi.c | 166
> +++++++++++++++++---------------
> include/linux/pci.h | 2
> 3 files changed, 96 insertions(+), 83 deletions(-)
>
> --- a/Documentation/ABI/testing/sysfs-bus-pci
> +++ b/Documentation/ABI/testing/sysfs-bus-pci
> @@ -70,18 +70,15 @@ Date: September, 2011
> Contact: Neil Horman <nhor...@tuxdriver.com>
> Description:
> The /sys/devices/.../msi_irqs directory contains a variable set
> - of sub-directories, with each sub-directory being named after a
> - corresponding msi irq vector allocated to that device. Each
> - numbered sub-directory N contains attributes of that irq.
> - Note that this directory is not created for device drivers which
> - do not support msi irqs
> + of files, with each file being named after a corresponding msi
> + irq vector allocated to that device.
>
> -What: /sys/bus/pci/devices/.../msi_irqs/<N>/mode
> +What: /sys/bus/pci/devices/.../msi_irqs/<N>
> Date: September 2011
> Contact: Neil Horman <nhor...@tuxdriver.com>
> Description:
> This attribute indicates the mode that the irq vector named by
> - the parent directory is in (msi vs. msix)
> + the file is in (msi vs. msix)
>
> What: /sys/bus/pci/devices/.../remove
> Date: January 2009
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -363,6 +363,9 @@ void write_msi_msg(unsigned int irq, str
> static void free_msi_irqs(struct pci_dev *dev)
> {
> struct msi_desc *entry, *tmp;
> + struct attribute **msi_attrs;
> + struct device_attribute *dev_attr;
> + int count = 0;
>
> list_for_each_entry(entry, &dev->msi_list, list) {
> int i, nvec;
> @@ -398,6 +401,22 @@ static void free_msi_irqs(struct pci_dev
> list_del(&entry->list);
> kfree(entry);
> }
> +
> + if (dev->msi_irq_groups) {
> + sysfs_remove_groups(&dev->dev.kobj, dev->msi_irq_groups);
> + msi_attrs = dev->msi_irq_groups[0]->attrs;
> + list_for_each_entry(entry, &dev->msi_list, list) {
> + dev_attr = container_of(msi_attrs[count],
> + struct device_attribute, attr);
> + kfree(dev_attr->attr.name);
> + kfree(dev_attr);
> + ++count;
> + }
> + kfree(msi_attrs);
> + kfree(dev->msi_irq_groups[0]);
> + kfree(dev->msi_irq_groups);
> + dev->msi_irq_groups = NULL;
> + }
> }
>
> static struct msi_desc *alloc_msi_entry(struct pci_dev *dev)
> @@ -471,94 +490,95 @@ void pci_restore_msi_state(struct pci_de
> }
> EXPORT_SYMBOL_GPL(pci_restore_msi_state);
>
> -
> -#define to_msi_attr(obj) container_of(obj, struct msi_attribute, attr)
> -#define to_msi_desc(obj) container_of(obj, struct msi_desc, kobj)
> -
> -struct msi_attribute {
> - struct attribute attr;
> - ssize_t (*show)(struct msi_desc *entry, struct msi_attribute *attr,
> - char *buf);
> - ssize_t (*store)(struct msi_desc *entry, struct msi_attribute *attr,
> - const char *buf, size_t count);
> -};
> -
> -static ssize_t show_msi_mode(struct msi_desc *entry, struct msi_attribute
> *atr,
> +static ssize_t msi_mode_show(struct device *dev, struct device_attribute
> *attr,
> char *buf)
> {
> - return sprintf(buf, "%s\n", entry->msi_attrib.is_msix ? "msix" : "msi");
> -}
> -
> -static ssize_t msi_irq_attr_show(struct kobject *kobj,
> - struct attribute *attr, char *buf)
> -{
> - struct msi_attribute *attribute = to_msi_attr(attr);
> - struct msi_desc *entry = to_msi_desc(kobj);
> -
> - if (!attribute->show)
> - return -EIO;
> -
> - return attribute->show(entry, attribute, buf);
> -}
> -
> -static const struct sysfs_ops msi_irq_sysfs_ops = {
> - .show = msi_irq_attr_show,
> -};
> -
> -static struct msi_attribute mode_attribute =
> - __ATTR(mode, S_IRUGO, show_msi_mode, NULL);
> -
> -
> -static struct attribute *msi_irq_default_attrs[] = {
> - &mode_attribute.attr,
> - NULL
> -};
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct msi_desc *entry;
> + unsigned long irq;
> + int retval;
>
> -static void msi_kobj_release(struct kobject *kobj)
> -{
> - struct msi_desc *entry = to_msi_desc(kobj);
> + retval = kstrtoul(attr->attr.name, 10, &irq);
> + if (retval)
> + return retval;
>
> - pci_dev_put(entry->dev);
> + list_for_each_entry(entry, &pdev->msi_list, list) {
> + if (entry->irq == irq) {
> + return sprintf(buf, "%s\n",
> + entry->msi_attrib.is_msix ? "msix" :
> "msi");
> + }
> + }
> + return -ENODEV;
> }
>
> -static struct kobj_type msi_irq_ktype = {
> - .release = msi_kobj_release,
> - .sysfs_ops = &msi_irq_sysfs_ops,
> - .default_attrs = msi_irq_default_attrs,
> -};
> -
> static int populate_msi_sysfs(struct pci_dev *pdev)
> {
> + struct attribute **msi_attrs;
> + struct attribute *msi_attr;
> + struct device_attribute *msi_dev_attr;
> + struct attribute_group *msi_irq_group;
> + const struct attribute_group **msi_irq_groups;
> struct msi_desc *entry;
> - struct kobject *kobj;
> - int ret;
> + int ret = -ENOMEM;
> + int num_msi = 0;
> int count = 0;
>
> - pdev->msi_kset = kset_create_and_add("msi_irqs", NULL, &pdev->dev.kobj);
> - if (!pdev->msi_kset)
> - return -ENOMEM;
> + /* Determine how many msi entries we have */
> + list_for_each_entry(entry, &pdev->msi_list, list) {
> + ++num_msi;
> + }
> + if (!num_msi)
> + return 0;
>
> + /* Dynamically create the MSI attributes for the PCI device */
> + msi_attrs = kzalloc(sizeof(void *) * (num_msi + 1), GFP_KERNEL);
> + if (!msi_attrs)
> + return -ENOMEM;
> list_for_each_entry(entry, &pdev->msi_list, list) {
> - kobj = &entry->kobj;
> - kobj->kset = pdev->msi_kset;
> - pci_dev_get(pdev);
> - ret = kobject_init_and_add(kobj, &msi_irq_ktype, NULL,
> - "%u", entry->irq);
> - if (ret)
> - goto out_unroll;
> + char *name = kmalloc(20, GFP_KERNEL);
> + msi_dev_attr = kzalloc(sizeof(*msi_dev_attr), GFP_KERNEL);
> + if (!msi_dev_attr)
> + goto error_attrs;
> + sprintf(name, "%d", entry->irq);
> + sysfs_attr_init(&msi_dev_attr->attr);
> + msi_dev_attr->attr.name = name;
> + msi_dev_attr->attr.mode = S_IRUGO;
> + msi_dev_attr->show = msi_mode_show;
> + msi_attrs[count] = &msi_dev_attr->attr;
> + ++count;
> + }
> +
> + msi_irq_group = kzalloc(sizeof(*msi_irq_group), GFP_KERNEL);
> + if (!msi_irq_group)
> + goto error_attrs;
> + msi_irq_group->name = "msi_irqs";
> + msi_irq_group->attrs = msi_attrs;
> +
> + msi_irq_groups = kzalloc(sizeof(void *) * 2, GFP_KERNEL);
> + if (!msi_irq_groups)
> + goto error_irq_group;
> + msi_irq_groups[0] = msi_irq_group;
>
> - count++;
> - }
> + ret = sysfs_create_groups(&pdev->dev.kobj, msi_irq_groups);
> + if (ret)
> + goto error_irq_groups;
> + pdev->msi_irq_groups = msi_irq_groups;
>
> return 0;
>
> -out_unroll:
> - list_for_each_entry(entry, &pdev->msi_list, list) {
> - if (!count)
> - break;
> - kobject_del(&entry->kobj);
> - kobject_put(&entry->kobj);
> - count--;
> +error_irq_groups:
> + kfree(msi_irq_groups);
> +error_irq_group:
> + kfree(msi_irq_group);
> +error_attrs:
> + count = 0;
> + msi_attr = msi_attrs[count];
> + while (msi_attr) {
> + msi_dev_attr = container_of(msi_attr, struct device_attribute,
> attr);
> + kfree(msi_attr->name);
> + kfree(msi_dev_attr);
> + ++count;
> + msi_attr = msi_attrs[count];
> }
> return ret;
> }
> @@ -925,8 +945,6 @@ void pci_disable_msi(struct pci_dev *dev
>
> pci_msi_shutdown(dev);
> free_msi_irqs(dev);
> - kset_unregister(dev->msi_kset);
> - dev->msi_kset = NULL;
> }
> EXPORT_SYMBOL(pci_disable_msi);
>
> @@ -1023,8 +1041,6 @@ void pci_disable_msix(struct pci_dev *de
>
> pci_msix_shutdown(dev);
> free_msi_irqs(dev);
> - kset_unregister(dev->msi_kset);
> - dev->msi_kset = NULL;
> }
> EXPORT_SYMBOL(pci_disable_msix);
>
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -351,7 +351,7 @@ struct pci_dev {
> struct bin_attribute *res_attr_wc[DEVICE_COUNT_RESOURCE]; /* sysfs file
> for WC mapping of resources */
> #ifdef CONFIG_PCI_MSI
> struct list_head msi_list;
> - struct kset *msi_kset;
> + const struct attribute_group **msi_irq_groups;
> #endif
> struct pci_vpd *vpd;
> #ifdef CONFIG_PCI_ATS
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
