Re: [patch V2 22/23] genirq/msi: Handle PCI/MSI allocation fail in core code

Bjorn Helgaas
2021-12-07T21:02:34Z

On Mon, Dec 06, 2021 at 11:27:59PM +0100, Thomas Gleixner wrote:
> Get rid of yet another irqdomain callback and let the core code return the
> already available information of how many descriptors could be allocated.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Tested-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com> # PCI

> ---
> drivers/pci/msi/irqdomain.c | 13 -------------
> include/linux/msi.h | 5 +----
> kernel/irq/msi.c | 29 +++++++++++++++++++++++++----
> 3 files changed, 26 insertions(+), 21 deletions(-)
>
> --- a/drivers/pci/msi/irqdomain.c
> +++ b/drivers/pci/msi/irqdomain.c
> @@ -95,16 +95,6 @@ static int pci_msi_domain_check_cap(stru
> return 0;
> }
>
> -static int pci_msi_domain_handle_error(struct irq_domain *domain,
> - struct msi_desc *desc, int error)
> -{
> - /* Special handling to support __pci_enable_msi_range() */
> - if (pci_msi_desc_is_multi_msi(desc) && error == -ENOSPC)
> - return 1;
> -
> - return error;
> -}
> -
> static void pci_msi_domain_set_desc(msi_alloc_info_t *arg,
> struct msi_desc *desc)
> {
> @@ -115,7 +105,6 @@ static void pci_msi_domain_set_desc(msi_
> static struct msi_domain_ops pci_msi_domain_ops_default = {
> .set_desc = pci_msi_domain_set_desc,
> .msi_check = pci_msi_domain_check_cap,
> - .handle_error = pci_msi_domain_handle_error,
> };
>
> static void pci_msi_domain_update_dom_ops(struct msi_domain_info *info)
> @@ -129,8 +118,6 @@ static void pci_msi_domain_update_dom_op
> ops->set_desc = pci_msi_domain_set_desc;
> if (ops->msi_check == NULL)
> ops->msi_check = pci_msi_domain_check_cap;
> - if (ops->handle_error == NULL)
> - ops->handle_error = pci_msi_domain_handle_error;
> }
> }
>
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -286,7 +286,6 @@ struct msi_domain_info;
> * @msi_check: Callback for verification of the domain/info/dev data
> * @msi_prepare: Prepare the allocation of the interrupts in the domain
> * @set_desc: Set the msi descriptor for an interrupt
> - * @handle_error: Optional error handler if the allocation fails
> * @domain_alloc_irqs: Optional function to override the default allocation
> * function.
> * @domain_free_irqs: Optional function to override the default free
> @@ -295,7 +294,7 @@ struct msi_domain_info;
> * @get_hwirq, @msi_init and @msi_free are callbacks used by the underlying
> * irqdomain.
> *
> - * @msi_check, @msi_prepare, @handle_error and @set_desc are callbacks used by
> + * @msi_check, @msi_prepare and @set_desc are callbacks used by
> * msi_domain_alloc/free_irqs().
> *
> * @domain_alloc_irqs, @domain_free_irqs can be used to override the
> @@ -332,8 +331,6 @@ struct msi_domain_ops {
> msi_alloc_info_t *arg);
> void (*set_desc)(msi_alloc_info_t *arg,
> struct msi_desc *desc);
> - int (*handle_error)(struct irq_domain *domain,
> - struct msi_desc *desc, int error);
> int (*domain_alloc_irqs)(struct irq_domain *domain,
> struct device *dev, int nvec);
> void (*domain_free_irqs)(struct irq_domain *domain,
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -538,6 +538,27 @@ static bool msi_check_reservation_mode(s
> return desc->pci.msi_attrib.is_msix || desc->pci.msi_attrib.can_mask;
> }
>
> +static int msi_handle_pci_fail(struct irq_domain *domain, struct msi_desc *desc,
> + int allocated)
> +{
> + switch(domain->bus_token) {
> + case DOMAIN_BUS_PCI_MSI:
> + case DOMAIN_BUS_VMD_MSI:
> + if (IS_ENABLED(CONFIG_PCI_MSI))
> + break;
> + fallthrough;
> + default:
> + return -ENOSPC;
> + }
> +
> + /* Let a failed PCI multi MSI allocation retry */
> + if (desc->nvec_used > 1)
> + return 1;
> +
> + /* If there was a successful allocation let the caller know */
> + return allocated ? allocated : -ENOSPC;
> +}
> +
> int __msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
> int nvec)
> {
> @@ -546,6 +567,7 @@ int __msi_domain_alloc_irqs(struct irq_d
> struct irq_data *irq_data;
> struct msi_desc *desc;
> msi_alloc_info_t arg = { };
> + int allocated = 0;
> int i, ret, virq;
> bool can_reserve;
>
> @@ -560,16 +582,15 @@ int __msi_domain_alloc_irqs(struct irq_d
> dev_to_node(dev), &arg, false,
> desc->affinity);
> if (virq < 0) {
> - ret = -ENOSPC;
> - if (ops->handle_error)
> - ret = ops->handle_error(domain, desc, ret);
> - return ret;
> + ret = msi_handle_pci_fail(domain, desc, allocated);
> + goto cleanup;
> }
>
> for (i = 0; i < desc->nvec_used; i++) {
> irq_set_msi_desc_off(virq, i, desc);
> irq_debugfs_copy_devname(virq + i, dev);
> }
> + allocated++;
> }
>
> can_reserve = msi_check_reservation_mode(domain, info, dev);
>