> -----Original Message----- > From: Giuseppe Bilotta [mailto:[email protected]] > Sent: Monday, October 12, 2015 16:38 > To: Yang, Rong R > Cc: [email protected] > Subject: Re: [Beignet] [PATCH] Use syslog for device self-test errors > > On Mon, Oct 12, 2015 at 3:43 AM, Yang, Rong R <[email protected]> > 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:[email protected]] On > >> Behalf Of Giuseppe Bilotta > >> Sent: Sunday, September 27, 2015 19:47 > >> To: [email protected] > >> 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 > >> [email protected] > >> http://lists.freedesktop.org/mailman/listinfo/beignet > > > > -- > Giuseppe "Oblomov" Bilotta _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
