On 06/26/2015 01:17 PM, Thomas Gleixner wrote:
> diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
> index d39f32cdd1b5..281ce29d295e 100644
> --- a/kernel/time/tick-broadcast.c
> +++ b/kernel/time/tick-broadcast.c
> @@ -662,46 +662,39 @@ static void broadcast_shutdown_local(struct
> clock_event_device *bc,
> clockevents_switch_state(dev, CLOCK_EVT_STATE_SHUTDOWN);
> }
>
> -/**
> - * tick_broadcast_oneshot_control - Enter/exit broadcast oneshot mode
> - * @state: The target state (enter/exit)
> - *
> - * The system enters/leaves a state, where affected devices might stop
> - * Returns 0 on success, -EBUSY if the cpu is used to broadcast wakeups.
> - *
> - * Called with interrupts disabled, so clockevents_lock is not
> - * required here because the local clock event device cannot go away
> - * under us.
> - */
> -int tick_broadcast_oneshot_control(enum tick_broadcast_state state)
> +int __tick_broadcast_oneshot_control(enum tick_broadcast_state state)
> {
> struct clock_event_device *bc, *dev;
> - struct tick_device *td;
> int cpu, ret = 0;
> ktime_t now;
>
> - /*
> - * Periodic mode does not care about the enter/exit of power
> - * states
> - */
> - if (tick_broadcast_device.mode == TICKDEV_MODE_PERIODIC)
> - return 0;
> + if (!tick_broadcast_device.evtdev)
> + return -EBUSY;
>
> - /*
> - * We are called with preemtion disabled from the depth of the
> - * idle code, so we can't be moved away.
> - */
> - td = this_cpu_ptr(&tick_cpu_device);
> - dev = td->evtdev;
> -
> - if (!(dev->features & CLOCK_EVT_FEAT_C3STOP))
> - return 0;
> + dev = this_cpu_ptr(&tick_cpu_device)->evtdev;
>
> raw_spin_lock(&tick_broadcast_lock);
> bc = tick_broadcast_device.evtdev;
> cpu = smp_processor_id();
>
> if (state == TICK_BROADCAST_ENTER) {
> + /*
> + * If the current CPU owns the hrtimer broadcast
> + * mechanism, it cannot go deep idle and we do not add
> + * the CPU to the broadcast mask. We don't have to go
> + * through the EXIT path as the local timer is not
> + * shutdown.
> + */
> + ret = broadcast_needs_cpu(bc, cpu);
> +
> + /*
> + * If the hrtimer broadcast check tells us that the
> + * cpu cannot go deep idle, or if the broadcast device
> + * is in periodic mode, we just return.
> + */
> + if (ret || tick_broadcast_device.mode == TICKDEV_MODE_PERIODIC)
> + goto out;
The check on PERIODIC mode is good, but I don't see the point of moving
broadcast_needs_cpu() up above. broadcast_shutdown_local() calls
broadcast_needs_cpu() internally.
Besides, by jumping to 'out', we will miss programming the broadcast
hrtimer in tick_broadcast_set_event() below, if the cpu happen to be the
broadcast cpu(which is why it was not allowed to go to deep idle).
> +
> if (!cpumask_test_and_set_cpu(cpu,
> tick_broadcast_oneshot_mask)) {
> WARN_ON_ONCE(cpumask_test_cpu(cpu,
> tick_broadcast_pending_mask));
> broadcast_shutdown_local(bc, dev);
> @@ -717,16 +710,6 @@ int tick_broadcast_oneshot_control(enum
> tick_broadcast_state state)
> dev->next_event.tv64 < bc->next_event.tv64)
> tick_broadcast_set_event(bc, cpu,
> dev->next_event);
> }
> - /*
> - * If the current CPU owns the hrtimer broadcast
> - * mechanism, it cannot go deep idle and we remove the
> - * CPU from the broadcast mask. We don't have to go
> - * through the EXIT path as the local timer is not
> - * shutdown.
> - */
> - ret = broadcast_needs_cpu(bc, cpu);
> - if (ret)
> - cpumask_clear_cpu(cpu, tick_broadcast_oneshot_mask);
> } else {
> if (cpumask_test_and_clear_cpu(cpu,
> tick_broadcast_oneshot_mask)) {
> clockevents_switch_state(dev, CLOCK_EVT_STATE_ONESHOT);
> @@ -938,6 +921,13 @@ bool tick_broadcast_oneshot_available(void)
> return bc ? bc->features & CLOCK_EVT_FEAT_ONESHOT : false;
> }
>
Regards
Preeti U Murthy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
