2018-12-21 22:45:30

by Madalin-cristian Bucur

Subject: [PATCH] soc: fsl: qbman: avoid race in clearing QMan interrupt

By clearing all interrupt sources, not only those that
already occurred, the existing code may acknowledge by
mistake interrupts that occurred after the code checks
for them.

Signed-off-by: Madalin Bucur <[email protected]>
Signed-off-by: Roy Pledge <[email protected]>
---
drivers/soc/fsl/qbman/qman.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
index 52c153cd795a..636f83f781f5 100644
--- a/drivers/soc/fsl/qbman/qman.c
+++ b/drivers/soc/fsl/qbman/qman.c
@@ -1143,18 +1143,19 @@ static void qm_mr_process_task(struct work_struct *work);
static irqreturn_t portal_isr(int irq, void *ptr)
{
struct qman_portal *p = ptr;
-
- u32 clear = QM_DQAVAIL_MASK | p->irq_sources;
u32 is = qm_in(&p->p, QM_REG_ISR) & p->irq_sources;
+ u32 clear = 0;

if (unlikely(!is))
return IRQ_NONE;

/* DQRR-handling if it's interrupt-driven */
- if (is & QM_PIRQ_DQRI)
+ if (is & QM_PIRQ_DQRI) {
__poll_portal_fast(p, QMAN_POLL_LIMIT);
+ clear = QM_DQAVAIL_MASK | QM_PIRQ_DQRI;
+ }
/* Handling of anything else that's interrupt-driven */
- clear |= __poll_portal_slow(p, is);
+ clear |= __poll_portal_slow(p, is) & QM_PIRQ_SLOW;
qm_out(&p->p, QM_REG_ISR, clear);
return IRQ_HANDLED;
}
--
2.1.0

2019-01-18 22:57:57

by Leo Li

Subject: Re: [PATCH] soc: fsl: qbman: avoid race in clearing QMan interrupt

On Fri, Dec 21, 2018 at 8:43 AM Madalin Bucur <[email protected]> wrote:
>
> By clearing all interrupt sources, not only those that
> already occurred, the existing code may acknowledge by
> mistake interrupts that occurred after the code checks
> for them.
>
> Signed-off-by: Madalin Bucur <[email protected]>
> Signed-off-by: Roy Pledge <[email protected]>

Applied for fix. Thanks. Suggest to forward it to
[email protected] after merged into Linus' tree. It would be
better to add "Cc: [email protected]" in the commit message for
similar patches next time.

Regards,
Leo

> ---
> drivers/soc/fsl/qbman/qman.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
> index 52c153cd795a..636f83f781f5 100644
> --- a/drivers/soc/fsl/qbman/qman.c
> +++ b/drivers/soc/fsl/qbman/qman.c
> @@ -1143,18 +1143,19 @@ static void qm_mr_process_task(struct work_struct *work);
> static irqreturn_t portal_isr(int irq, void *ptr)
> {
> struct qman_portal *p = ptr;
> -
> - u32 clear = QM_DQAVAIL_MASK | p->irq_sources;
> u32 is = qm_in(&p->p, QM_REG_ISR) & p->irq_sources;
> + u32 clear = 0;
>
> if (unlikely(!is))
> return IRQ_NONE;
>
> /* DQRR-handling if it's interrupt-driven */
> - if (is & QM_PIRQ_DQRI)
> + if (is & QM_PIRQ_DQRI) {
> __poll_portal_fast(p, QMAN_POLL_LIMIT);
> + clear = QM_DQAVAIL_MASK | QM_PIRQ_DQRI;
> + }
> /* Handling of anything else that's interrupt-driven */
> - clear |= __poll_portal_slow(p, is);
> + clear |= __poll_portal_slow(p, is) & QM_PIRQ_SLOW;
> qm_out(&p->p, QM_REG_ISR, clear);
> return IRQ_HANDLED;
> }
> --
> 2.1.0
>