On 08/12/2013 06:23 PM, Stephen Boyd wrote: > On 08/12/13 03:53, Daniel Lezcano wrote: >> On 08/09/2013 07:27 PM, Stephen Boyd wrote: >>> On 08/09, Daniel Lezcano wrote: >>>> yes, but at least the broadcast mechanism should send an IPI to cpu0 to >>>> wake it up, no ? As Stephen stated this kind of configuration should has >>>> never been tested before so the tick broadcast code is not handling this >>>> case properly IMHO. >>>> >>> If you have a per-cpu tick device that isn't suffering from >>> FEAT_C3_STOP why wouldn't you use that for the tick versus a >>> per-cpu tick device that has FEAT_C3_STOP? It sounds like there >>> is a bug in the preference logic or you should boost the rating >>> of the arm global timer above the twd. Does this patch help? It >>> should make the arm global timer the tick device and whatever the >>> cadence timer you have into the broadcast device. >>> >>> ---8<---- >>> diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c >>> index 218bcb5..d3539e5 100644 >>> --- a/kernel/time/tick-broadcast.c >>> +++ b/kernel/time/tick-broadcast.c >>> @@ -77,6 +77,9 @@ static bool tick_check_broadcast_device(struct >>> clock_event_device *curdev, >>> !(newdev->features & CLOCK_EVT_FEAT_ONESHOT)) >>> return false; >>> >>> + if (cpumask_equal(newdev->cpumask, cpumask_of(smp_processor_id()))) >>> + return false; >> Yes, that makes sense to prevent local timer devices to be a broadcast one. >> >> In the case of the arm global timer, each cpu will register their timer, >> so the test will be ok but is it possible the cpu0 registers the timers >> for the other cpus ? > > As far as I know every tick device has to be registered on the CPU that > it will be used on. See tick_check_percpu().
Ah, ok I see. Thx for the pointer. >>> return !curdev || newdev->rating > curdev->rating; >>> } >>> >>> diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c >>> index 64522ec..1628b9f 100644 >>> --- a/kernel/time/tick-common.c >>> +++ b/kernel/time/tick-common.c >>> @@ -245,6 +245,15 @@ static bool tick_check_preferred(struct >>> clock_event_device *curdev, >>> } >>> >>> /* >>> + * Prefer tick devices that don't suffer from FEAT_C3_STOP >>> + * regardless of their rating >>> + */ >>> + if (curdev && cpumask_equal(curdev->cpumask, newdev->cpumask) && >>> + !(newdev->features & CLOCK_EVT_FEAT_C3STOP) && >>> + (curdev->features & CLOCK_EVT_FEAT_C3STOP)) >>> + return true; >> That sounds reasonable, but what is the acceptable gap between the >> ratings ? I am wondering if there isn't too much heuristic in the tick >> code... > > Yes I wonder if we should just change the ratings of the clockevents. > But it feels to me like the rating should only matter if the two are > equal in features. Otherwise we can use the features to determine what > we want. Is it desirable for real time system ? (I am not expert in this area, so may be this question has no sense :) -- <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 -- 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/
