On Sun, 9 Mar 2025 09:41:44 +0100 (CET) Thomas Gleixner <[email protected]> wrote:
> Provide a lock guard for MSI descriptor locking and update the core code > accordingly. > > No functional change intended. > > Signed-off-by: Thomas Gleixner <[email protected]> One case in here made me go back to the helpful docs Dan wrote after early confusion on how to use this stuff. We might want to consider updating as you've found a case that I at least didn't consider when reviewing those. +CC Dan. Anyhow, this seems fine to me. Reviewed-by: Jonathan Cameron <[email protected]> > > --- a/kernel/irq/msi.c > +++ b/kernel/irq/msi.c > @@ -443,7 +443,6 @@ EXPORT_SYMBOL_GPL(msi_next_desc); > > @@ -1037,25 +1032,23 @@ bool msi_create_device_irq_domain(struct > if (msi_setup_device_data(dev)) Hmm. We might want to make the docs in cleanup.h more nuanced. They specifically say to not mix goto and auto cleanup, but in the case of scoped_guard() unlikely almost any other case it should be fine. > goto free_fwnode; > > - msi_lock_descs(dev); > - > - if (WARN_ON_ONCE(msi_get_device_domain(dev, domid))) > - goto fail; > - > - if (!pops->init_dev_msi_info(dev, parent, parent, &bundle->info)) > - goto fail; > - > - domain = __msi_create_irq_domain(fwnode, &bundle->info, > IRQ_DOMAIN_FLAG_MSI_DEVICE, parent); > - if (!domain) > - goto fail; > - > - domain->dev = dev; > - dev->msi.data->__domains[domid].domain = domain; > - msi_unlock_descs(dev); > - return true; > + scoped_guard(msi_descs_lock, dev) { > + if (WARN_ON_ONCE(msi_get_device_domain(dev, domid))) > + goto free_fwnode; > + > + if (!pops->init_dev_msi_info(dev, parent, parent, > &bundle->info)) > + goto free_fwnode; > + > + domain = __msi_create_irq_domain(fwnode, &bundle->info, > IRQ_DOMAIN_FLAG_MSI_DEVICE, > + parent); > + if (!domain) > + goto free_fwnode; > + > + domain->dev = dev; > + dev->msi.data->__domains[domid].domain = domain; > + return true; > + } > > -fail: > - msi_unlock_descs(dev); > free_fwnode: > irq_domain_free_fwnode(fwnalloced); > free_bundle:
