Fix threading issues and bugs

api.c

@@ -34,11 +34,11 @@
 // Current code assumes it can socket send this size also
 #define MYBUFSIZ	16384
 
-// Number of requets to queue - normally would be small
+// Number of requests to queue - normally would be small
-#define QUEUE	30
+#define QUEUE	10
 
-static char *buffer = NULL;
+static char *io_buffer = NULL;
-static char error_buffer[BUFSIZ];
+static char *msg_buffer = NULL;
 
 static const char *UNAVAILABLE = " - API will not be available";
 
@@ -60,6 +60,11 @@ static const char *SEPARATORSTR = "|";
 #define MSG_GPUDIS 12
 #define MSG_GPUREI 13
 #define MSG_INVCMD 14
+#define MSG_MISID 15
+#define MSG_CPUNON 16
+#define MSG_GPUDEV 17
+#define MSG_CPUDEV 18
+#define MSG_INVCPU 19
 
 enum code_severity {
 	SEVERITY_ERR,
@@ -71,16 +76,18 @@ enum code_severity {
 
 enum code_parameters {
 	PARAM_GPU,
-	PARAM_NONE,
+	PARAM_CPU,
 	PARAM_GPUMAX,
+	PARAM_CPUMAX,
 	PARAM_PMAX,
 	PARAM_GCMAX,
+	PARAM_NONE
 };
 
 struct CODES {
-	enum code_severity severity;
+	const enum code_severity severity;
-	int code;
+	const int code;
-	enum code_parameters params;
+	const enum code_parameters params;
 	const char *description;
 } codes[] = {
  { SEVERITY_ERR,   MSG_INVGPU,	PARAM_GPUMAX,	"Invalid GPU id %d - range is 0 - %d" },
@@ -88,15 +95,20 @@ struct CODES {
  { SEVERITY_INFO,  MSG_ALRDIS,	PARAM_GPU,	"GPU %d already disabled" },
  { SEVERITY_WARN,  MSG_GPUMRE,	PARAM_GPU,	"GPU %d must be restarted first" },
  { SEVERITY_INFO,  MSG_GPUREN,	PARAM_GPU,	"GPU %d sent enable message" },
- { SEVERITY_WARN,  MSG_GPUNON,	PARAM_NONE,	"No GPUs" },
+ { SEVERITY_ERR,   MSG_GPUNON,	PARAM_NONE,	"No GPUs" },
- { SEVERITY_SUCC,  MSG_POOL,	PARAM_PMAX,	"%d Pool" },
+ { SEVERITY_SUCC,  MSG_POOL,	PARAM_PMAX,	"%d Pool(s)" },
- { SEVERITY_WARN,  MSG_NOPOOL,	PARAM_NONE,	"No pools" },
+ { SEVERITY_ERR,   MSG_NOPOOL,	PARAM_NONE,	"No pools" },
- { SEVERITY_SUCC,  MSG_DEVS,	PARAM_GCMAX,	"%d GPU - %d CPU" },
+ { SEVERITY_SUCC,  MSG_DEVS,	PARAM_GCMAX,	"%d GPU(s) - %d CPU(s)" },
- { SEVERITY_WARN,  MSG_NODEVS,	PARAM_NONE,	"No GPUs/CPUs" },
+ { SEVERITY_ERR,   MSG_NODEVS,	PARAM_NONE,	"No GPUs/CPUs" },
  { SEVERITY_SUCC,  MSG_SUMM,	PARAM_NONE,	"Summary" },
  { SEVERITY_INFO,  MSG_GPUDIS,	PARAM_GPU,	"GPU %d set disable flag" },
  { SEVERITY_INFO,  MSG_GPUREI,	PARAM_GPU,	"GPU %d restart attempted" },
  { SEVERITY_ERR,   MSG_INVCMD,	PARAM_NONE,	"Invalid command" },
+ { SEVERITY_ERR,   MSG_MISID,	PARAM_NONE,	"Missing device id parameter" },
+ { SEVERITY_ERR,   MSG_CPUNON,	PARAM_NONE,	"No CPUs" },
+ { SEVERITY_SUCC,  MSG_GPUDEV,	PARAM_GPU,	"GPU%d" },
+ { SEVERITY_SUCC,  MSG_CPUDEV,	PARAM_CPU,	"CPU%d" },
+ { SEVERITY_ERR,   MSG_INVCPU,	PARAM_CPUMAX,	"Invalid CPU id %d - range is 0 - %d" },
  { SEVERITY_FAIL }
 };
 
@@ -118,6 +130,7 @@ static char *message(int messageid, int gpuid)
 {
 	char severity;
 	char *ptr;
+	int cpu;
 	int i;
 
 	for (i = 0; codes[i].severity != SEVERITY_FAIL; i++) {
@@ -138,58 +151,62 @@ static char *message(int messageid, int gpuid)
 				break;
 			}
 
-			sprintf(error_buffer, "STATUS=%c,CODE=%d,MSG=",
+			sprintf(msg_buffer, "STATUS=%c,CODE=%d,MSG=", severity, messageid);
-					severity, messageid);
 
-			ptr = error_buffer + strlen(error_buffer);
+			ptr = msg_buffer + strlen(msg_buffer);
 
 			switch(codes[i].params) {
 			case PARAM_GPU:
 				sprintf(ptr, codes[i].description, gpuid);
 				break;
+			case PARAM_CPU:
+				sprintf(ptr, codes[i].description, gpuid);
+				break;
 			case PARAM_GPUMAX:
-				sprintf(ptr, codes[i].description,
+				sprintf(ptr, codes[i].description, gpuid, nDevs - 1);
-						gpuid, gpu_threads - 1);
 				break;
 			case PARAM_PMAX:
 				sprintf(ptr, codes[i].description, total_pools);
 				break;
 			case PARAM_GCMAX:
-				sprintf(ptr, codes[i].description,
+				if (opt_n_threads > 0)
-						gpu_threads,
+					cpu = num_processors;
-						(mining_threads - gpu_threads));
+				else
+					cpu = 0;
+
+				sprintf(ptr, codes[i].description, nDevs, cpu);
 				break;
 			case PARAM_NONE:
 			default:
 				strcpy(ptr, codes[i].description);
 			}
 
-			strcat(error_buffer, SEPARATORSTR);
+			strcat(msg_buffer, SEPARATORSTR);
 
-			return error_buffer;
+			return msg_buffer;
 		}
 	}
 
-	sprintf(error_buffer, "STATUS=F,CODE=-1,MSG=%d%c", messageid, SEPARATOR);
+	sprintf(msg_buffer, "STATUS=F,CODE=-1,MSG=%d%c", messageid, SEPARATOR);
-	return error_buffer;
+	return msg_buffer;
 }
 
-char *apiversion(char *params)
+void apiversion(char *params)
 {
-	return (char *)APIVERSION;
+	strcpy(io_buffer, APIVERSION);
 }
 
-void gpustatus(int thr_id)
+void gpustatus(int gpu)
 {
-	char intensity[10];
+	char intensity[20];
 	char buf[BUFSIZ];
 	char *enabled;
 	char *status;
 	float gt;
 	int gf, gp;
+	int i;
 
-	if (thr_id >= 0 && thr_id < gpu_threads) {
+	if (gpu >= 0 && gpu < nDevs) {
-		int gpu = thr_info[thr_id].cgpu->cpu_gpu;
 		struct cgpu_info *cgpu = &gpus[gpu];
 
 		cgpu->utility = cgpu->accepted / ( total_secs ? total_secs : 1 ) * 60;
@@ -229,16 +246,15 @@ void gpustatus(int thr_id)
 			cgpu->accepted, cgpu->rejected, cgpu->hw_errors,
 			cgpu->utility, intensity, SEPARATOR);
 
-		strcat(buffer, buf);
+		strcat(io_buffer, buf);
 	}
 }
 
-void cpustatus(int thr_id)
+void cpustatus(int cpu)
 {
 	char buf[BUFSIZ];
 
-	if (thr_id >= gpu_threads) {
+	if (opt_n_threads > 0 && cpu >= 0 && cpu < num_processors) {
-		int cpu = thr_info[thr_id].cgpu->cpu_gpu;
 		struct cgpu_info *cgpu = &cpus[cpu];
 
 		cgpu->utility = cgpu->accepted / ( total_secs ? total_secs : 1 ) * 60;
@@ -249,38 +265,91 @@ void cpustatus(int thr_id)
 			cgpu->accepted, cgpu->rejected,
 			cgpu->utility, SEPARATOR);
 
-		strcat(buffer, buf);
+		strcat(io_buffer, buf);
 	}
 }
 
-char *devstatus(char *params)
+void devstatus(char *params)
 {
 	int i;
 
-	if (gpu_threads == 0 && mining_threads == 0)
+	if (nDevs == 0 && num_processors == 0) {
-		return message(MSG_NODEVS, 0);
+		strcpy(io_buffer, message(MSG_NODEVS, 0));
+		return;
+	}
 
-	strcpy(buffer, message(MSG_DEVS, 0));
+	strcpy(io_buffer, message(MSG_DEVS, 0));
 
-	for (i = 0; i < gpu_threads; i++)
+	for (i = 0; i < nDevs; i++)
 		gpustatus(i);
 
-	for (i = gpu_threads; i < mining_threads; i++)
+	if (opt_n_threads > 0)
-		cpustatus(i);
+		for (i = 0; i < num_processors; i++)
-
+			cpustatus(i);
-	return buffer;
 }
 
-char *poolstatus(char *params)
+void gpudev(char *params)
+{
+	int id;
+
+	if (nDevs == 0) {
+		strcpy(io_buffer, message(MSG_GPUNON, 0));
+		return;
+	}
+
+	if (*params == '\0') {
+		strcpy(io_buffer, message(MSG_MISID, 0));
+		return;
+	}
+
+	id = atoi(params);
+	if (id < 0 || id >= nDevs) {
+		strcpy(io_buffer, message(MSG_INVGPU, id));
+		return;
+	}
+
+	strcpy(io_buffer, message(MSG_GPUDEV, id));
+
+	gpustatus(id);
+}
+
+void cpudev(char *params)
+{
+	int id;
+
+	if (opt_n_threads == 0) {
+		strcpy(io_buffer, message(MSG_CPUNON, 0));
+		return;
+	}
+
+	if (*params == '\0') {
+		strcpy(io_buffer, message(MSG_MISID, 0));
+		return;
+	}
+
+	id = atoi(params);
+	if (id < 0 || id >= num_processors) {
+		strcpy(io_buffer, message(MSG_INVCPU, id));
+		return;
+	}
+
+	strcpy(io_buffer, message(MSG_CPUDEV, id));
+
+	cpustatus(id);
+}
+
+void poolstatus(char *params)
 {
 	char buf[BUFSIZ];
 	char *status, *lp;
 	int i;
 
-	if (total_pools == 0)
+	if (total_pools == 0) {
-		return message(MSG_NOPOOL, 0);
+		strcpy(io_buffer, message(MSG_NOPOOL, 0));
+		return;
+	}
 
-	strcpy(buffer, message(MSG_POOL, 0));
+	strcpy(io_buffer, message(MSG_POOL, 0));
 
 	for (i = 0; i < total_pools; i++) {
 		struct pool *pool = pools[i];
@@ -309,108 +378,136 @@ char *poolstatus(char *params)
 			pool->getfail_occasions,
 			pool->remotefail_occasions, SEPARATOR);
 
-		strcat(buffer, buf);
+		strcat(io_buffer, buf);
 	}
-
-	return buffer;
 }
 
