| Message ID | 1af3a250749c7c103ef38b52d653559ac4ec899c.1397537987.git.viresh.kumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Tue, 15 Apr 2014, Viresh Kumar wrote: > If 'curdev' passed to tick_check_preferred() is the current clock_event_device > then these two checks look exactly same, because td->mode is set to > TICKDEV_MODE_ONESHOT only when the event device has ONESHOT feature. > > if (curdev && (curdev->features & CLOCK_EVT_FEAT_ONESHOT)) > return false; > > if (tick_oneshot_mode_active()) > return false; > > Now left the case where 'curdev' is not the current clock_event_device. This can > happen from the sequence started from clockevents_replace(). Here we are trying > to find the best possible device that we should choose. And so even in this case > we don't need the above check as we aren't really worried about the current > device. Wrong. If curdev is NULL, you might select a device w/o ONESHOT if the system is in oneshot mode. Go figure. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 16 April 2014 00:00, Thomas Gleixner <tglx@linutronix.de> wrote: > On Tue, 15 Apr 2014, Viresh Kumar wrote: > >> If 'curdev' passed to tick_check_preferred() is the current clock_event_device >> then these two checks look exactly same, because td->mode is set to >> TICKDEV_MODE_ONESHOT only when the event device has ONESHOT feature. >> >> if (curdev && (curdev->features & CLOCK_EVT_FEAT_ONESHOT)) >> return false; >> >> if (tick_oneshot_mode_active()) >> return false; >> >> Now left the case where 'curdev' is not the current clock_event_device. This can >> happen from the sequence started from clockevents_replace(). Here we are trying >> to find the best possible device that we should choose. And so even in this case >> we don't need the above check as we aren't really worried about the current >> device. > > Wrong. If curdev is NULL, you might select a device w/o ONESHOT if the > system is in oneshot mode. Go figure. Okay, so the logs must have another case where curdev is NULL. But codewise we are already taking care of that here: return !curdev || newdev->rating > curdev->rating || !cpumask_equal(curdev->cpumask, newdev->cpumask); And so this patch wouldn't harm. And this is preserved in the next patch (3/5) as well, which adds checks for other cases as well. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c index 0a0608e..69cab28 100644 --- a/kernel/time/tick-common.c +++ b/kernel/time/tick-common.c @@ -256,8 +256,6 @@ static bool tick_check_preferred(struct clock_event_device *curdev, if (!(newdev->features & CLOCK_EVT_FEAT_ONESHOT)) { if (curdev && (curdev->features & CLOCK_EVT_FEAT_ONESHOT)) return false; - if (tick_oneshot_mode_active()) - return false; } /*
If 'curdev' passed to tick_check_preferred() is the current clock_event_device then these two checks look exactly same, because td->mode is set to TICKDEV_MODE_ONESHOT only when the event device has ONESHOT feature. if (curdev && (curdev->features & CLOCK_EVT_FEAT_ONESHOT)) return false; if (tick_oneshot_mode_active()) return false; Now left the case where 'curdev' is not the current clock_event_device. This can happen from the sequence started from clockevents_replace(). Here we are trying to find the best possible device that we should choose. And so even in this case we don't need the above check as we aren't really worried about the current device. So, the second check can be removed. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- kernel/time/tick-common.c | 2 -- 1 file changed, 2 deletions(-)