Skip to content

Commit bf5e758

Browse files
KAGA-KOKOKAGA-KOKO
committed
genirq/msi: Simplify sysfs handling
The sysfs handling for MSI is a convoluted maze and it is in the way of supporting dynamic expansion of the MSI-X vectors because it only supports a one off bulk population/free of the sysfs entries. Change it to do: 1) Creating an empty sysfs attribute group when msi_device_data is allocated 2) Populate the entries when the MSI descriptor is initialized 3) Free the entries when a MSI descriptor is detached from a Linux interrupt. 4) Provide functions for the legacy non-irqdomain fallback code to do a bulk population/free. This code won't support dynamic expansion. This makes the code simpler and reduces the number of allocations as the empty attribute group can be shared. Signed-off-by: Thomas Gleixner <[email protected]> Tested-by: Michael Kelley <[email protected]> Tested-by: Nishanth Menon <[email protected]> Reviewed-by: Greg Kroah-Hartman <[email protected]> Reviewed-by: Jason Gunthorpe <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent ef3350c commit bf5e758

File tree

2 files changed

+103
-118
lines changed

2 files changed

+103
-118
lines changed

‎include/linux/msi.h

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ struct irq_data;
7171
struct msi_desc;
7272
struct pci_dev;
7373
struct platform_msi_priv_data;
74-
struct attribute_group;
74+
struct device_attribute;
7575