-char *summary(char *params)
+void summary(char *params)
 {
 	double utility, mhs;
 
 	utility = total_accepted / ( total_secs ? total_secs : 1 ) * 60;
 	mhs = total_mhashes_done / total_secs;
 
-	sprintf(buffer, "%sSUMMARY=all,EL=%.0lf,ALGO=%s,MHS=%.2lf,SOL=%d,Q=%d,A=%d,R=%d,HW=%d,U=%.2lf,DW=%d,ST=%d,GF=%d,LW=%d,RO=%d,BC=%d%c",
+	sprintf(io_buffer, "%sSUMMARY=all,EL=%.0lf,ALGO=%s,MHS=%.2lf,SOL=%d,Q=%d,A=%d,R=%d,HW=%d,U=%.2lf,DW=%d,ST=%d,GF=%d,LW=%d,RO=%d,BC=%d%c",
 		message(MSG_SUMM, 0),
 		total_secs, algo_names[opt_algo], mhs, found_blocks,
 		total_getworks, total_accepted, total_rejected,
 		hw_errors, utility, total_discarded, total_stale,
 		total_go, local_work, total_ro, new_blocks, SEPARATOR);
-
-	return buffer;
 }
 
-char *gpuenable(char *params)
+void gpuenable(char *params)
 {
 	struct thr_info *thr;
 	int gpu;
 	int id;
 	int i;
 
-	if (gpu_threads == 0)
+	if (gpu_threads == 0) {
-		return message(MSG_GPUNON, 0);
+		strcpy(io_buffer, message(MSG_GPUNON, 0));
+		return;
+	}
+
+	if (*params == '\0') {
+		strcpy(io_buffer, message(MSG_MISID, 0));
+		return;
+	}
 
 	id = atoi(params);
-	if (id < 0 || id >= gpu_threads)
+	if (id < 0 || id >= nDevs) {
-		return message(MSG_INVGPU, id);
+		strcpy(io_buffer, message(MSG_INVGPU, id));
+		return;
+	}
 
-	if (gpu_devices[id])
+	if (gpu_devices[id]) {
-		return message(MSG_ALRENA, id);
+		strcpy(io_buffer, message(MSG_ALRENA, id));
+		return;
+	}
 
 	for (i = 0; i < gpu_threads; i++) {
 		gpu = thr_info[i].cgpu->cpu_gpu;
 		if (gpu == id) {
 			thr = &thr_info[i];
-			if (thr->cgpu->status != LIFE_WELL)
+			if (thr->cgpu->status != LIFE_WELL) {
-				return message(MSG_GPUMRE, id);
+				strcpy(io_buffer, message(MSG_GPUMRE, id));
+				return;
+			}
 
 			gpu_devices[id] = true;
 			tq_push(thr->q, &ping);
 
-			return message(MSG_GPUREN, id);
 		}
 	}
 
-	return message(-2, 0);
+	strcpy(io_buffer, message(MSG_GPUREN, id));
 }
 
-char *gpudisable(char *params)
+void gpudisable(char *params)
 {
 	int id;
 
-	if (gpu_threads == 0)
+	if (nDevs == 0) {
-		return message(MSG_GPUNON, 0);
+		strcpy(io_buffer, message(MSG_GPUNON, 0));
+		return;
+	}
+
+	if (*params == '\0') {
+		strcpy(io_buffer, message(MSG_MISID, 0));
+		return;
+	}
 
 	id = atoi(params);
-	if (id < 0 || id >= gpu_threads)
+	if (id < 0 || id >= nDevs) {
-		return message(MSG_INVGPU, id);
+		strcpy(io_buffer, message(MSG_INVGPU, id));
+		return;
+	}
 
-	if (!gpu_devices[id])
+	if (!gpu_devices[id]) {
-		return message(MSG_ALRDIS, id);
+		strcpy(io_buffer, message(MSG_ALRDIS, id));
+		return;
+	}
 
 	gpu_devices[id] = false;
 
-	return message(MSG_GPUDIS, id);
+	strcpy(io_buffer, message(MSG_GPUDIS, id));
 }
 
-char *gpurestart(char *params)
+void gpurestart(char *params)
 {
 	int id;
 
-	if (gpu_threads == 0)
+	if (nDevs == 0) {
-		return message(MSG_GPUNON, 0);
+		strcpy(io_buffer, message(MSG_GPUNON, 0));
+		return;
+	}
+
+	if (*params == '\0') {
+		strcpy(io_buffer, message(MSG_MISID, 0));
+		return;
+	}
 
 	id = atoi(params);
-	if (id < 0 || id >= gpu_threads)
+	if (id < 0 || id >= nDevs) {
-		return message(MSG_INVGPU, id);
+		strcpy(io_buffer, message(MSG_INVGPU, id));
+		return;
+	}
 
 	reinit_device(&gpus[id]);
 
-	return message(MSG_GPUREI, id);
+	strcpy(io_buffer, message(MSG_GPUREI, id));
 }
 
-char *doquit(char *params)
+void doquit(char *params)
 {
+	*io_buffer = '\0';
 	bye = 1;
 	kill_work();
-	return NULL;
 }
 
 struct CMDS {
 	char *name;
-	char *(*func)(char *);
+	void (*func)(char *);
 } cmds[] = {
 	{ "apiversion",	apiversion },
 	{ "dev",	devstatus },
@@ -419,37 +516,35 @@ struct CMDS {
 	{ "gpuenable",	gpuenable },
 	{ "gpudisable",	gpudisable },
 	{ "gpurestart",	gpurestart },
+	{ "gpu",	gpudev },
+	{ "cpu",	cpudev },
 	{ "quit",	doquit },
-	{ NULL,		NULL }
+	{ NULL }
 };
 
-void send_result(int c, char *result)
+void send_result(int c)
 {
 	int n;
 
-	if (result == NULL)
-		result = (char *)BLANK;
-
 	// ignore failure - it's closed immediately anyway
-	n = write(c, result, strlen(result)+1);
+	n = write(c, io_buffer, strlen(io_buffer)+1);
 }
 
 void api(void)
 {
 	char buf[BUFSIZ];
-	const char *addr = "127.0.0.1";
+	const char *localaddr = "127.0.0.1";
 	int c, sock, n, bound;
-	char tmpaddr[32];
+	char connectaddr[32];
 	char *binderror;
 	time_t bindstart;
 	short int port = opt_api_port;
 	struct sockaddr_in serv;
 	struct sockaddr_in cli;
 	socklen_t clisiz;
-	long long counter;
-	char *result;
 	char *params;
-	int did;
+	bool portok;
+	bool did;
 	int i;
 
 	sock = socket(AF_INET, SOCK_STREAM, 0);
@@ -461,12 +556,14 @@ void api(void)
 	memset(&serv, 0, sizeof(serv));
 
 	serv.sin_family = AF_INET;
+
 	if (!opt_api_listen) {
-		if (inet_pton(AF_INET, addr, &(serv.sin_addr)) == 0) {
+		if (inet_pton(AF_INET, localaddr, &(serv.sin_addr)) == 0) {
 			applog(LOG_ERR, "API2 initialisation failed (%s)%s", strerror(errno), UNAVAILABLE);
 			return;
 		}
 	}
+
 	serv.sin_port = htons(port);
 
 	// try for 1 minute ... in case the old one hasn't completely gone yet
@@ -497,8 +594,6 @@ void api(void)
 		return;
 	}
 
-	buffer = malloc(MYBUFSIZ+1);
-
 	sleep(opt_log_interval);
 
 	if (opt_api_listen)
@@ -506,45 +601,53 @@ void api(void)
 	else
 		applog(LOG_WARNING, "API running in restricted access mode");
 
-	counter = 0;
+	io_buffer = malloc(MYBUFSIZ+1);
-	while (bye == 0) {
+	msg_buffer = malloc(MYBUFSIZ+1);
-		counter++;
 
+	while (bye == 0) {
 		clisiz = sizeof(cli);
 		if ((c = accept(sock, (struct sockaddr *)(&cli), &clisiz)) < 0) {
 			applog(LOG_ERR, "API failed (%s)%s", strerror(errno), UNAVAILABLE);
-			close(sock);
+			goto die;
-			free(buffer);
-			return;
 		}
 
-		if (!opt_api_listen)
+		if (opt_api_listen)
-			inet_ntop(AF_INET, &(cli.sin_addr), &(tmpaddr[0]), sizeof(tmpaddr)-1);
+			portok = true;
+		else {
+			inet_ntop(AF_INET, &(cli.sin_addr), &(connectaddr[0]), sizeof(connectaddr)-1);
+			portok = (strcmp(connectaddr, localaddr) == 0);
+		}
 
-		if (opt_api_listen || strcmp(tmpaddr, addr) == 0) {
+		if (portok) {
 			n = read(c, &buf[0], BUFSIZ-1);
 			if (n >= 0) {
 				did = false;
 				buf[n] = '\0';
 				params = strchr(buf, SEPARATOR);
-				if (params != NULL)
+				if (params == NULL)
+					params = (char *)BLANK;
+				else
 					*(params++) = '\0';
 
 				for (i = 0; cmds[i].name != NULL; i++) {
 					if (strcmp(buf, cmds[i].name) == 0) {
-						result = (cmds[i].func)(params);
+						(cmds[i].func)(params);
-						send_result(c, result);
+						send_result(c);
 						did = true;
 						break;
 					}
 				}
-				if (!did)
+				if (!did) {
-					send_result(c, message(MSG_INVCMD, 0));
+					strcpy(io_buffer, message(MSG_INVCMD, 0));
+					send_result(c);
+				}
 			}
 		}
 		close(c);
 	}
+die:
+	free(msg_buffer);
+	free(io_buffer);
 
 	close(sock);
-	free(buffer);
 }

main.c

@@ -203,16 +203,16 @@ enum sha256_algos opt_algo = ALGO_SSE2_32;
 #else
 enum sha256_algos opt_algo = ALGO_C;
 #endif
-static int nDevs;
+int nDevs;
 static int opt_g_threads = 2;
 static int opt_device;
 static int total_devices;
 bool gpu_devices[MAX_GPUDEVICES];
 int gpu_threads;
 static bool forced_n_threads;
-static int opt_n_threads;
+int opt_n_threads;
 int mining_threads;
-static int num_processors;
+int num_processors;
 bool use_curses = true;
 static bool opt_submit_stale;
 static bool opt_nogpu;
@@ -1502,7 +1502,7 @@ static struct opt_table opt_config_table[] = {
 			"Enable API to listen on/for any address, default: only 127.0.0.1"),
 	OPT_WITH_ARG("--api-port",
 		     set_int_1_to_65535, opt_show_intval, &opt_api_port,
-		     "Port number of miner API, default: 4028"),
+		     "Port number of miner API"),
 #ifdef HAVE_ADL
 	OPT_WITHOUT_ARG("--auto-fan",
 			opt_set_bool, &opt_autofan,
@@ -2542,6 +2542,11 @@ void kill_work(void)
 		applog(LOG_DEBUG, "Killing off work thread");
 	thr = &thr_info[work_thr_id];
 	thr_info_cancel(thr);
+
+	if (opt_debug)
+		applog(LOG_DEBUG, "Killing off API thread");
+	thr = &thr_info[api_thr_id];
+	thr_info_cancel(thr);
 }
 
 void quit(int status, const char *format, ...);
@@ -5840,7 +5845,7 @@ int main (int argc, char *argv[])
 
 	mining_threads = opt_n_threads + gpu_threads;
 
-	total_threads = mining_threads + 7;
+	total_threads = mining_threads + 8;
 	work_restart = calloc(total_threads, sizeof(*work_restart));
 	if (!work_restart)
 		quit(1, "Failed to calloc work_restart");

miner.h

@@ -419,6 +419,9 @@ extern void api(void);
 #define MAX_GPUDEVICES 16
 #define MAX_POOLS (32)
 
+extern int nDevs;
+extern int opt_n_threads;
+extern int num_processors;
 extern int hw_errors;
 extern bool use_syslog;
 extern struct thr_info *thr_info;