linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Garry <john.garry@huawei.com>
To: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Zhou Wang <wangzhou1@hisilicon.com>,
	<linux-kernel@vger.kernel.org>
Subject: Re: PCI MSI issue with reinserting a driver
Date: Thu, 4 Feb 2021 10:45:12 +0000	[thread overview]
Message-ID: <fd88ce05-8aee-5b1f-5ab6-be88fa53d3aa@huawei.com> (raw)
In-Reply-To: <3d3d0155e66429968cb4f6b4feeae4b3@kernel.org>

On 03/02/2021 17:23, Marc Zyngier wrote:
>>
>> Before:
>>  In free path, we have:
>>      its_irq_domain_free(nvecs = 27)
>>        bitmap_release_region(count order = 5 == 32bits)
>>
>> Current:
>>  In free path, we have:
>>      its_irq_domain_free(nvecs = 1) for free each 27 vecs
>>        bitmap_release_region(count order = 0 == 1bit)
> 
> Right. I was focusing on the patch and blindly ignored the explanation
> at the top of the email. Apologies for that.

Yeah, it was a distraction.

> 
> I'm not overly keen on handling this in the ITS though, and I'd rather
> we try and do it in the generic code. How about this (compile tested
> only)?
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 6aacd342cd14..cfccad83c2df 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -1399,8 +1399,19 @@ static void irq_domain_free_irqs_hierarchy(struct 
> irq_domain *domain,
>           return;
> 
>       for (i = 0; i < nr_irqs; i++) {
> -        if (irq_domain_get_irq_data(domain, irq_base + i))
> -            domain->ops->free(domain, irq_base + i, 1);
> +        int n ;
> +
> +        /* Find the largest possible span of IRQs to free in one go */
> +        for (n = 0;
> +             ((i + n) < nr_irqs &&
> +              irq_domain_get_irq_data(domain, irq_base + i + n));
> +             n++);
> +
> +        if (!n)
> +            continue;
> +
> +        domain->ops->free(domain, irq_base + i, n);
> +        i += n;
>       }
>   }

That fixed my problem.

For my benefit, if MSIs must be allocated in power of 2, could we check 
a flag for the dealloc method? Like, if MSI domain, then do as before 
4615fbc3788d. But I'm not sure on the specific scenario which that 
commit fixed. Or even whether you want specifics like that in core code.

Thanks,
John

  reply	other threads:[~2021-02-04 10:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-01 18:34 PCI MSI issue with reinserting a driver John Garry
2021-02-01 18:50 ` Marc Zyngier
2021-02-02  8:37   ` John Garry
2021-02-02 12:38     ` John Garry
2021-02-02 14:48       ` Marc Zyngier
2021-02-02 15:46         ` John Garry
2021-02-03 17:23           ` Marc Zyngier
2021-02-04 10:45             ` John Garry [this message]
2022-08-04 10:59               ` John Garry
2021-04-06  9:46             ` John Garry
2021-08-27  8:33             ` luojiaxing
2023-08-29 23:00             ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fd88ce05-8aee-5b1f-5ab6-be88fa53d3aa@huawei.com \
    --to=john.garry@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=wangzhou1@hisilicon.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).