All of lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
To: Mitchel Humpherys <mitchelh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org"
	<iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Subject: Re: [PATCH] iommu/arm-smmu: use a threaded handler for context interrupts
Date: Fri, 23 Jan 2015 11:24:15 +0000	[thread overview]
Message-ID: <20150123112415.GD23058@arm.com> (raw)
In-Reply-To: <1421970482-11722-1-git-send-email-mitchelh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Hi Mitch,

On Thu, Jan 22, 2015 at 11:48:02PM +0000, Mitchel Humpherys wrote:
> Context interrupts can call domain-specific handlers which might sleep.
> Currently we register our handler with request_irq, so our handler is
> called in atomic context, so domain handlers that sleep result in an
> invalid context BUG.  Fix this by using request_threaded_irq.
> 
> This also prepares the way for doing things like enabling clocks within
> our interrupt handler.
> 
> Signed-off-by: Mitchel Humpherys <mitchelh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 6cd47b75286f..81f6b54d94b1 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -973,8 +973,9 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
>  	spin_unlock_irqrestore(&smmu_domain->lock, flags);
>  
>  	irq = smmu->irqs[smmu->num_global_irqs + cfg->irptndx];
> -	ret = request_irq(irq, arm_smmu_context_fault, IRQF_SHARED,
> -			  "arm-smmu-context-fault", domain);
> +	ret = request_threaded_irq(irq, NULL, arm_smmu_context_fault,
> +				IRQF_ONESHOT | IRQF_SHARED,
> +				"arm-smmu-context-fault", domain);
>  	if (IS_ERR_VALUE(ret)) {
>  		dev_err(smmu->dev, "failed to request context IRQ %d (%u)\n",
>  			cfg->irptndx, irq);

I think I'd rather keep a simple atomic handler, then have a threaded
handler for actually issuing the report_iommu_fault. i.e. we only wake
the thread when it looks like there's some work to do. That also works
much better for shared interrupts.

Will

WARNING: multiple messages have this Message-ID (diff)
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] iommu/arm-smmu: use a threaded handler for context interrupts
Date: Fri, 23 Jan 2015 11:24:15 +0000	[thread overview]
Message-ID: <20150123112415.GD23058@arm.com> (raw)
In-Reply-To: <1421970482-11722-1-git-send-email-mitchelh@codeaurora.org>

Hi Mitch,

On Thu, Jan 22, 2015 at 11:48:02PM +0000, Mitchel Humpherys wrote:
> Context interrupts can call domain-specific handlers which might sleep.
> Currently we register our handler with request_irq, so our handler is
> called in atomic context, so domain handlers that sleep result in an
> invalid context BUG.  Fix this by using request_threaded_irq.
> 
> This also prepares the way for doing things like enabling clocks within
> our interrupt handler.
> 
> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
> ---
>  drivers/iommu/arm-smmu.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 6cd47b75286f..81f6b54d94b1 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -973,8 +973,9 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
>  	spin_unlock_irqrestore(&smmu_domain->lock, flags);
>  
>  	irq = smmu->irqs[smmu->num_global_irqs + cfg->irptndx];
> -	ret = request_irq(irq, arm_smmu_context_fault, IRQF_SHARED,
> -			  "arm-smmu-context-fault", domain);
> +	ret = request_threaded_irq(irq, NULL, arm_smmu_context_fault,
> +				IRQF_ONESHOT | IRQF_SHARED,
> +				"arm-smmu-context-fault", domain);
>  	if (IS_ERR_VALUE(ret)) {
>  		dev_err(smmu->dev, "failed to request context IRQ %d (%u)\n",
>  			cfg->irptndx, irq);

I think I'd rather keep a simple atomic handler, then have a threaded
handler for actually issuing the report_iommu_fault. i.e. we only wake
the thread when it looks like there's some work to do. That also works
much better for shared interrupts.

Will

  parent reply	other threads:[~2015-01-23 11:24 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-22 23:48 [PATCH] iommu/arm-smmu: use a threaded handler for context interrupts Mitchel Humpherys
2015-01-22 23:48 ` Mitchel Humpherys
     [not found] ` <1421970482-11722-1-git-send-email-mitchelh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2015-01-23 11:24   ` Will Deacon [this message]
2015-01-23 11:24     ` Will Deacon
     [not found]     ` <20150123112415.GD23058-5wv7dgnIgG8@public.gmane.org>
2015-01-23 22:33       ` Mitchel Humpherys
2015-01-23 22:33         ` Mitchel Humpherys
     [not found]         ` <vnkw61bx2klr.fsf-Yf+dfxj6toJBVvN7MMdr1KRtKmQZhJ7pQQ4Iyu8u01E@public.gmane.org>
2015-01-28 12:07           ` Will Deacon
2015-01-28 12:07             ` Will Deacon
     [not found]             ` <20150128120738.GJ1569-5wv7dgnIgG8@public.gmane.org>
2015-02-02 20:10               ` Mitchel Humpherys
2015-02-02 20:10                 ` Mitchel Humpherys
     [not found]                 ` <vnkwa90why79.fsf-Yf+dfxj6toJBVvN7MMdr1KRtKmQZhJ7pQQ4Iyu8u01E@public.gmane.org>
2015-02-04 11:33                   ` Will Deacon
2015-02-04 11:33                     ` Will Deacon
     [not found]                     ` <20150204113305.GA28902-5wv7dgnIgG8@public.gmane.org>
2015-02-04 17:19                       ` Mitchel Humpherys
2015-02-04 17:19                         ` Mitchel Humpherys

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=20150123112415.GD23058@arm.com \
    --to=will.deacon-5wv7dgnigg8@public.gmane.org \
    --cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=mitchelh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.