linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: daniel.lezcano@linaro.org (Daniel Lezcano)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: timer: Shutdown clock event device when stopping local timer
Date: Mon, 01 Apr 2013 02:06:17 +0200	[thread overview]
Message-ID: <5158CF79.2060702@linaro.org> (raw)
In-Reply-To: <1364549049-29278-1-git-send-email-ning.n.jiang@gmail.com>

On 03/29/2013 10:24 AM, ning.n.jiang at gmail.com wrote:
> From: Ning Jiang <ning.n.jiang@gmail.com>
> 
> Currently there are two problems when we try to stop local timer.
> First, it calls set_mode function directly so mode state is not
> updated for the clock event device. Second, it makes the device
> unused instead of shutdown.
> 
> A subtle error will happen because of it. When a cpu is plugged out
> it will stop the local timer. It will call tick_nohz_idle_enter()
> in idle thread afterwards. It will cancel the sched timer and try
> to reprogram the next event. This is wrong since the local timer
> is supposed to be stopped.
> 
> The right way to stop the local timer is to shutdown it by calling
> clockevents_set_mode(). Thus when we try to reprogram the clock
> event device, it will return directly without doing anything since
> the clock mode is CLOCK_EVT_MODE_SHUTDOWN.
> 
> Signed-off-by: Ning Jiang <ning.n.jiang@gmail.com>
> ---


Don't you see a kernel BUG with this patch ?

In the clockevents code, there is:

/**
 * clockevents_notify - notification about relevant events
 */
void clockevents_notify(unsigned long reason, void *arg)
{
	...
	case CLOCK_EVT_NOTIFY_CPU_DEAD:
	...
                cpu = *((int *)arg);
                list_for_each_entry_safe(dev, tmp, &clockevent_devices,
					list) {
                       if (cpumask_test_cpu(cpu, dev->cpumask) &&
                            cpumask_weight(dev->cpumask) == 1 &&
                            !tick_is_broadcast_device(dev)) {
                                BUG_ON(dev->mode !=
					CLOCK_EVT_MODE_UNUSED);

				^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

                                list_del(&dev->list);
                        }
                }
                break;
	...
}

This is called triggered from hrtimer_cpu_notify with the CPU_DEAD event.


-- 
 <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

  parent reply	other threads:[~2013-04-01  0:06 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-29  9:24 [PATCH] ARM: timer: Shutdown clock event device when stopping local timer ning.n.jiang at gmail.com
2013-03-29 18:32 ` Stephen Boyd
2013-03-30  9:57   ` Ning Jiang
2013-03-30 10:04     ` Russell King - ARM Linux
2013-03-30 11:56       ` Ning Jiang
2013-03-31 13:11         ` Ning Jiang
2013-03-31 23:41 ` Daniel Lezcano
2013-04-01  1:56   ` Ning Jiang
2013-04-02 12:01     ` Ning Jiang
2013-04-01  0:06 ` Daniel Lezcano [this message]
2013-04-01  2:05   ` Ning Jiang

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=5158CF79.2060702@linaro.org \
    --to=daniel.lezcano@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.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 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).