[Beignet] [PATCH] Use syslog for device self-test errors

[Yang, Rong R]

> -----Original Message-----
> From: Giuseppe Bilotta [mailto:giuseppe.bilotta at gmail.com]
> Sent: Monday, October 12, 2015 16:38
> To: Yang, Rong R
> Cc: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH] Use syslog for device self-test errors
> 
> On Mon, Oct 12, 2015 at 3:43 AM, Yang, Rong R <rong.r.yang at intel.com>
> wrote:
> > It is good idea print warnings to syslog avoid application confuse.
> > But for errors, I think also need print to stdout, give the clear hint to
> application.
> >
> > Any other consideration?
> 
> In my opinion a library should never produce (console) output unless
> explicitly requested to do so, so I think using syslog in any case is the correct
> approach. Maybe have some environment variable (OCL_VERBOSE or
> something like that) to control output to console?

I think it is worthy to produce some output if catch the known issues. It is helpful for application to locate the problem.
Or Maybe print in debug build only?

> 
> (In any case, even if the printf is preserved, it should at the very least be an
> fprintf(stderr, ...) instead)
Yes, agree with you.

> 
> >> -----Original Message-----
> >> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On
> >> Behalf Of Giuseppe Bilotta
> >> Sent: Sunday, September 27, 2015 19:47
> >> To: beignet at lists.freedesktop.org
> >> Cc: Giuseppe Bilotta
> >> Subject: [Beignet] [PATCH] Use syslog for device self-test errors
> >>
> >> ---
> >>  src/cl_device_id.c | 15 +++++++++++----
> >>  1 file changed, 11 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/src/cl_device_id.c b/src/cl_device_id.c index
> >> 78d2cf4..47bc772
> >> 100644
> >> --- a/src/cl_device_id.c
> >> +++ b/src/cl_device_id.c
> >> @@ -36,6 +36,8 @@
> >>  #include <stdlib.h>
> >>  #include <sys/sysinfo.h>
> >>
> >> +#include <syslog.h>
> >> +
> >>  #ifndef CL_VERSION_1_2
> >>  #define CL_DEVICE_BUILT_IN_KERNELS 0x103F  #endif @@ -611,7 +613,7
> >> @@ cl_self_test(cl_device_id device, cl_self_test_res atomic_in_l3_flag)
> >>                        ret = SELF_TEST_PASS;
> >>                      } else {
> >>                        ret = SELF_TEST_SLM_FAIL;
> >> -                      printf("Beignet: self-test failed: (3, 7, 5) + (5, 7, 3) returned
> >> (%i, %i, %i)\n"
> >> +                      syslog(LOG_ERR, "Beignet: self-test failed:
> >> + (3, 7, 5) + (5, 7, 3)
> >> returned (%i, %i, %i)\n"
> >>                               "See README.md or
> >> http://www.freedesktop.org/wiki/Software/Beignet/\n",
> >>                               test_data[0], test_data[1],
> >> test_data[2]);
> >>
> >> @@ -646,6 +648,8 @@ cl_get_device_ids(cl_platform_id    platform,
> >>  {
> >>    cl_device_id device;
> >>
> >> +  openlog("beignet", LOG_CONS, LOG_USER);
> >> +
> >>    /* Do we have a usable device? */
> >>    device = cl_get_gt_device();
> >>    if (device) {
> >> @@ -653,7 +657,7 @@ cl_get_device_ids(cl_platform_id    platform,
> >>      if (ret == SELF_TEST_ATOMIC_FAIL) {
> >>        device->atomic_test_result = ret;
> >>        ret = cl_self_test(device, ret);
> >> -      printf("Beignet: warning - disable atomic in L3 feature.\n");
> >> +      syslog(LOG_WARNING, "Beignet: Warning - disable atomic in L3
> >> + feature.\n");
> >>      }
> >>
> >>      if(ret == SELF_TEST_SLM_FAIL) {
> >> @@ -664,13 +668,16 @@ cl_get_device_ids(cl_platform_id    platform,
> >>          sscanf(env, "%i", &disable_self_test);
> >>        }
> >>        if (disable_self_test) {
> >> -        printf("Beignet: Warning - overriding self-test failure\n");
> >> +        syslog(LOG_WARNING, "Beignet: Warning - overriding self-test
> >> + failure\n");
> >>        } else {
> >> -        printf("Beignet: disabling non-working device\n");
> >> +        syslog(LOG_ERR, "Beignet: disabling non-working device\n");
> >>          device = 0;
> >>        }
> >>      }
> >>    }
> >> +
> >> +  closelog();
> >> +
> >>    if (!device) {
> >>      if (num_devices)
> >>        *num_devices = 0;
> >> --
> >> 2.6.0.rc2.233.g6dd8a9a.dirty
> >>
> >> _______________________________________________
> >> Beignet mailing list
> >> Beignet at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/beignet
> 
> 
> 
> --
> Giuseppe "Oblomov" Bilotta