| Message ID | 20190819133618.7538-1-rpalethorpe@suse.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | tst_res: Print errno number in addition to error name | expand |
I think this patch makes sense. Only have tiny comments in below. Acked-by: Li Wang <liwang@redhat.com> On Mon, 19 Aug 2019 at 21:36, Richard Palethorpe <rpalethorpe@suse.com> wrote: > > Occasionally new error numbers are added to the kernel (maybe by > accident). Currently if we do not know the name of them then we just print > ???. > > This commit simply always prints the error number to aid with debugging. > > Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> > --- > > It appears we are atleast missing ENOTSUPP (524) which is probably returned by > create_timer for some alarm clocks on none x86 arches. This isn't entirely > clear, but what is clear is that it would help to know what the error number > is without using strace. > > lib/tst_test.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/lib/tst_test.c b/lib/tst_test.c > index 245e287fa..46481ca3f 100644 > --- a/lib/tst_test.c > +++ b/lib/tst_test.c > @@ -177,7 +177,7 @@ static void print_result(const char *file, const int lineno, int ttype, > { > char buf[1024]; > char *str = buf; > - int ret, size = sizeof(buf), ssize; > + int ret, size = sizeof(buf), ssize, int_errno; > const char *str_errno = NULL; > const char *res; > > @@ -205,15 +205,19 @@ static void print_result(const char *file, const int lineno, int ttype, > abort(); > } > > - if (ttype & TERRNO) > + if (ttype & TERRNO) { > str_errno = tst_strerrno(errno); > + int_errno = errno; Can we just reverse the order of these two statements for better typesetting? > + } > > - if (ttype & TTERRNO) > + if (ttype & TTERRNO) { > str_errno = tst_strerrno(TST_ERR); > + int_errno = TST_ERR; And this two. > + } > > if (ttype & TRERRNO) { > - ret = TST_RET < 0 ? -(int)TST_RET : (int)TST_RET; > - str_errno = tst_strerrno(ret); > + int_errno = TST_RET < 0 ? -(int)TST_RET : (int)TST_RET; > + str_errno = tst_strerrno(int_errno); > } > > ret = snprintf(str, size, "%s:%i: ", file, lineno); > @@ -237,7 +241,7 @@ static void print_result(const char *file, const int lineno, int ttype, > "Next message is too long and truncated:"); > } else if (str_errno) { > ssize = size - 2; > - ret = snprintf(str, size, ": %s", str_errno); > + ret = snprintf(str, size, ": %s (%d)", str_errno, int_errno); > str += MIN(ret, ssize); > size -= MIN(ret, ssize); > if (ret >= ssize) > -- > 2.22.0 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
Hi! > It appears we are atleast missing ENOTSUPP (524) which is probably returned by > create_timer for some alarm clocks on none x86 arches. This isn't entirely > clear, but what is clear is that it would help to know what the error number > is without using strace. Can we please also patch the lib/errnos.h so that the ENOTSUPP is included there as well?
Hi! > > It appears we are atleast missing ENOTSUPP (524) which is probably returned by > > create_timer for some alarm clocks on none x86 arches. This isn't entirely > > clear, but what is clear is that it would help to know what the error number > > is without using strace. > > Can we please also patch the lib/errnos.h so that the ENOTSUPP is > included there as well? And also patch the timer_create() tests to return TCONF if we got ENOTSUPP?
On Tue, Aug 20, 2019 at 5:56 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > > > It appears we are atleast missing ENOTSUPP (524) which is probably returned by > > > create_timer for some alarm clocks on none x86 arches. This isn't entirely > > > clear, but what is clear is that it would help to know what the error number > > > is without using strace. > > > > Can we please also patch the lib/errnos.h so that the ENOTSUPP is > > included there as well? > > And also patch the timer_create() tests to return TCONF if we got > ENOTSUPP? I think we shouldn't TCONF this until we get clear the problem. This failure very probably a bug for kernel: # grep RTC_CLASS /boot/config-4.18.0-128.el8.aarch64 CONFIG_RTC_CLASS=y # ./timer_create01 tst_test.c:1104: INFO: Timeout per run is 0h 05m 00s ... timer_create01.c:87: FAIL: Failed to create timer for CLOCK_BOOTTIME_ALARM: ??? (524) timer_create01.c:87: FAIL: Failed to create timer for CLOCK_REALTIME_ALARM: ??? (524) # cat kernel/time/alarmtimer.c -n ... 57 #ifdef CONFIG_RTC_CLASS ... 72 struct rtc_device *alarmtimer_get_rtcdev(void) 73 { 74 unsigned long flags; 75 struct rtc_device *ret; 76 77 spin_lock_irqsave(&rtcdev_lock, flags); 78 ret = rtcdev; 79 spin_unlock_irqrestore(&rtcdev_lock, flags); 80 81 return ret; 82 } 83 EXPORT_SYMBOL_GPL(alarmtimer_get_rtcdev); ... 140 #else 141 struct rtc_device *alarmtimer_get_rtcdev(void) 142 { 143 return NULL; 144 } ... 670 static int alarm_timer_create(struct k_itimer *new_timer) 671 { 672 enum alarmtimer_type type; 673 674 if (!alarmtimer_get_rtcdev()) 675 return -ENOTSUPP; 676 677 if (!capable(CAP_WAKE_ALARM)) 678 return -EPERM; 679 680 type = clock2alarm(new_timer->it_clock); 681 alarm_init(&new_timer->it.alarm.alarmtimer, type, alarm_handle_timer); 682 return 0; 683 }
On Tue, Aug 20, 2019 at 5:55 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > > It appears we are atleast missing ENOTSUPP (524) which is probably returned by > > create_timer for some alarm clocks on none x86 arches. This isn't entirely > > clear, but what is clear is that it would help to know what the error number > > is without using strace. > > Can we please also patch the lib/errnos.h so that the ENOTSUPP is > included there as well? Not sure if we should do that since it(from google) says ENOTSUPP is not intended to used in userland. But if we just for catching more errors to report in LTP, I also think it does not hurt anything.
Hi! > > > It appears we are atleast missing ENOTSUPP (524) which is probably returned by > > > create_timer for some alarm clocks on none x86 arches. This isn't entirely > > > clear, but what is clear is that it would help to know what the error number > > > is without using strace. > > > > Can we please also patch the lib/errnos.h so that the ENOTSUPP is > > included there as well? > > Not sure if we should do that since it(from google) says ENOTSUPP is > not intended to used in userland. Looks like there are two of these ENOTSUP and ENOTSUPP so the syscall returning ENOTSUPP is a bug itself since it should return ENOTSUP. Also btw ENOTSUP == EOPNOTSUPP and we do have EOPNOTSUPP in errnos.h.
Hi! > > > > It appears we are atleast missing ENOTSUPP (524) which is probably returned by > > > > create_timer for some alarm clocks on none x86 arches. This isn't entirely > > > > clear, but what is clear is that it would help to know what the error number > > > > is without using strace. > > > > > > Can we please also patch the lib/errnos.h so that the ENOTSUPP is > > > included there as well? > > > > And also patch the timer_create() tests to return TCONF if we got > > ENOTSUPP? > > I think we shouldn't TCONF this until we get clear the problem. Indeed, if nothing else it should return ENOTSUP/EOPNOTSUPP instead.
Hello, Cyril Hrubis <chrubis@suse.cz> writes: > Hi! >> > > It appears we are atleast missing ENOTSUPP (524) which is probably returned by >> > > create_timer for some alarm clocks on none x86 arches. This isn't entirely >> > > clear, but what is clear is that it would help to know what the error number >> > > is without using strace. >> > >> > Can we please also patch the lib/errnos.h so that the ENOTSUPP is >> > included there as well? >> >> Not sure if we should do that since it(from google) says ENOTSUPP is >> not intended to used in userland. I suppose we could print a warning/fail if any error value over 512 (ERESTARTSYS) is seen by tst_res(). We can define these error numbers in user land for debugging purposes. > > Looks like there are two of these ENOTSUP and ENOTSUPP so the syscall > returning ENOTSUPP is a bug itself since it should return ENOTSUP. Also > btw ENOTSUP == EOPNOTSUPP and we do have EOPNOTSUPP in errnos.h. I think there is quite a lot of stuff missing. Maybe it is time to create a script for generating this list? -- Thank you, Richard.
Hi! > >> > > It appears we are atleast missing ENOTSUPP (524) which is probably returned by > >> > > create_timer for some alarm clocks on none x86 arches. This isn't entirely > >> > > clear, but what is clear is that it would help to know what the error number > >> > > is without using strace. > >> > > >> > Can we please also patch the lib/errnos.h so that the ENOTSUPP is > >> > included there as well? > >> > >> Not sure if we should do that since it(from google) says ENOTSUPP is > >> not intended to used in userland. > > I suppose we could print a warning/fail if any error value over 512 > (ERESTARTSYS) is seen by tst_res(). We can define these error numbers in > user land for debugging purposes. Sounds good to me, I wouldn't do it a fail, even simple TINFO message that would say that we got unexpected errno. And we should btw do that for the tst_strerrno() as well. > > Looks like there are two of these ENOTSUP and ENOTSUPP so the syscall > > returning ENOTSUPP is a bug itself since it should return ENOTSUP. Also > > btw ENOTSUP == EOPNOTSUPP and we do have EOPNOTSUPP in errnos.h. > > I think there is quite a lot of stuff missing. Maybe it is time to > create a script for generating this list? We can but I would just commit the resulting header to LTP since the list will not change frequently or at all.
Hi! Pushed, thanks.
diff --git a/lib/tst_test.c b/lib/tst_test.c index 245e287fa..46481ca3f 100644 --- a/lib/tst_test.c +++ b/lib/tst_test.c @@ -177,7 +177,7 @@ static void print_result(const char *file, const int lineno, int ttype, { char buf[1024]; char *str = buf; - int ret, size = sizeof(buf), ssize; + int ret, size = sizeof(buf), ssize, int_errno; const char *str_errno = NULL; const char *res; @@ -205,15 +205,19 @@ static void print_result(const char *file, const int lineno, int ttype, abort(); } - if (ttype & TERRNO) + if (ttype & TERRNO) { str_errno = tst_strerrno(errno); + int_errno = errno; + } - if (ttype & TTERRNO) + if (ttype & TTERRNO) { str_errno = tst_strerrno(TST_ERR); + int_errno = TST_ERR; + } if (ttype & TRERRNO) { - ret = TST_RET < 0 ? -(int)TST_RET : (int)TST_RET; - str_errno = tst_strerrno(ret); + int_errno = TST_RET < 0 ? -(int)TST_RET : (int)TST_RET; + str_errno = tst_strerrno(int_errno); } ret = snprintf(str, size, "%s:%i: ", file, lineno); @@ -237,7 +241,7 @@ static void print_result(const char *file, const int lineno, int ttype, "Next message is too long and truncated:"); } else if (str_errno) { ssize = size - 2; - ret = snprintf(str, size, ": %s", str_errno); + ret = snprintf(str, size, ": %s (%d)", str_errno, int_errno); str += MIN(ret, ssize); size -= MIN(ret, ssize); if (ret >= ssize)
Occasionally new error numbers are added to the kernel (maybe by accident). Currently if we do not know the name of them then we just print ???. This commit simply always prints the error number to aid with debugging. Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> --- It appears we are atleast missing ENOTSUPP (524) which is probably returned by create_timer for some alarm clocks on none x86 arches. This isn't entirely clear, but what is clear is that it would help to know what the error number is without using strace. lib/tst_test.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)