Re: [PATCH v3 1/3] clocksource/vt8500: Increase the minimum delta

[Daniel Lezcano]

On 01/05/2016 10:42 AM, Roman Volkov wrote:
> Ð Tue, 5 Jan 2016 10:01:07 +0100
> Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> ÐÐÑÐÑ:
>
> > On 01/01/2016 02:24 PM, Roman Volkov wrote:
> > > From: Roman Volkov <rvolkov@xxxxxxxxx>
> > >
> > > The vt8500 clocksource driver declares itself as capable to handle
> > > the minimum delay of 4 cycles by passing the value into
> > > clockevents_config_and_register(). The vt8500_timer_set_next_event()
> > > requires the passed cycles value to be at least 16. The impact is
> > > that userspace hangs in nanosleep() calls with small delay
> > > intervals.
> > >
> > > This problem is reproducible in Linux 4.2 starting from:
> > > c6eb3f70d448 ('hrtimer: Get rid of hrtimer softirq')
> > >
> > > Signed-off-by: Roman Volkov <rvolkov@xxxxxxxxx>
> > > Acked-by: Alexey Charkov <alchark@xxxxxxxxx>
> >
> > Hi Roman,
> >
> > I looked at the email thread, and IIUC if set_next_event fails, the
> > system freeze. Your patch fixes the issue for your driver but not the
> > real issue because if set_next_event fails, at least a warning should
> > appear in the log or better nanosleep should fail gracefully.
>
> Hi Daniel,
>
> I agree, but if nanosleep will return immediately, this can lead to
> undefined behavior in the software.

The nanosleep syscall is supposed to return an error code. If the 
software does not pay attention to the syscall's return code, then the 
bug is in the software, it is not up to the kernel to work around it.

> Maybe the system can go busyloop
> to somehow recover from this state and print a message to the log? At
> the driver level it seems to be enough to fail the function without
> printing logs.
>
> > BTW why min delta is MIN_OSCR_DELTA * 2 in
> > clockevents_config_and_register ?
>
> All this just to be consistent with PXA. Maybe PXA works with lesser
> values, e.g., 8. For vt8500, accessing the registers is more complex,
> and this should consume more time. IIUC, if the driver does not support
> too small delays, the system will handle it with busyloop?

[ Added John Stultz and Thomas Gleixner ] to answer those questions above.

> Why multiply by two? Good question. Maybe there is a reserve for
> stability. The value passed by the system to the set_next_event() should
> be not lesser than this value, and theoretically, we should not
> multiply MIN_OSCR_DELTA by two. As I can see, in many drivers there is
> no such minimal values at all.
>
> Added Robert
>
> Regards,
> Roman


--
 <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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/