All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Preeti U Murthy <preeti@linux.vnet.ibm.com>
Cc: paulmck@linux.vnet.ibm.com, paulus@samba.org,
	Anton Blanchard <anton@samba.org>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] powerpc: irq work racing with timer interrupt can result in timer interrupt hang
Date: Sun, 11 May 2014 19:03:28 +1000	[thread overview]
Message-ID: <1399799008.17624.43.camel@pasglop> (raw)
In-Reply-To: <536F384C.3070409@linux.vnet.ibm.com>

On Sun, 2014-05-11 at 14:13 +0530, Preeti U Murthy wrote:
> 
> Isn't this patch required too?
> 
> @@ -503,12 +503,13 @@ void __timer_interrupt(void)
>                 now = *next_tb - now;
>                 if (now <= DECREMENTER_MAX)
>                         set_dec((int)now);
> -               /* We may have raced with new irq work */
> -               if (test_irq_work_pending())
> -                       set_dec(1);
>                 __get_cpu_var(irq_stat).timer_irqs_others++;
>         }
>
> +       /* We may have raced with new irq work */
> +       if (test_irq_work_pending())
> +               set_dec(1);
> +
> 
> The event_handler cannot be relied upon to call
> decrementer_set_next_event() all the time. This is in the case where
> there are no pending timers. In that case we need to have the check on
> irq work pending at the end of __timer_interrupt() no?

I don't think we need to move the test no. If there's a pending
irq_work, at that point, it will have done set_dec when being queued up.
So we only care about cases where we might change the decrementer.

If the event handler doesn't call decrementer_set_next_event() then
nothing will modify the decrementer and it will still trigger soon.

Cheers,
Ben.

  reply	other threads:[~2014-05-11  9:03 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-09  7:47 [PATCH] powerpc: irq work racing with timer interrupt can result in timer interrupt hang Anton Blanchard
2014-05-09  9:52 ` Preeti U Murthy
2014-05-10  4:26   ` Benjamin Herrenschmidt
2014-05-10 15:36     ` Preeti U Murthy
2014-05-10 22:25       ` Benjamin Herrenschmidt
2014-05-11  8:15         ` Preeti U Murthy
2014-05-11  8:37           ` Benjamin Herrenschmidt
2014-05-11  8:43             ` Preeti U Murthy
2014-05-11  9:03               ` Benjamin Herrenschmidt [this message]
2014-05-11  9:07                 ` Preeti U Murthy
2014-05-09 13:41 ` Paul E. McKenney
2014-05-09 21:50   ` Gabriel Paubert
2014-05-09 22:08     ` Paul E. McKenney
2014-05-10  6:33       ` Paul Mackerras
2014-05-10 16:33         ` Paul E. McKenney

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=1399799008.17624.43.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=anton@samba.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=paulus@samba.org \
    --cc=preeti@linux.vnet.ibm.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 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.