lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2375c9f90910200114q641c4368s49695c36ca76f7c9@mail.gmail.com>
Date:	Tue, 20 Oct 2009 16:14:44 +0800
From:	Américo Wang <xiyou.wangcong@...il.com>
To:	Matthew Wilcox <matthew@....cx>
Cc:	Greg Kroah-Hartman <gregkh@...e.de>, linux-kernel@...r.kernel.org,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	linux-pci@...r.kernel.org
Subject: Re: [PATCH 3/3] Expose MSI-X interrupts through a dynamically 
	generated sysfs directory

On Tue, Oct 20, 2009 at 1:50 PM, Matthew Wilcox <matthew@....cx> wrote:
>
> From: Matthew Wilcox <willy@...ux.intel.com>
> Date: Mon, 19 Oct 2009 01:35:41 -0400
> Subject: [PATCH 3/3] Expose MSI-X interrupts through a dynamically generated sysfs directory
>
> Introduce the ability to dynamically generate the attributes (which are
> then added to sysfs).  Add a user in the form of the PCI MSI code, which
> was why I started on this in the first place.
> ---
>  drivers/pci/msi.c     |   77 ++++++++++++++++++++++++++++++++++++++++++++++--
>  fs/sysfs/group.c      |    7 ++++-
>  include/linux/pci.h   |    2 +
>  include/linux/sysfs.h |    9 ++++++
>  4 files changed, 90 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index f9cf317..e0971e6 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -275,6 +275,9 @@ static void free_msi_irqs(struct pci_dev *dev)
>  {
>        struct msi_desc *entry, *tmp;
>
> +       if (dev->msix_dir.name)
> +               sysfs_remove_group(&dev->dev.kobj, &dev->msix_dir);
> +
>        list_for_each_entry(entry, &dev->msi_list, list) {
>                int i, nvec;
>                if (!entry->irq)
> @@ -447,13 +450,12 @@ static void __iomem *msix_map_region(struct pci_dev *dev, unsigned pos,
>  }
>
>  static int msix_setup_entries(struct pci_dev *dev, unsigned pos,
> -                               void __iomem *base, struct msix_entry *entries,
> -                               int nvec)
> +                               void __iomem *base, struct msix_entry *entries)
>  {
>        struct msi_desc *entry;
>        int i;
>
> -       for (i = 0; i < nvec; i++) {
> +       for (i = 0; i < dev->nr_irqs; i++) {
>                entry = alloc_msi_entry(dev);
>                if (!entry) {
>                        if (!i)
> @@ -495,6 +497,64 @@ static void msix_program_entries(struct pci_dev *dev,
>        }
>  }
>
> +struct pci_msix_attribute {
> +       struct device_attribute da;
> +       unsigned irq;
> +       char name[8];   /* current max is 5: "2047\0" */
> +};
> +
> +static ssize_t
> +pci_msix_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +       struct pci_msix_attribute *msix_attr;
> +       msix_attr = container_of(attr, struct pci_msix_attribute, da);
> +       return snprintf(buf, PAGE_SIZE, "%u\n", msix_attr->irq);
> +}
> +
> +#define kobj_to_pci_dev(obj) to_pci_dev(container_of(obj, struct device, kobj))


You define this, but no one uses it?

> +
> +static int msix_populate(struct dentry *dentry, struct attribute_group *grp)
> +{
> +       struct pci_dev *pdev = container_of(grp, struct pci_dev, msix_dir);
> +       unsigned i, nr_irqs = pdev->nr_irqs;
> +       struct pci_msix_attribute *attr;
> +       struct attribute **array;
> +       struct msi_desc *desc;
> +
> +       array = kmalloc((nr_irqs + 1) * sizeof(void *), GFP_KERNEL);
> +       if (!array)
> +               return -ENOMEM;
> +       attr = kmalloc(nr_irqs * sizeof(*attr), GFP_KERNEL);
> +       if (!attr)
> +               return -ENOMEM;

Here leaks memory allocated above.

> +
> +       for (i = 0; i < nr_irqs; i++)
> +               array[i] = &attr[i].da.attr;
> +       array[i] = NULL;
> +       grp->attrs = array;
> +
> +       list_for_each_entry(desc, &pdev->msi_list, list) {
> +               attr->irq = desc->irq;
> +               snprintf(attr->name, sizeof(attr->name), "%u",
> +                                               desc->msi_attrib.entry_nr);
> +               attr->da.attr.name = (const char *)&attr->name;
> +               attr->da.attr.mode = 0444;
> +               attr->da.show = pci_msix_show;
> +               attr++;
> +       }
> +
> +       return sysfs_populate_group(dentry, grp);
> +}
> +
> +static void msix_depopulate(struct dentry *dentry, struct attribute_group *grp)
> +{
> +       sysfs_depopulate_group(dentry, grp);
> +
> +       kfree(grp->attrs[0]);
> +       kfree(grp->attrs);
> +       grp->attrs = NULL;
> +}
> +
>  /**
>  * msix_capability_init - configure device's MSI-X capability
>  * @dev: pointer to the pci_dev data structure of MSI-X device function
> @@ -524,10 +584,19 @@ static int msix_capability_init(struct pci_dev *dev,
>        if (!base)
>                return -ENOMEM;
>
> -       ret = msix_setup_entries(dev, pos, base, entries, nvec);
> +       dev->nr_irqs = nvec;
> +
> +       dev->msix_dir.name = "irqs";
> +       dev->msix_dir.populate = msix_populate;
> +       dev->msix_dir.depopulate = msix_depopulate;
> +       ret = sysfs_create_group(&dev->dev.kobj, &dev->msix_dir);
>        if (ret)
>                return ret;
>
> +       ret = msix_setup_entries(dev, pos, base, entries);
> +       if (ret)
> +               goto error;
> +
>        ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX);
>        if (ret)
>                goto error;
> diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
> index 37ac584..c97139f 100644
> --- a/fs/sysfs/group.c
> +++ b/fs/sysfs/group.c
> @@ -102,6 +102,8 @@ static
>  int group_populate(struct dentry *dentry, struct sysfs_dirent *sd)
>  {
>        struct attribute_group *grp = sd->s_dir.data;
> +       if (grp->populate)
> +               return grp->populate(dentry, grp);
>        return sysfs_populate_group(dentry, grp);
>  }
>
> @@ -109,7 +111,10 @@ static
>  void group_depopulate(struct dentry *dentry, struct sysfs_dirent *sd)
>  {
>        struct attribute_group *grp = sd->s_dir.data;
> -       sysfs_depopulate_group(dentry, grp);
> +       if (grp->depopulate)
> +               grp->depopulate(dentry, grp);
> +       else
> +               sysfs_depopulate_group(dentry, grp);
>  }
>
>  static void remove_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index f5c7cd3..47c7fc6 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -291,6 +291,8 @@ 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 attribute_group msix_dir;
> +       unsigned short nr_irqs;
>  #endif
>        struct pci_vpd *vpd;
>  #ifdef CONFIG_PCI_IOV
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index 9d68fed..cf9d200 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -19,6 +19,7 @@
>
>  struct kobject;
>  struct module;
> +struct dentry;
>
>  /* FIXME
>  * The *owner field is no longer used.
> @@ -36,6 +37,10 @@ struct attribute_group {
>        mode_t                  (*is_visible)(struct kobject *,
>                                              struct attribute *, int);
>        struct attribute        **attrs;
> +       int                     (*populate)(struct dentry *,
> +                                               struct attribute_group *);
> +       void                    (*depopulate)(struct dentry *,
> +                                               struct attribute_group *);
>  };
>
>
> @@ -115,6 +120,10 @@ int sysfs_update_group(struct kobject *kobj,
>                       const struct attribute_group *grp);
>  void sysfs_remove_group(struct kobject *kobj,
>                        const struct attribute_group *grp);
> +int sysfs_populate_group(struct dentry *dentry,
> +                       const struct attribute_group *grp);
> +void sysfs_depopulate_group(struct dentry *dentry,
> +                       const struct attribute_group *grp);
>  int sysfs_add_file_to_group(struct kobject *kobj,
>                        const struct attribute *attr, const char *group);
>  void sysfs_remove_file_from_group(struct kobject *kobj,
> --
> 1.6.3.3
>
> --
> Matthew Wilcox                          Intel Open Source Technology Centre
> "Bill, look, we understand that you're interested in selling us this
> operating system, but compare it to ours.  We can't possibly take such
> a retrograde step."
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