From 39563824502e836a3213e61af15360e25d4f8431 Mon Sep 17 00:00:00 2001 From: Con Kolivas Date: Fri, 18 Oct 2013 22:04:21 +1100 Subject: [PATCH 1/7] Send pthread_cancel to failed completion_timeout that has timed out. --- util.c | 11 +++++++---- util.h | 3 +-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/util.c b/util.c index 48c92251..36bced24 100644 --- a/util.c +++ b/util.c @@ -2410,7 +2410,7 @@ void _cgsem_wait(cgsem_t *cgsem, const char *file, const char *func, const int l applog(LOG_WARNING, "Failed to read errno=%d" IN_FMT_FFL, errno, file, func, line); } -void _cgsem_destroy(cgsem_t *cgsem) +void cgsem_destroy(cgsem_t *cgsem) { close(cgsem->pipefd[1]); close(cgsem->pipefd[0]); @@ -2480,7 +2480,7 @@ int _cgsem_mswait(cgsem_t *cgsem, int ms, const char *file, const char *func, co return 0; } -void _cgsem_destroy(cgsem_t *cgsem) +void cgsem_destroy(cgsem_t *cgsem) { sem_destroy(cgsem); } @@ -2499,7 +2499,7 @@ void *completion_thread(void *arg) { struct cg_completion *cgc = (struct cg_completion *)arg; - pthread_detach(pthread_self()); + pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL); cgc->fn(cgc->fnarg); cgsem_post(&cgc->cgsem); @@ -2522,7 +2522,10 @@ bool cg_completion_timeout(void *fn, void *fnarg, int timeout) pthread_create(&pthread, NULL, completion_thread, (void *)cgc); ret = cgsem_mswait(&cgc->cgsem, timeout); - if (!ret) + if (!ret) { + pthread_join(pthread, NULL); free(cgc); + } else + pthread_cancel(pthread); return !ret; } diff --git a/util.h b/util.h index f258c40e..61fb3516 100644 --- a/util.h +++ b/util.h @@ -133,14 +133,13 @@ void _cgsem_init(cgsem_t *cgsem, const char *file, const char *func, const int l void _cgsem_post(cgsem_t *cgsem, const char *file, const char *func, const int line); void _cgsem_wait(cgsem_t *cgsem, const char *file, const char *func, const int line); int _cgsem_mswait(cgsem_t *cgsem, int ms, const char *file, const char *func, const int line); -void _cgsem_destroy(cgsem_t *cgsem); +void cgsem_destroy(cgsem_t *cgsem); bool cg_completion_timeout(void *fn, void *fnarg, int timeout); #define cgsem_init(_sem) _cgsem_init(_sem, __FILE__, __func__, __LINE__) #define cgsem_post(_sem) _cgsem_post(_sem, __FILE__, __func__, __LINE__) #define cgsem_wait(_sem) _cgsem_wait(_sem, __FILE__, __func__, __LINE__) #define cgsem_mswait(_sem, _timeout) _cgsem_mswait(_sem, _timeout, __FILE__, __func__, __LINE__) -#define cgsem_destroy(_sem) _cgsem_destroy(_sem) /* Align a size_t to 4 byte boundaries for fussy arches */ static inline void align_len(size_t *len) From 8e9f32a81b61096dec9feff914a07c34f4df6721 Mon Sep 17 00:00:00 2001 From: Con Kolivas Date: Fri, 18 Oct 2013 23:30:05 +1100 Subject: [PATCH 2/7] Add a forcelog variant of applog which invalidates any console lock to force output. --- cgminer.c | 2 +- logging.c | 12 +++++++++--- logging.h | 22 ++++++++++++++++------ usbutils.c | 2 +- 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/cgminer.c b/cgminer.c index fb739f2e..26482f1c 100644 --- a/cgminer.c +++ b/cgminer.c @@ -423,7 +423,7 @@ static void applog_and_exit(const char *fmt, ...) va_start(ap, fmt); vsnprintf(exit_buf, sizeof(exit_buf), fmt, ap); va_end(ap); - _applog(LOG_ERR, exit_buf); + _applog(LOG_ERR, exit_buf, true); exit(1); } diff --git a/logging.c b/logging.c index d00600f8..52eb9ef7 100644 --- a/logging.c +++ b/logging.c @@ -21,11 +21,17 @@ bool opt_log_output = false; /* per default priorities higher than LOG_NOTICE are logged */ int opt_log_level = LOG_NOTICE; -static void my_log_curses(int prio, const char *datetime, const char *str) +static void my_log_curses(int prio, const char *datetime, const char *str, bool force) { if (opt_quiet && prio != LOG_ERR) return; + /* Mutex could be locked by dead thread on shutdown so forcelog will + * invalidate any console lock status. */ + if (force) { + mutex_trylock(&console_lock); + mutex_unlock(&console_lock); + } #ifdef HAVE_CURSES extern bool use_curses; if (use_curses && log_curses_only(prio, datetime, str)) @@ -44,7 +50,7 @@ static void my_log_curses(int prio, const char *datetime, const char *str) /* * log function */ -void _applog(int prio, const char *str) +void _applog(int prio, const char *str, bool force) { #ifdef HAVE_SYSLOG_H if (use_syslog) { @@ -77,6 +83,6 @@ void _applog(int prio, const char *str) fflush(stderr); } - my_log_curses(prio, datetime, str); + my_log_curses(prio, datetime, str, force); } } diff --git a/logging.h b/logging.h index 2956452f..4d8a2501 100644 --- a/logging.h +++ b/logging.h @@ -28,7 +28,7 @@ extern int opt_log_level; #define LOGBUFSIZ 256 -extern void _applog(int prio, const char *str); +extern void _applog(int prio, const char *str, bool force); #define IN_FMT_FFL " in %s %s():%d" @@ -37,7 +37,7 @@ extern void _applog(int prio, const char *str); if (use_syslog || opt_log_output || prio <= opt_log_level) { \ char tmp42[LOGBUFSIZ]; \ snprintf(tmp42, sizeof(tmp42), fmt, ##__VA_ARGS__); \ - _applog(prio, tmp42); \ + _applog(prio, tmp42, false); \ } \ } \ } while (0) @@ -47,7 +47,17 @@ extern void _applog(int prio, const char *str); if (use_syslog || opt_log_output || prio <= opt_log_level) { \ char tmp42[_SIZ]; \ snprintf(tmp42, sizeof(tmp42), fmt, ##__VA_ARGS__); \ - _applog(prio, tmp42); \ + _applog(prio, tmp42, false); \ + } \ + } \ +} while (0) + +#define forcelog(prio, fmt, ...) do { \ + if (opt_debug || prio != LOG_DEBUG) { \ + if (use_syslog || opt_log_output || prio <= opt_log_level) { \ + char tmp42[LOGBUFSIZ]; \ + snprintf(tmp42, sizeof(tmp42), fmt, ##__VA_ARGS__); \ + _applog(prio, tmp42, true); \ } \ } \ } while (0) @@ -56,7 +66,7 @@ extern void _applog(int prio, const char *str); if (fmt) { \ char tmp42[LOGBUFSIZ]; \ snprintf(tmp42, sizeof(tmp42), fmt, ##__VA_ARGS__); \ - _applog(LOG_ERR, tmp42); \ + _applog(LOG_ERR, tmp42, true); \ } \ _quit(status); \ } while (0) @@ -66,7 +76,7 @@ extern void _applog(int prio, const char *str); char tmp42[LOGBUFSIZ]; \ snprintf(tmp42, sizeof(tmp42), fmt IN_FMT_FFL, \ ##__VA_ARGS__, __FILE__, __func__, __LINE__); \ - _applog(LOG_ERR, tmp42); \ + _applog(LOG_ERR, tmp42, true); \ } \ _quit(status); \ } while (0) @@ -76,7 +86,7 @@ extern void _applog(int prio, const char *str); char tmp42[LOGBUFSIZ]; \ snprintf(tmp42, sizeof(tmp42), fmt IN_FMT_FFL, \ ##__VA_ARGS__, _file, _func, _line); \ - _applog(LOG_ERR, tmp42); \ + _applog(LOG_ERR, tmp42, true); \ } \ _quit(status); \ } while (0) diff --git a/usbutils.c b/usbutils.c index 8f083d69..0a9c78f2 100644 --- a/usbutils.c +++ b/usbutils.c @@ -933,7 +933,7 @@ void usb_all(int level) for (i = 0; i < count; i++) usb_full(&j, list[i], &buf, &off, &len, level); - _applog(LOG_WARNING, buf); + _applog(LOG_WARNING, buf, false); free(buf); From 06776af000b098dad3d79b22b72881b485a5aba9 Mon Sep 17 00:00:00 2001 From: Con Kolivas Date: Fri, 18 Oct 2013 23:34:55 +1100 Subject: [PATCH 3/7] Use the forcelog function on shutdown to cope with indeterminate console lock states due to killing of threads. --- cgminer.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/cgminer.c b/cgminer.c index 26482f1c..336377ca 100644 --- a/cgminer.c +++ b/cgminer.c @@ -3159,7 +3159,7 @@ static void kill_mining(void) struct thr_info *thr; int i; - applog(LOG_DEBUG, "Killing off mining threads"); + forcelog(LOG_DEBUG, "Killing off mining threads"); /* Kill the mining threads*/ for (i = 0; i < mining_threads; i++) { pthread_t *pth = NULL; @@ -3186,29 +3186,29 @@ static void __kill_work(void) if (!successful_connect) return; - applog(LOG_INFO, "Received kill message"); + forcelog(LOG_INFO, "Received kill message"); #ifdef USE_USBUTILS /* Best to get rid of it first so it doesn't * try to create any new devices */ if (!opt_scrypt) { - applog(LOG_DEBUG, "Killing off HotPlug thread"); + forcelog(LOG_DEBUG, "Killing off HotPlug thread"); thr = &control_thr[hotplug_thr_id]; kill_timeout(thr); } #endif - applog(LOG_DEBUG, "Killing off watchpool thread"); + forcelog(LOG_DEBUG, "Killing off watchpool thread"); /* Kill the watchpool thread */ thr = &control_thr[watchpool_thr_id]; kill_timeout(thr); - applog(LOG_DEBUG, "Killing off watchdog thread"); + forcelog(LOG_DEBUG, "Killing off watchdog thread"); /* Kill the watchdog thread */ thr = &control_thr[watchdog_thr_id]; kill_timeout(thr); - applog(LOG_DEBUG, "Shutting down mining threads"); + forcelog(LOG_DEBUG, "Shutting down mining threads"); for (i = 0; i < mining_threads; i++) { struct cgpu_info *cgpu; @@ -3226,12 +3226,12 @@ static void __kill_work(void) cg_completion_timeout(&kill_mining, NULL, 3000); - applog(LOG_DEBUG, "Killing off stage thread"); + forcelog(LOG_DEBUG, "Killing off stage thread"); /* Stop the others */ thr = &control_thr[stage_thr_id]; kill_timeout(thr); - applog(LOG_DEBUG, "Killing off API thread"); + forcelog(LOG_DEBUG, "Killing off API thread"); thr = &control_thr[api_thr_id]; kill_timeout(thr); @@ -3239,10 +3239,10 @@ static void __kill_work(void) /* Release USB resources in case it's a restart * and not a QUIT */ if (!opt_scrypt) { - applog(LOG_DEBUG, "Releasing all USB devices"); + forcelog(LOG_DEBUG, "Releasing all USB devices"); cg_completion_timeout(&usb_cleanup, NULL, 1000); - applog(LOG_DEBUG, "Killing off usbres thread"); + forcelog(LOG_DEBUG, "Killing off usbres thread"); thr = &control_thr[usbres_thr_id]; kill_timeout(thr); } From d77f3672794c95e56fa73101cbe7a259afc4fc26 Mon Sep 17 00:00:00 2001 From: Con Kolivas Date: Fri, 18 Oct 2013 23:42:02 +1100 Subject: [PATCH 4/7] Fixing the memory leak for remaining semaphores means we can go back to using async transfers on other OSes with our own timeout management again. --- usbutils.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/usbutils.c b/usbutils.c index 0a9c78f2..2e160add 100644 --- a/usbutils.c +++ b/usbutils.c @@ -2296,15 +2296,9 @@ usb_bulk_transfer(struct libusb_device_handle *dev_handle, int intinfo, USBDEBUG("USB debug: @usb_bulk_transfer(%s (nodev=%s),intinfo=%d,epinfo=%d,data=%p,length=%d,timeout=%u,mode=%d,cmd=%s,seq=%d) endpoint=%d", cgpu->drv->name, bool_str(cgpu->usbinfo.nodev), intinfo, epinfo, data, length, timeout, mode, usb_cmdname(cmd), seq, (int)endpoint); init_usb_transfer(&ut); -#ifdef LINUX /* We give the transfer no timeout since we manage timeouts ourself */ libusb_fill_bulk_transfer(ut.transfer, dev_handle, endpoint, buf, length, transfer_callback, &ut, 0); -#else - /* All other OSes not so lucky */ - libusb_fill_bulk_transfer(ut.transfer, dev_handle, endpoint, buf, length, - transfer_callback, &ut, timeout); -#endif STATS_TIMEVAL(&tv_start); cg_rlock(&cgusb_fd_lock); err = libusb_submit_transfer(ut.transfer); @@ -2740,13 +2734,8 @@ static int usb_control_transfer(struct cgpu_info *cgpu, libusb_device_handle *de wIndex, wLength); if ((bmRequestType & LIBUSB_ENDPOINT_DIR_MASK) == LIBUSB_ENDPOINT_OUT) memcpy(buf + LIBUSB_CONTROL_SETUP_SIZE, buffer, wLength); -#ifdef LINUX libusb_fill_control_transfer(ut.transfer, dev_handle, buf, transfer_callback, &ut, 0); -#else - libusb_fill_control_transfer(ut.transfer, dev_handle, buf, transfer_callback, - &ut, timeout); -#endif err = libusb_submit_transfer(ut.transfer); if (!err) err = callback_wait(&ut, &transferred, timeout); From d58f2f0faa708b827e223c9e9cfdafa916cac124 Mon Sep 17 00:00:00 2001 From: Con Kolivas Date: Fri, 18 Oct 2013 23:45:19 +1100 Subject: [PATCH 5/7] Update NEWS. --- NEWS | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/NEWS b/NEWS index d4308524..f4d8f5bd 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,28 @@ +Version 3.6.4 - 18th October 2013 + +- Fixing the memory leak for remaining semaphores means we can go back to using +async transfers on other OSes with our own timeout management again. +- Use the forcelog function on shutdown to cope with indeterminate console lock +states due to killing of threads. +- Add a forcelog variant of applog which invalidates any console lock to force +output. +- Send pthread_cancel to failed completion_timeout that has timed out. +- Simplify queued hashtable by storing unqueued work separately in a single +pointer. +- bflsc use getinfo chip parallelization if it is present +- bflsc - fix brackets so [Chips] isn't always null +- Remove unused variables. +- Use cgcompletion timeouts for the unreliable shutdown functions on kill_work. +- Fix cgcompletion return code and free on successful completion. +- Provide a cg_completion_timeout helper function for unreliable functions that +takes arbitrary functions and parameters and reliably returns. +- Perform sync transfers on shutdown to allow final transfers to complete. +- Destroy cgsems used after transfers to not leave open files on osx. +- klondike rewrite work control +- allow __work_complete() access +- miner.h allow devices to tv_stamp work + + Version 3.6.3 - 17th October 2013 - API add 'MHS %ds' to 'summary' From 43699c76b7540007158719b0dd711ff8363494fb Mon Sep 17 00:00:00 2001 From: Con Kolivas Date: Fri, 18 Oct 2013 23:45:41 +1100 Subject: [PATCH 6/7] Bump version to 3.6.4 --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index fb66fe88..49c1d3b6 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ ##--##--##--##--##--##--##--##--##--##--##--##--##--##--##--##--## m4_define([v_maj], [3]) m4_define([v_min], [6]) -m4_define([v_mic], [3]) +m4_define([v_mic], [4]) ##--##--##--##--##--##--##--##--##--##--##--##--##--##--##--##--## m4_define([v_ver], [v_maj.v_min.v_mic]) m4_define([lt_rev], m4_eval(v_maj + v_min)) From eed0afcd0426824598770c46b3cc44ebe7d92748 Mon Sep 17 00:00:00 2001 From: Con Kolivas Date: Sat, 19 Oct 2013 13:29:59 +1100 Subject: [PATCH 7/7] Convert libusb transfer errors to regular libusb error messages to allow for accurate message reporting. --- usbutils.c | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/usbutils.c b/usbutils.c index 2e160add..963cc50e 100644 --- a/usbutils.c +++ b/usbutils.c @@ -2232,6 +2232,33 @@ static void LIBUSB_CALL transfer_callback(struct libusb_transfer *transfer) cgsem_post(&ut->cgsem); } +static int usb_transfer_toerr(int ret) +{ + switch (ret) { + default: + case LIBUSB_TRANSFER_COMPLETED: + ret = LIBUSB_SUCCESS; + break; + case LIBUSB_TRANSFER_ERROR: + ret = LIBUSB_ERROR_IO; + break; + case LIBUSB_TRANSFER_TIMED_OUT: + case LIBUSB_TRANSFER_CANCELLED: + ret = LIBUSB_ERROR_TIMEOUT; + break; + case LIBUSB_TRANSFER_STALL: + ret = LIBUSB_ERROR_PIPE; + break; + case LIBUSB_TRANSFER_NO_DEVICE: + ret = LIBUSB_ERROR_NO_DEVICE; + break; + case LIBUSB_TRANSFER_OVERFLOW: + ret = LIBUSB_ERROR_OVERFLOW; + break; + } + return ret; +} + /* Wait for callback function to tell us it has finished the USB transfer, but * use our own timer to cancel the request if we go beyond the timeout. */ static int callback_wait(struct usb_transfer *ut, int *transferred, unsigned int timeout) @@ -2248,8 +2275,7 @@ static int callback_wait(struct usb_transfer *ut, int *transferred, unsigned int cgsem_wait(&ut->cgsem); } ret = transfer->status; - if (ret == LIBUSB_TRANSFER_CANCELLED || ret == LIBUSB_TRANSFER_TIMED_OUT) - ret = LIBUSB_ERROR_TIMEOUT; + ret = usb_transfer_toerr(ret); /* No need to sort out mutexes here since they won't be reused */ *transferred = transfer->actual_length; @@ -2316,7 +2342,7 @@ usb_bulk_transfer(struct libusb_device_handle *dev_handle, int intinfo, cgpu->drv->name, cgpu->device_id, usb_cmdname(cmd), *transferred, err, errn); - if (err == LIBUSB_ERROR_PIPE || err == LIBUSB_TRANSFER_STALL) { + if (err == LIBUSB_ERROR_PIPE) { int retries = 0; do { @@ -2739,15 +2765,14 @@ static int usb_control_transfer(struct cgpu_info *cgpu, libusb_device_handle *de err = libusb_submit_transfer(ut.transfer); if (!err) err = callback_wait(&ut, &transferred, timeout); - if (err == LIBUSB_TRANSFER_COMPLETED && transferred) { + if (err == LIBUSB_SUCCESS && transferred) { if ((bmRequestType & LIBUSB_ENDPOINT_DIR_MASK) == LIBUSB_ENDPOINT_IN) memcpy(buffer, libusb_control_transfer_get_data(ut.transfer), transferred); err = transferred; goto out; } - if ((err) == LIBUSB_TRANSFER_CANCELLED) - err = LIBUSB_ERROR_TIMEOUT; + err = usb_transfer_toerr(err); out: complete_usb_transfer(&ut); return err;