* [PATCH] alarmtimer: return EINVAL instead of ENOTSUPP if rtcdev doesn't exist
@ 2013-10-14 21:33 kosaki.motohiro
2013-10-17 17:05 ` John Stultz
0 siblings, 1 reply; 6+ messages in thread
From: kosaki.motohiro @ 2013-10-14 21:33 UTC (permalink / raw)
To: linux-kernel
Cc: KOSAKI Motohiro, Thomas Gleixner, Frederic Weisbecker,
John Stultz
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Fedora Ruby maintainer reported latest Ruby doesn't work on Fedora Rawhide
on ARM. (http://bugs.ruby-lang.org/issues/9008)
Because of, commit 1c6b39ad3f (alarmtimers: Return -ENOTSUPP if no
RTC device is present) intruduced to return ENOTSUPP when
clock_get{time,res} can't find a RTC device. However it is incorrect.
Posix and Linux man pages agree that clock_gettime and clock_getres
should return EINVAL if clk_id argument is invalid. This is significant
different from timer_create API.
This patch fixes it.
Reported-by: Vit Ondruch <v.ondruch@tiscali.cz>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: John Stultz <john.stultz@linaro.org>
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
kernel/time/alarmtimer.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index eec50fc..88c9c65 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -490,7 +490,7 @@ static int alarm_clock_getres(const clockid_t which_clock, struct timespec *tp)
clockid_t baseid = alarm_bases[clock2alarm(which_clock)].base_clockid;
if (!alarmtimer_get_rtcdev())
- return -ENOTSUPP;
+ return -EINVAL;
return hrtimer_get_res(baseid, tp);
}
@@ -507,7 +507,7 @@ static int alarm_clock_get(clockid_t which_clock, struct timespec *tp)
struct alarm_base *base = &alarm_bases[clock2alarm(which_clock)];
if (!alarmtimer_get_rtcdev())
- return -ENOTSUPP;
+ return -EINVAL;
*tp = ktime_to_timespec(base->gettime());
return 0;
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] alarmtimer: return EINVAL instead of ENOTSUPP if rtcdev doesn't exist
2013-10-14 21:33 [PATCH] alarmtimer: return EINVAL instead of ENOTSUPP if rtcdev doesn't exist kosaki.motohiro
@ 2013-10-17 17:05 ` John Stultz
2013-10-18 1:12 ` KOSAKI Motohiro
0 siblings, 1 reply; 6+ messages in thread
From: John Stultz @ 2013-10-17 17:05 UTC (permalink / raw)
To: kosaki.motohiro, linux-kernel
Cc: KOSAKI Motohiro, Thomas Gleixner, Frederic Weisbecker
On 10/14/2013 02:33 PM, kosaki.motohiro@gmail.com wrote:
> From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>
> Fedora Ruby maintainer reported latest Ruby doesn't work on Fedora Rawhide
> on ARM. (http://bugs.ruby-lang.org/issues/9008)
>
> Because of, commit 1c6b39ad3f (alarmtimers: Return -ENOTSUPP if no
> RTC device is present) intruduced to return ENOTSUPP when
> clock_get{time,res} can't find a RTC device. However it is incorrect.
>
> Posix and Linux man pages agree that clock_gettime and clock_getres
> should return EINVAL if clk_id argument is invalid. This is significant
> different from timer_create API.
>
> This patch fixes it.
Hrm... So I feel like there is a difference here. The clockid for
CLOCK_BOOTTIME_ALARM and CLOCK_REALTIME_ALARM are both valid.
Its just that they're not supported on this specific hardware because it
apparently lacks a RTC that has told the system it can be used as a
wakeup device (Its actually quite likely on the hardware that the RTC
can be a wakeup device, but that the driver is probably setting the
wakeup flag after the RTC registered - so there is probably a driver bug
here too).
So I feel like in this case EINVAL isn't quite right. I'll admit it is
somewhat new behavior, because we haven't had any clockids before that
were dependent on the particular hardware, they either existed in a
kernel verison or didn't.
Would updating the manpage be a better route?
thanks
-john
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] alarmtimer: return EINVAL instead of ENOTSUPP if rtcdev doesn't exist
2013-10-17 17:05 ` John Stultz
@ 2013-10-18 1:12 ` KOSAKI Motohiro
2013-10-18 22:39 ` John Stultz
0 siblings, 1 reply; 6+ messages in thread
From: KOSAKI Motohiro @ 2013-10-18 1:12 UTC (permalink / raw)
To: John Stultz
Cc: kosaki.motohiro, linux-kernel, KOSAKI Motohiro, Thomas Gleixner,
Frederic Weisbecker
(10/17/13 1:05 PM), John Stultz wrote:
> On 10/14/2013 02:33 PM, kosaki.motohiro@gmail.com wrote:
>> From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>>
>> Fedora Ruby maintainer reported latest Ruby doesn't work on Fedora Rawhide
>> on ARM. (http://bugs.ruby-lang.org/issues/9008)
>>
>> Because of, commit 1c6b39ad3f (alarmtimers: Return -ENOTSUPP if no
>> RTC device is present) intruduced to return ENOTSUPP when
>> clock_get{time,res} can't find a RTC device. However it is incorrect.
>>
>> Posix and Linux man pages agree that clock_gettime and clock_getres
>> should return EINVAL if clk_id argument is invalid. This is significant
>> different from timer_create API.
>>
>> This patch fixes it.
>
> Hrm... So I feel like there is a difference here. The clockid for
> CLOCK_BOOTTIME_ALARM and CLOCK_REALTIME_ALARM are both valid.
>
> Its just that they're not supported on this specific hardware because it
> apparently lacks a RTC that has told the system it can be used as a
> wakeup device (Its actually quite likely on the hardware that the RTC
> can be a wakeup device, but that the driver is probably setting the
> wakeup flag after the RTC registered - so there is probably a driver bug
> here too).
>
> So I feel like in this case EINVAL isn't quite right. I'll admit it is
> somewhat new behavior, because we haven't had any clockids before that
> were dependent on the particular hardware, they either existed in a
> kernel verison or didn't.
>
> Would updating the manpage be a better route?
Nope.
ENOTSUPP is not exported to userland. ENOTSUP (single P) and EOPNOTSUP is
valid errno (and they are same on linux), but ENOTSUPP is a kernel internal specific.
Moreover, I completely disagree your position. Both CLOCK_REALTIME_ALARM unsupported
kernel and ARM which doesn't support RTC should use the same error because application
need the same fallback.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] alarmtimer: return EINVAL instead of ENOTSUPP if rtcdev doesn't exist
2013-10-18 1:12 ` KOSAKI Motohiro
@ 2013-10-18 22:39 ` John Stultz
2013-10-18 23:12 ` KOSAKI Motohiro
0 siblings, 1 reply; 6+ messages in thread
From: John Stultz @ 2013-10-18 22:39 UTC (permalink / raw)
To: KOSAKI Motohiro
Cc: linux-kernel, KOSAKI Motohiro, Thomas Gleixner,
Frederic Weisbecker
On 10/17/2013 06:12 PM, KOSAKI Motohiro wrote:
> (10/17/13 1:05 PM), John Stultz wrote:
>> On 10/14/2013 02:33 PM, kosaki.motohiro@gmail.com wrote:
>>> From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>>>
>>> Fedora Ruby maintainer reported latest Ruby doesn't work on Fedora
>>> Rawhide
>>> on ARM. (http://bugs.ruby-lang.org/issues/9008)
>>>
>>> Because of, commit 1c6b39ad3f (alarmtimers: Return -ENOTSUPP if no
>>> RTC device is present) intruduced to return ENOTSUPP when
>>> clock_get{time,res} can't find a RTC device. However it is incorrect.
>>>
>>> Posix and Linux man pages agree that clock_gettime and clock_getres
>>> should return EINVAL if clk_id argument is invalid. This is significant
>>> different from timer_create API.
>>>
>>> This patch fixes it.
>>
>> Hrm... So I feel like there is a difference here. The clockid for
>> CLOCK_BOOTTIME_ALARM and CLOCK_REALTIME_ALARM are both valid.
>>
>> Its just that they're not supported on this specific hardware because it
>> apparently lacks a RTC that has told the system it can be used as a
>> wakeup device (Its actually quite likely on the hardware that the RTC
>> can be a wakeup device, but that the driver is probably setting the
>> wakeup flag after the RTC registered - so there is probably a driver bug
>> here too).
>>
>> So I feel like in this case EINVAL isn't quite right. I'll admit it is
>> somewhat new behavior, because we haven't had any clockids before that
>> were dependent on the particular hardware, they either existed in a
>> kernel verison or didn't.
>>
>> Would updating the manpage be a better route?
>
> Nope.
>
> ENOTSUPP is not exported to userland. ENOTSUP (single P) and EOPNOTSUP is
> valid errno (and they are same on linux), but ENOTSUPP is a kernel
> internal specific.
>
> Moreover, I completely disagree your position. Both
> CLOCK_REALTIME_ALARM unsupported
> kernel and ARM which doesn't support RTC should use the same error
> because application
> need the same fallback.
Ok. You're right. The technicality that the clockid is valid but
unsupported isn't really useful to the applications, since the effect is
the same.
What is the urgency on this? As the issue has been around since 3.0, is
it ok if it gets queued for 3.13 and marked for stable, or does it need
to land in 3.12?
thanks
-john
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] alarmtimer: return EINVAL instead of ENOTSUPP if rtcdev doesn't exist
2013-10-18 22:39 ` John Stultz
@ 2013-10-18 23:12 ` KOSAKI Motohiro
2013-10-18 23:29 ` John Stultz
0 siblings, 1 reply; 6+ messages in thread
From: KOSAKI Motohiro @ 2013-10-18 23:12 UTC (permalink / raw)
To: john.stultz, kosaki.motohiro; +Cc: linux-kernel, tglx, fweisbec
On 10/18/2013 6:39 PM, John Stultz wrote:
> On 10/17/2013 06:12 PM, KOSAKI Motohiro wrote:
>> (10/17/13 1:05 PM), John Stultz wrote:
>>> On 10/14/2013 02:33 PM, kosaki.motohiro@gmail.com wrote:
>>>> From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>>>>
>>>> Fedora Ruby maintainer reported latest Ruby doesn't work on Fedora
>>>> Rawhide
>>>> on ARM. (http://bugs.ruby-lang.org/issues/9008)
>>>>
>>>> Because of, commit 1c6b39ad3f (alarmtimers: Return -ENOTSUPP if no
>>>> RTC device is present) intruduced to return ENOTSUPP when
>>>> clock_get{time,res} can't find a RTC device. However it is incorrect.
>>>>
>>>> Posix and Linux man pages agree that clock_gettime and clock_getres
>>>> should return EINVAL if clk_id argument is invalid. This is significant
>>>> different from timer_create API.
>>>>
>>>> This patch fixes it.
>>>
>>> Hrm... So I feel like there is a difference here. The clockid for
>>> CLOCK_BOOTTIME_ALARM and CLOCK_REALTIME_ALARM are both valid.
>>>
>>> Its just that they're not supported on this specific hardware because it
>>> apparently lacks a RTC that has told the system it can be used as a
>>> wakeup device (Its actually quite likely on the hardware that the RTC
>>> can be a wakeup device, but that the driver is probably setting the
>>> wakeup flag after the RTC registered - so there is probably a driver bug
>>> here too).
>>>
>>> So I feel like in this case EINVAL isn't quite right. I'll admit it is
>>> somewhat new behavior, because we haven't had any clockids before that
>>> were dependent on the particular hardware, they either existed in a
>>> kernel verison or didn't.
>>>
>>> Would updating the manpage be a better route?
>>
>> Nope.
>>
>> ENOTSUPP is not exported to userland. ENOTSUP (single P) and EOPNOTSUP is
>> valid errno (and they are same on linux), but ENOTSUPP is a kernel
>> internal specific.
>>
>> Moreover, I completely disagree your position. Both
>> CLOCK_REALTIME_ALARM unsupported
>> kernel and ARM which doesn't support RTC should use the same error
>> because application
>> need the same fallback.
>
> Ok. You're right. The technicality that the clockid is valid but
> unsupported isn't really useful to the applications, since the effect is
> the same.
>
> What is the urgency on this? As the issue has been around since 3.0, is
> it ok if it gets queued for 3.13 and marked for stable, or does it need
> to land in 3.12?
3.13 is OK to me.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] alarmtimer: return EINVAL instead of ENOTSUPP if rtcdev doesn't exist
2013-10-18 23:12 ` KOSAKI Motohiro
@ 2013-10-18 23:29 ` John Stultz
0 siblings, 0 replies; 6+ messages in thread
From: John Stultz @ 2013-10-18 23:29 UTC (permalink / raw)
To: KOSAKI Motohiro, kosaki.motohiro; +Cc: linux-kernel, tglx, fweisbec
On 10/18/2013 04:12 PM, KOSAKI Motohiro wrote:
> On 10/18/2013 6:39 PM, John Stultz wrote:
>> On 10/17/2013 06:12 PM, KOSAKI Motohiro wrote:
>>> (10/17/13 1:05 PM), John Stultz wrote:
>>>> On 10/14/2013 02:33 PM, kosaki.motohiro@gmail.com wrote:
>>>>> From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>>>>>
>>>>> Fedora Ruby maintainer reported latest Ruby doesn't work on Fedora
>>>>> Rawhide
>>>>> on ARM. (http://bugs.ruby-lang.org/issues/9008)
>>>>>
>>>>> Because of, commit 1c6b39ad3f (alarmtimers: Return -ENOTSUPP if no
>>>>> RTC device is present) intruduced to return ENOTSUPP when
>>>>> clock_get{time,res} can't find a RTC device. However it is incorrect.
>>>>>
>>>>> Posix and Linux man pages agree that clock_gettime and clock_getres
>>>>> should return EINVAL if clk_id argument is invalid. This is significant
>>>>> different from timer_create API.
>>>>>
>>>>> This patch fixes it.
>>>> Hrm... So I feel like there is a difference here. The clockid for
>>>> CLOCK_BOOTTIME_ALARM and CLOCK_REALTIME_ALARM are both valid.
>>>>
>>>> Its just that they're not supported on this specific hardware because it
>>>> apparently lacks a RTC that has told the system it can be used as a
>>>> wakeup device (Its actually quite likely on the hardware that the RTC
>>>> can be a wakeup device, but that the driver is probably setting the
>>>> wakeup flag after the RTC registered - so there is probably a driver bug
>>>> here too).
>>>>
>>>> So I feel like in this case EINVAL isn't quite right. I'll admit it is
>>>> somewhat new behavior, because we haven't had any clockids before that
>>>> were dependent on the particular hardware, they either existed in a
>>>> kernel verison or didn't.
>>>>
>>>> Would updating the manpage be a better route?
>>> Nope.
>>>
>>> ENOTSUPP is not exported to userland. ENOTSUP (single P) and EOPNOTSUP is
>>> valid errno (and they are same on linux), but ENOTSUPP is a kernel
>>> internal specific.
>>>
>>> Moreover, I completely disagree your position. Both
>>> CLOCK_REALTIME_ALARM unsupported
>>> kernel and ARM which doesn't support RTC should use the same error
>>> because application
>>> need the same fallback.
>> Ok. You're right. The technicality that the clockid is valid but
>> unsupported isn't really useful to the applications, since the effect is
>> the same.
>>
>> What is the urgency on this? As the issue has been around since 3.0, is
>> it ok if it gets queued for 3.13 and marked for stable, or does it need
>> to land in 3.12?
> 3.13 is OK to me.
Ok. Applied to my 3.13 queue.
thanks
-john
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-18 23:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-14 21:33 [PATCH] alarmtimer: return EINVAL instead of ENOTSUPP if rtcdev doesn't exist kosaki.motohiro
2013-10-17 17:05 ` John Stultz
2013-10-18 1:12 ` KOSAKI Motohiro
2013-10-18 22:39 ` John Stultz
2013-10-18 23:12 ` KOSAKI Motohiro
2013-10-18 23:29 ` John Stultz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox