| Message ID | 20211127000918.664542907@linutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | genirq/msi, PCI/MSI: Support for dynamic MSI-X vector expansion - Part 4 | expand |
On Sat, 27 Nov 2021 01:24:34 +0000, Thomas Gleixner <tglx@linutronix.de> wrote: > > Convert the MSI descriptor related functions to ranges and fixup the call > sites. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > drivers/base/platform-msi.c | 3 ++- > include/linux/msi.h | 7 ++++--- > kernel/irq/msi.c | 38 +++++++++++++++++++------------------- > 3 files changed, 25 insertions(+), 23 deletions(-) This particular patch breaks one of my test boxes when allocating the MSIs for the first SMMUv3 it encounters: [ 14.700206] arm-smmu-v3 arm-smmu-v3.0.auto: option mask 0x0 [ 14.705848] arm-smmu-v3 arm-smmu-v3.0.auto: ias 48-bit, oas 48-bit (features 0x00041fff) [ 14.716184] arm-smmu-v3 arm-smmu-v3.0.auto: allocated 65536 entries for cmdq [ 14.723285] arm-smmu-v3 arm-smmu-v3.0.auto: allocated 128 entries for evtq [ 14.730170] arm-smmu-v3 arm-smmu-v3.0.auto: allocated 256 entries for priq [ 41.282305] watchdog: BUG: soft lockup - CPU#43 stuck for 26s! [swapper/0:1] [ 41.289383] Modules linked in: [ 41.292430] irq event stamp: 3906684 [ 41.295994] hardirqs last enabled at (3906683): [<ffffb73f677c54d8>] ___slab_alloc+0x7c8/0x8c0 [ 41.304698] hardirqs last disabled at (3906684): [<ffffb73f6806d478>] el1_interrupt+0x38/0xb0 [ 41.313220] softirqs last enabled at (3798058): [<ffffb73f6746099c>] __do_softirq+0x40c/0x58c [ 41.321825] softirqs last disabled at (3798053): [<ffffb73f674ee610>] __irq_exit_rcu+0x120/0x160 [ 41.330607] CPU: 43 PID: 1 Comm: swapper/0 Not tainted 5.16.0-rc2-00078-g76af42494903 #41 [ 41.338775] Hardware name: WIWYNN Mt.Jade Server System B81.030Z1.0007/Mt.Jade Motherboard, BIOS 1.3.20210110 2021/01/10 [ 41.349634] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 41.356585] pc : lock_is_held_type+0x124/0x20c [ 41.361019] lr : lock_is_held_type+0xe8/0x20c [ 41.365365] sp : ffff80001051b840 [ 41.368669] pmr_save: 000000e0 [ 41.371712] x29: ffff80001051b840 x28: ffff000034d80000 x27: ffff07ff87606600 [ 41.378838] x26: 00000000000000e0 x25: 00000000ffffffff x24: ffffb73f68733d00 [ 41.385964] x23: 0000000000000028 x22: ffff07ff87606fb8 x21: ffffb73f68bffc38 [ 41.393090] x20: ffff07ff87606fe0 x19: 0000000000000002 x18: 0000000000000014 [ 41.400217] x17: 0000000076a13aac x16: 00000000486de301 x15: 00000000ed5ff5e1 [ 41.407342] x14: 00000000fda7b077 x13: 0000000000000006 x12: 00000000b82b73dd [ 41.414468] x11: ffff07ff87606fb8 x10: ffffb73f6952d000 x9 : ffffb73f675a3750 [ 41.421594] x8 : 0000000000000000 x7 : ffffb73f68e16000 x6 : ffffb73f677c6d64 [ 41.428720] x5 : 0000000000000000 x4 : 0000000000000001 x3 : 000000000000ffff [ 41.435846] x2 : ffff50ff17454000 x1 : 0000000000000000 x0 : 0000000000000000 [ 41.442972] Call trace: [ 41.445407] lock_is_held_type+0x124/0x20c [ 41.449494] rcu_read_lock_sched_held+0x68/0xac [ 41.454018] trace_lock_acquire+0x78/0x1c0 [ 41.458107] lock_acquire+0x40/0x90 [ 41.461585] fs_reclaim_acquire+0x90/0x114 [ 41.465676] kmem_cache_alloc_trace+0x80/0x300 [ 41.470110] msi_add_simple_msi_descs+0x70/0x150 [ 41.474718] msi_domain_alloc_irqs_descs_locked+0x90/0xfc [ 41.480106] msi_domain_alloc_irqs+0x58/0xa0 [ 41.484364] platform_msi_domain_alloc_irqs+0x5c/0xa0 [ 41.489410] arm_smmu_device_probe+0xfc0/0x1230 [ 41.493936] platform_probe+0x74/0xe4 [ 41.497590] really_probe+0xc4/0x470 [ 41.501156] __driver_probe_device+0x11c/0x190 [ 41.505589] driver_probe_device+0x48/0x110 [ 41.509761] __driver_attach+0xe0/0x200 [ 41.513585] bus_for_each_dev+0x7c/0xe0 [ 41.517412] driver_attach+0x30/0x3c [ 41.520976] bus_add_driver+0x150/0x230 [ 41.524801] driver_register+0x84/0x140 [ 41.528626] __platform_driver_register+0x34/0x40 [ 41.533319] arm_smmu_driver_init+0x2c/0x38 [ 41.537496] do_one_initcall+0x80/0x3d0 [ 41.541321] kernel_init_freeable+0x318/0x3a0 [ 41.545672] kernel_init+0x30/0x14c [ 41.549151] ret_from_fork+0x10/0x20 The issue seems to be originating in the previous patch, where the following line was added: + struct msi_range range = { .first = 0, .last = UINT_MAX, .ndesc = nvec, }; In that context, only 'ndesc' was used, and that was fine. However, in the current patch, ndesc use is removed, only first/last are considered, and UINT_MAX is... a lot of MSIs. This fixes it: diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c index bef5b74a7268..a520bfd94a56 100644 --- a/kernel/irq/msi.c +++ b/kernel/irq/msi.c @@ -975,7 +975,7 @@ int msi_domain_alloc_irqs_descs_locked(struct irq_domain *domain, struct device */ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev, int nvec) { - struct msi_range range = { .first = 0, .last = UINT_MAX, .ndesc = nvec, }; + struct msi_range range = { .first = 0, .last = nvec - 1, .ndesc = nvec, }; int ret; msi_lock_descs(dev); However, it'd be good to clarify the use of range->ndesc. [...] > --- a/kernel/irq/msi.c > +++ b/kernel/irq/msi.c > @@ -101,19 +101,19 @@ int msi_add_msi_desc(struct device *dev, > * > * Return: 0 on success or an appropriate failure code. > */ > -static int msi_add_simple_msi_descs(struct device *dev, unsigned int index, unsigned int ndesc) > +static int msi_add_simple_msi_descs(struct device *dev, struct msi_range *range) nit: most of the functions changed in this patch need to have their documentation tidied up. Thanks, M.
On Sun, Nov 28 2021 at 15:57, Marc Zyngier wrote: > On Sat, 27 Nov 2021 01:24:34 +0000, > Thomas Gleixner <tglx@linutronix.de> wrote: > > The issue seems to be originating in the previous patch, where the > following line was added: > > + struct msi_range range = { .first = 0, .last = UINT_MAX, .ndesc = nvec, }; > > In that context, only 'ndesc' was used, and that was fine. > > However, in the current patch, ndesc use is removed, only first/last > are considered, and UINT_MAX is... a lot of MSIs. > > This fixes it: > > diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c > index bef5b74a7268..a520bfd94a56 100644 > --- a/kernel/irq/msi.c > +++ b/kernel/irq/msi.c > @@ -975,7 +975,7 @@ int msi_domain_alloc_irqs_descs_locked(struct irq_domain *domain, struct device > */ > int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev, int nvec) > { > - struct msi_range range = { .first = 0, .last = UINT_MAX, .ndesc = nvec, }; > + struct msi_range range = { .first = 0, .last = nvec - 1, .ndesc = nvec, }; > int ret; > > msi_lock_descs(dev); > > However, it'd be good to clarify the use of range->ndesc. Hrm. The stupid search should terminated nevertheless. Let me stare at it again. >> -static int msi_add_simple_msi_descs(struct device *dev, unsigned int index, unsigned int ndesc) >> +static int msi_add_simple_msi_descs(struct device *dev, struct msi_range *range) > > nit: most of the functions changed in this patch need to have their > documentation tidied up. Duh, yes. Thanks, tglx
On Sun, Nov 28 2021 at 20:17, Thomas Gleixner wrote: > On Sun, Nov 28 2021 at 15:57, Marc Zyngier wrote: > Hrm. The stupid search should terminated nevertheless. Let me stare at > it again. Found it. Just my inability to read xarray documentation. Thanks, tglx
--- a/drivers/base/platform-msi.c +++ b/drivers/base/platform-msi.c @@ -320,11 +320,12 @@ struct irq_domain * void platform_msi_device_domain_free(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs) { + struct msi_range range = { .first = virq, .last = virq + nr_irqs - 1, }; struct platform_msi_priv_data *data = domain->host_data; msi_lock_descs(data->dev); irq_domain_free_irqs_common(domain, virq, nr_irqs); - msi_free_msi_descs_range(data->dev, MSI_DESC_ALL, virq, nr_irqs); + msi_free_msi_descs_range(data->dev, MSI_DESC_ALL, &range); msi_unlock_descs(data->dev); } --- a/include/linux/msi.h +++ b/include/linux/msi.h @@ -321,8 +321,7 @@ static inline void pci_write_msi_msg(uns #endif /* CONFIG_PCI_MSI */ int msi_add_msi_desc(struct device *dev, struct msi_desc *init_desc); -void msi_free_msi_descs_range(struct device *dev, enum msi_desc_filter filter, - unsigned int base_index, unsigned int ndesc); +void msi_free_msi_descs_range(struct device *dev, enum msi_desc_filter filter, struct msi_range *range); /** * msi_free_msi_descs - Free MSI descriptors of a device @@ -330,7 +329,9 @@ void msi_free_msi_descs_range(struct dev */ static inline void msi_free_msi_descs(struct device *dev) { - msi_free_msi_descs_range(dev, MSI_DESC_ALL, 0, UINT_MAX); + struct msi_range range = { .first = 0, .last = UINT_MAX, }; + + msi_free_msi_descs_range(dev, MSI_DESC_ALL, &range); } void __pci_read_msi_msg(struct msi_desc *entry, struct msi_msg *msg); --- a/kernel/irq/msi.c +++ b/kernel/irq/msi.c @@ -101,19 +101,19 @@ int msi_add_msi_desc(struct device *dev, * * Return: 0 on success or an appropriate failure code. */ -static int msi_add_simple_msi_descs(struct device *dev, unsigned int index, unsigned int ndesc) +static int msi_add_simple_msi_descs(struct device *dev, struct msi_range *range) { struct msi_desc *desc; - unsigned long i; + unsigned long idx; int ret; lockdep_assert_held(&dev->msi.data->mutex); - for (i = 0; i < ndesc; i++) { + for (idx = range->first; idx <= range->last; idx++) { desc = msi_alloc_desc(dev, 1, NULL); if (!desc) goto fail_mem; - ret = msi_insert_desc(dev->msi.data, desc, index + i); + ret = msi_insert_desc(dev->msi.data, desc, idx); if (ret) goto fail; } @@ -122,7 +122,7 @@ static int msi_add_simple_msi_descs(stru fail_mem: ret = -ENOMEM; fail: - msi_free_msi_descs_range(dev, MSI_DESC_NOTASSOCIATED, index, ndesc); + msi_free_msi_descs_range(dev, MSI_DESC_NOTASSOCIATED, range); return ret; } @@ -148,14 +148,14 @@ static bool msi_desc_match(struct msi_de * @ndesc: Number of descriptors to free */ void msi_free_msi_descs_range(struct device *dev, enum msi_desc_filter filter, - unsigned int base_index, unsigned int ndesc) + struct msi_range *range) { struct msi_desc *desc; unsigned long idx; lockdep_assert_held(&dev->msi.data->mutex); - xa_for_each_range(&dev->msi.data->store, idx, desc, base_index, base_index + ndesc - 1) { + xa_for_each_range(&dev->msi.data->store, idx, desc, range->first, range->last) { if (msi_desc_match(desc, filter)) { xa_erase(&dev->msi.data->store, idx); msi_free_desc(desc); @@ -746,17 +746,18 @@ int msi_domain_prepare_irqs(struct irq_d int msi_domain_populate_irqs(struct irq_domain *domain, struct device *dev, int virq_base, int nvec, msi_alloc_info_t *arg) { + struct msi_range range = { .first = virq_base, .last = virq_base + nvec - 1 }; struct msi_domain_info *info = domain->host_data; struct msi_domain_ops *ops = info->ops; struct msi_desc *desc; int ret, virq; msi_lock_descs(dev); - ret = msi_add_simple_msi_descs(dev, virq_base, nvec); + ret = msi_add_simple_msi_descs(dev, &range); if (ret) goto unlock; - for (virq = virq_base; virq < virq_base + nvec; virq++) { + for (virq = range.first; virq <= range.last; virq++) { desc = xa_load(&dev->msi.data->store, virq); desc->irq = virq; @@ -773,7 +774,7 @@ int msi_domain_populate_irqs(struct irq_ fail: for (--virq; virq >= virq_base; virq--) irq_domain_free_irqs_common(domain, virq, 1); - msi_free_msi_descs_range(dev, MSI_DESC_ALL, virq_base, nvec); + msi_free_msi_descs_range(dev, MSI_DESC_ALL, &range); unlock: msi_unlock_descs(dev); return ret; @@ -932,14 +933,13 @@ int __msi_domain_alloc_irqs(struct irq_d return 0; } -static int msi_domain_add_simple_msi_descs(struct msi_domain_info *info, - struct device *dev, - unsigned int num_descs) +static int msi_domain_add_simple_msi_descs(struct msi_domain_info *info, struct device *dev, + struct msi_range *range) { if (!(info->flags & MSI_FLAG_ALLOC_SIMPLE_MSI_DESCS)) return 0; - return msi_add_simple_msi_descs(dev, 0, num_descs); + return msi_add_simple_msi_descs(dev, range); } /** @@ -964,7 +964,7 @@ int msi_domain_alloc_irqs_descs_locked(s lockdep_assert_held(&dev->msi.data->mutex); - ret = msi_domain_add_simple_msi_descs(info, dev, range->ndesc); + ret = msi_domain_add_simple_msi_descs(info, dev, range); if (ret) return ret; @@ -1017,11 +1017,11 @@ void __msi_domain_free_irqs(struct irq_d } } -static void msi_domain_free_msi_descs(struct msi_domain_info *info, - struct device *dev) +static void msi_domain_free_msi_descs(struct msi_domain_info *info, struct device *dev, + struct msi_range *range) { if (info->flags & MSI_FLAG_FREE_MSI_DESCS) - msi_free_msi_descs(dev); + msi_free_msi_descs_range(dev, MSI_DESC_ALL, range); } /** @@ -1043,7 +1043,7 @@ void msi_domain_free_irqs_descs_locked(s lockdep_assert_held(&dev->msi.data->mutex); ops->domain_free_irqs(domain, dev, range); - msi_domain_free_msi_descs(info, dev); + msi_domain_free_msi_descs(info, dev, range); } /**
Convert the MSI descriptor related functions to ranges and fixup the call sites. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- drivers/base/platform-msi.c | 3 ++- include/linux/msi.h | 7 ++++--- kernel/irq/msi.c | 38 +++++++++++++++++++------------------- 3 files changed, 25 insertions(+), 23 deletions(-)