7676
void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
7777
#ifdef CONFIG_GENERIC_MSI_IRQ
@@ -129,6 +129,7 @@ struct pci_msi_desc {
129129
* @dev: Pointer to the device which uses this descriptor
130130
* @msg: The last set MSI message cached for reuse
131131
* @affinity: Optional pointer to a cpu affinity mask for this descriptor
132+
* @sysfs_attr: Pointer to sysfs device attribute
132133
*
133134
* @write_msi_msg: Callback that may be called when the MSI message
134135
* address or data changes
@@ -148,6 +149,9 @@ struct msi_desc {
148149
#ifdef CONFIG_IRQ_MSI_IOMMU
149150
const void *iommu_cookie;
150151
#endif
152+
#ifdef CONFIG_SYSFS
153+
struct device_attribute *sysfs_attrs;
154+
#endif
151155

152156
void (*write_msi_msg)(struct msi_desc *entry, void *data);
153157
void *write_msi_msg_data;
@@ -171,15 +175,13 @@ enum msi_desc_filter {
171175
/**
172176
* msi_device_data - MSI per device data
173177
* @properties: MSI properties which are interesting to drivers
174-
* @attrs: Pointer to the sysfs attribute group
175178
* @platform_data: Platform-MSI specific data
176179
* @list: List of MSI descriptors associated to the device
177180
* @mutex: Mutex protecting the MSI list
178181
* @__next: Cached pointer to the next entry for iterators
179182
*/
180183
struct msi_device_data {
181184
unsigned long properties;
182-
const struct attribute_group **attrs;
183185
struct platform_msi_priv_data *platform_data;
184186
struct list_head list;
185187
struct mutex mutex;
@@ -264,14 +266,6 @@ void __pci_write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
264266
void pci_msi_mask_irq(struct irq_data *data);
265267
void pci_msi_unmask_irq(struct irq_data *data);
266268

267-
#ifdef CONFIG_SYSFS
268-
int msi_device_populate_sysfs(struct device *dev);
269-
void msi_device_destroy_sysfs(struct device *dev);
270-
#else /* CONFIG_SYSFS */
271-
static inline int msi_device_populate_sysfs(struct device *dev) { return 0; }
272-
static inline void msi_device_destroy_sysfs(struct device *dev) { }
273-
#endif /* !CONFIG_SYSFS */
274-
275269
/*
276270
* The arch hooks to setup up msi irqs. Default functions are implemented
277271
* as weak symbols so that they /can/ be overriden by architecture specific
@@ -285,6 +279,13 @@ int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc);
285279
void arch_teardown_msi_irq(unsigned int irq);
286280
int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
287281
void arch_teardown_msi_irqs(struct pci_dev *dev);
282+
#ifdef CONFIG_SYSFS
283+
int msi_device_populate_sysfs(struct device *dev);
284+
void msi_device_destroy_sysfs(struct device *dev);
285+
#else /* CONFIG_SYSFS */
286+
static inline int msi_device_populate_sysfs(struct device *dev) { return 0; }
287+
static inline void msi_device_destroy_sysfs(struct device *dev) { }
288+
#endif /* !CONFIG_SYSFS */
288289
#endif /* CONFIG_PCI_MSI_ARCH_FALLBACKS */
289290

290291
/*

‎kernel/irq/msi.c

Lines changed: 91 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include "internals.h"
2121

22+
static inline int msi_sysfs_create_group(struct device *dev);
2223
#define dev_to_msi_list(dev) (&(dev)->msi.data->list)
2324

2425
/**
@@ -178,6 +179,7 @@ static void msi_device_data_release(struct device *dev, void *res)
178179
int msi_setup_device_data(struct device *dev)
179180
{
180181
struct msi_device_data *md;
182+
int ret;
181183

182184
if (dev->msi.data)
183185
return 0;
@@ -186,6 +188,12 @@ int msi_setup_device_data(struct device *dev)
186188
if (!md)
187189
return -ENOMEM;
188190

191+
ret = msi_sysfs_create_group(dev);
192+
if (ret) {
193+
devres_free(md);
194+
return ret;
195+
}
196+
189197
INIT_LIST_HEAD(&md->list);
190198
mutex_init(&md->mutex);
191199
dev->msi.data = md;
@@ -351,6 +359,20 @@ unsigned int msi_get_virq(struct device *dev, unsigned int index)
351359
EXPORT_SYMBOL_GPL(msi_get_virq);
352360

353361
#ifdef CONFIG_SYSFS
362+
static struct attribute *msi_dev_attrs[] = {
363+
NULL
364+
};
365+
366+
static const struct attribute_group msi_irqs_group = {
367+
.name = "msi_irqs",
368+
.attrs = msi_dev_attrs,
369+
};
370+
371+
static inline int msi_sysfs_create_group(struct device *dev)
372+
{
373+
return devm_device_add_group(dev, &msi_irqs_group);
374+
}
375+
354376
static ssize_t msi_mode_show(struct device *dev, struct device_attribute *attr,
355377
char *buf)
356378
{
@@ -360,97 +382,74 @@ static ssize_t msi_mode_show(struct device *dev, struct device_attribute *attr,
360382
return sysfs_emit(buf, "%s\n", is_msix ? "msix" : "msi");
361383
}
362384

363-
/**
364-
* msi_populate_sysfs - Populate msi_irqs sysfs entries for devices
365-
* @dev: The device(PCI, platform etc) who will get sysfs entries
366-
*/
367-
static const struct attribute_group **msi_populate_sysfs(struct device *dev)
385+
static void msi_sysfs_remove_desc(struct device *dev, struct msi_desc *desc)
368386
{
369-
const struct attribute_group **msi_irq_groups;
370-
struct attribute **msi_attrs, *msi_attr;
371-
struct device_attribute *msi_dev_attr;
372-
struct attribute_group *msi_irq_group;
373-
struct msi_desc *entry;
374-
int ret = -ENOMEM;
375-
int num_msi = 0;
376-
int count = 0;
387+
struct device_attribute *attrs = desc->sysfs_attrs;
377388
int i;
378389

379-
/* Determine how many msi entries we have */
380-
msi_for_each_desc(entry, dev, MSI_DESC_ALL)
381-
num_msi += entry->nvec_used;
382-
if (!num_msi)
383-
return NULL;
390+
if (!attrs)
391+
return;
384392

385-
/* Dynamically create the MSI attributes for the device */
386-
msi_attrs = kcalloc(num_msi + 1, sizeof(void *), GFP_KERNEL);
387-
if (!msi_attrs)
388-
return ERR_PTR(-ENOMEM);
389-
390-
msi_for_each_desc(entry, dev, MSI_DESC_ALL) {
391-
for (i = 0; i < entry->nvec_used; i++) {
392-
msi_dev_attr = kzalloc(sizeof(*msi_dev_attr), GFP_KERNEL);
393-
if (!msi_dev_attr)
394-
goto error_attrs;
395-
msi_attrs[count] = &msi_dev_attr->attr;
396-
397-
sysfs_attr_init(&msi_dev_attr->attr);
398-
msi_dev_attr->attr.name = kasprintf(GFP_KERNEL, "%d",
399-
entry->irq + i);
400-
if (!msi_dev_attr->attr.name)
401-
goto error_attrs;
402-
msi_dev_attr->attr.mode = 0444;
403-
msi_dev_attr->show = msi_mode_show;
404-
++count;
405-
}
393+
desc->sysfs_attrs = NULL;
394+
for (i = 0; i < desc->nvec_used; i++) {
395+
if (attrs[i].show)
396+
sysfs_remove_file_from_group(&dev->kobj, &attrs[i].attr, msi_irqs_group.name);
397+
kfree(attrs[i].attr.name);
406398
}
399+
kfree(attrs);
400+
}
407401

408-
msi_irq_group = kzalloc(sizeof(*msi_irq_group), GFP_KERNEL);
409-
if (!msi_irq_group)
410-
goto error_attrs;
411-
msi_irq_group->name = "msi_irqs";
412-
msi_irq_group->attrs = msi_attrs;
402+
static int msi_sysfs_populate_desc(struct device *dev, struct msi_desc *desc)
403+
{
404+
struct device_attribute *attrs;
405+
int ret, i;
413406

414-
msi_irq_groups = kcalloc(2, sizeof(void *), GFP_KERNEL);
415-
if (!msi_irq_groups)
416-
goto error_irq_group;
417-
msi_irq_groups[0] = msi_irq_group;
407+
attrs = kcalloc(desc->nvec_used, sizeof(*attrs), GFP_KERNEL);
408+
if (!attrs)
409+
return -ENOMEM;
418410

419-
ret = sysfs_create_groups(&dev->kobj, msi_irq_groups);
420-
if (ret)
421-
goto error_irq_groups;
422-
423-
return msi_irq_groups;
424-
425-
error_irq_groups:
426-
kfree(msi_irq_groups);
427-
error_irq_group:
428-
kfree(msi_irq_group);
429-
error_attrs:
430-
count = 0;
431-
msi_attr = msi_attrs[count];
432-
while (msi_attr) {
433-
msi_dev_attr = container_of(msi_attr, struct device_attribute, attr);
434-
kfree(msi_attr->name);
435-
kfree(msi_dev_attr);
436-
++count;
437-
msi_attr = msi_attrs[count];
411+
desc->sysfs_attrs = attrs;
412+
for (i = 0; i < desc->nvec_used; i++) {
413+
sysfs_attr_init(&attrs[i].attr);
414+
attrs[i].attr.name = kasprintf(GFP_KERNEL, "%d", desc->irq + i);
415+
if (!attrs[i].attr.name) {
416+
ret = -ENOMEM;
417+
goto fail;
418+
}
419+
420+
attrs[i].attr.mode = 0444;
421+
attrs[i].show = msi_mode_show;
422+
423+
ret = sysfs_add_file_to_group(&dev->kobj, &attrs[i].attr, msi_irqs_group.name);
424+
if (ret) {
425+
attrs[i].show = NULL;
426+
goto fail;
427+
}
438428
}
439-
kfree(msi_attrs);
440-
return ERR_PTR(ret);
429+
return 0;
430+
431+
fail:
432+
msi_sysfs_remove_desc(dev, desc);
433+
return ret;
441434
}
442435

436+
#ifdef CONFIG_PCI_MSI_ARCH_FALLBACKS
443437
/**
444438
* msi_device_populate_sysfs - Populate msi_irqs sysfs entries for a device
445439
* @dev: The device (PCI, platform etc) which will get sysfs entries
446440
*/
447441
int msi_device_populate_sysfs(struct device *dev)
448442
{
449-
const struct attribute_group **group = msi_populate_sysfs(dev);
443+
struct msi_desc *desc;
444+
int ret;
450445

451-
if (IS_ERR(group))
452-
return PTR_ERR(group);
453-
dev->msi.data->attrs = group;
446+
msi_for_each_desc(desc, dev, MSI_DESC_ASSOCIATED) {
447+
if (desc->sysfs_attrs)
448+
continue;
449+
ret = msi_sysfs_populate_desc(dev, desc);
450+
if (ret)
451+
return ret;
452+
}
454453
return 0;
455454
}
456455

@@ -461,28 +460,17 @@ int msi_device_populate_sysfs(struct device *dev)
461460
*/
462461
void msi_device_destroy_sysfs(struct device *dev)
463462
{
464-
const struct attribute_group **msi_irq_groups = dev->msi.data->attrs;
465-
struct device_attribute *dev_attr;
466-
struct attribute **msi_attrs;
467-
int count = 0;
468-
469-
dev->msi.data->attrs = NULL;
470-
if (!msi_irq_groups)
471-
return;
463+
struct msi_desc *desc;
472464

473-
sysfs_remove_groups(&dev->kobj, msi_irq_groups);
474-
msi_attrs = msi_irq_groups[0]->attrs;
475-
while (msi_attrs[count]) {
476-
dev_attr = container_of(msi_attrs[count], struct device_attribute, attr);
477-
kfree(dev_attr->attr.name);
478-
kfree(dev_attr);
479-
++count;
480-
}
481-
kfree(msi_attrs);
482-
kfree(msi_irq_groups[0]);
483-
kfree(msi_irq_groups);
465+
msi_for_each_desc(desc, dev, MSI_DESC_ALL)
466+
msi_sysfs_remove_desc(dev, desc);
484467
}
485-
#endif
468+
#endif /* CONFIG_PCI_MSI_ARCH_FALLBACK */
469+
#else /* CONFIG_SYSFS */
470+
static inline int msi_sysfs_create_group(struct device *dev) { return 0; }
471+
static inline int msi_sysfs_populate_desc(struct device *dev, struct msi_desc *desc) { return 0; }
472+
static inline void msi_sysfs_remove_desc(struct device *dev, struct msi_desc *desc) { }
473+
#endif /* !CONFIG_SYSFS */
486474

487475
#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
488476
static inline void irq_chip_write_msi_msg(struct irq_data *data,
@@ -914,6 +902,12 @@ int __msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
914902
ret = msi_init_virq(domain, virq + i, vflags);
915903
if (ret)
916904
return ret;
905+
906+
if (info->flags & MSI_FLAG_DEV_SYSFS) {
907+
ret = msi_sysfs_populate_desc(dev, desc);
908+
if (ret)
909+
return ret;
910+
}
917911
}
918912
allocated++;
919913
}
@@ -958,18 +952,7 @@ int msi_domain_alloc_irqs_descs_locked(struct irq_domain *domain, struct device
958952

959953
ret = ops->domain_alloc_irqs(domain, dev, nvec);
960954
if (ret)
961-
goto cleanup;
962-
963-
if (!(info->flags & MSI_FLAG_DEV_SYSFS))
964-
return 0;
965-
966-
ret = msi_device_populate_sysfs(dev);
967-
if (ret)
968-
goto cleanup;
969-
return 0;
970-
971-
cleanup:
972-
msi_domain_free_irqs_descs_locked(domain, dev);
955+
msi_domain_free_irqs_descs_locked(domain, dev);
973956
return ret;
974957
}
975958

@@ -994,6 +977,7 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev, int nve
994977

995978
void __msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
996979
{
980+
struct msi_domain_info *info = domain->host_data;
997981
struct irq_data *irqd;
998982
struct msi_desc *desc;
999983
int i;
@@ -1008,6 +992,8 @@ void __msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
1008992
}
1009993

1010994
irq_domain_free_irqs(desc->irq, desc->nvec_used);
995+
if (info->flags & MSI_FLAG_DEV_SYSFS)
996+
msi_sysfs_remove_desc(dev, desc);
1011997
desc->irq = 0;
1012998
}
1013999
}
@@ -1036,8 +1022,6 @@ void msi_domain_free_irqs_descs_locked(struct irq_domain *domain, struct device
10361022

10371023
lockdep_assert_held(&dev->msi.data->mutex);
10381024

1039-
if (info->flags & MSI_FLAG_DEV_SYSFS)
1040-
msi_device_destroy_sysfs(dev);
10411025
ops->domain_free_irqs(domain, dev);
10421026
msi_domain_free_msi_descs(info, dev);
10431027
}

0 commit comments

Comments
 (0)