From 11fc2fb67df9e452698b84eae0e029805ce1a018 Mon Sep 17 00:00:00 2001 From: ckolivas Date: Sat, 17 Jan 2015 17:05:24 +1100 Subject: [PATCH 1/4] Revert "Only iterate over workers in workerstats if some are left in the worker_instances list" This reverts commit 450f2040b2022633e56783cc1c9f1a5d0966b65a. Of no advantage. --- src/stratifier.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/stratifier.c b/src/stratifier.c index b669e235..c04aa3f3 100644 --- a/src/stratifier.c +++ b/src/stratifier.c @@ -3517,9 +3517,6 @@ static void *statsupdate(void *arg) worker_instance_t *worker; bool idle = false; - if (instance->workers < 1) - continue; - /* Decay times per worker */ DL_FOREACH(instance->worker_instances, worker) { per_tdiff = tvdiff(&now, &worker->last_share); From b172162f276e5993ce974b1032e6567c48bee7fa Mon Sep 17 00:00:00 2001 From: ckolivas Date: Sat, 17 Jan 2015 17:07:34 +1100 Subject: [PATCH 2/4] kill_instance always follows del_disconnected --- src/stratifier.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/stratifier.c b/src/stratifier.c index c04aa3f3..d45fedce 100644 --- a/src/stratifier.c +++ b/src/stratifier.c @@ -857,10 +857,27 @@ static void update_base(ckpool_t *ckp, int prio) create_pthread(pth, do_update, ur); } +/* Add a stratum instance to the dead instances list */ +static void __kill_instance(sdata_t *sdata, stratum_instance_t *client) +{ + user_instance_t *instance = client->user_instance; + + if (instance) { + worker_instance_t *worker = client->worker_instance; + + DL_DELETE(instance->instances, client); + if (worker) + DL_DELETE(instance->worker_instances, worker); + } + LL_PREPEND(sdata->dead_instances, client); + sdata->stats.dead++; +} + static void __del_disconnected(sdata_t *sdata, stratum_instance_t *client) { HASH_DEL(sdata->disconnected_instances, client); sdata->stats.disconnected--; + __kill_instance(sdata, client); } static void drop_allclients(ckpool_t *ckp) @@ -1112,22 +1129,6 @@ static stratum_instance_t *__stratum_add_instance(ckpool_t *ckp, int64_t id, int return instance; } -/* Add a stratum instance to the dead instances list */ -static void __kill_instance(sdata_t *sdata, stratum_instance_t *client) -{ - user_instance_t *instance = client->user_instance; - - if (instance) { - worker_instance_t *worker = client->worker_instance; - - DL_DELETE(instance->instances, client); - if (worker) - DL_DELETE(instance->worker_instances, worker); - } - LL_PREPEND(sdata->dead_instances, client); - sdata->stats.dead++; -} - /* Only supports a full ckpool instance sessionid with an 8 byte sessionid */ static bool disconnected_sessionid_exists(sdata_t *sdata, const char *sessionid, int64_t id) { @@ -1157,7 +1158,6 @@ static bool disconnected_sessionid_exists(sdata_t *sdata, const char *sessionid, /* If we've found a matching disconnected instance, use it only * once and discard it */ __del_disconnected(sdata, instance); - __kill_instance(sdata, instance); ret = true; } out_unlock: @@ -1290,7 +1290,6 @@ static void drop_client(sdata_t *sdata, int64_t id) continue; LOGINFO("Discarding aged disconnected instance %ld", client->id); __del_disconnected(sdata, client); - __kill_instance(sdata, client); } /* Discard any dead instances that no longer hold any reference counts, * freeing up their memory safely */ From 1465002ecc1e2c1e474575d8ce2165ffcec9c598 Mon Sep 17 00:00:00 2001 From: ckolivas Date: Sat, 17 Jan 2015 17:17:09 +1100 Subject: [PATCH 3/4] Only lock and release the write instance lock once in drop_client --- src/stratifier.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/stratifier.c b/src/stratifier.c index d45fedce..b0351cba 100644 --- a/src/stratifier.c +++ b/src/stratifier.c @@ -1244,17 +1244,18 @@ static void dec_worker(ckpool_t *ckp, user_instance_t *instance) static void drop_client(sdata_t *sdata, int64_t id) { stratum_instance_t *client, *tmp; + user_instance_t *instance = NULL; time_t now_t = time(NULL); + ckpool_t *ckp = NULL; bool dec = false; - LOGINFO("Stratifier dropping client %ld", id); + LOGINFO("Stratifier requested to drop client %ld", id); ck_wlock(&sdata->instance_lock); client = __instance_by_id(sdata, id); if (client) { stratum_instance_t *old_client = NULL; - __inc_instance_ref(client); if (client->authorised) { dec = true; client->authorised = false; @@ -1269,19 +1270,11 @@ static void drop_client(sdata_t *sdata, int64_t id) client->disconnected_time = time(NULL); } else __kill_instance(sdata, client); + ckp = client->ckp; + instance = client->user_instance; + LOGINFO("Stratifer dropped %sauthorised client %ld", dec ? "" : "un", id); } - ck_wunlock(&sdata->instance_lock); - - /* Decrease worker count outside of instance_lock to avoid recursive - * locking */ - if (dec) - dec_worker(client->ckp, client->user_instance); - /* Cull old unused clients lazily when there are no more reference - * counts for them. */ - ck_wlock(&sdata->instance_lock); - if (client) - __dec_instance_ref(client); /* Old disconnected instances will not have any valid shares so remove * them from the disconnected instances list if they've been dead for * more than 10 minutes */ @@ -1291,8 +1284,9 @@ static void drop_client(sdata_t *sdata, int64_t id) LOGINFO("Discarding aged disconnected instance %ld", client->id); __del_disconnected(sdata, client); } - /* Discard any dead instances that no longer hold any reference counts, - * freeing up their memory safely */ + + /* Cull old unused clients lazily when there are no more reference + * counts for them. */ LL_FOREACH_SAFE(sdata->dead_instances, client, tmp) { if (!client->ref) { LOGINFO("Stratifier discarding instance %ld", client->id); @@ -1304,6 +1298,11 @@ static void drop_client(sdata_t *sdata, int64_t id) } } ck_wunlock(&sdata->instance_lock); + + /* Decrease worker count outside of instance_lock to avoid recursive + * locking. ckp and instance are guaranteed to be set if dec is true */ + if (dec) + dec_worker(ckp, instance); } static void stratum_broadcast_message(sdata_t *sdata, const char *msg) From e91ea6c235b9972fe0bb132608278bc45ca65c00 Mon Sep 17 00:00:00 2001 From: ckolivas Date: Sat, 17 Jan 2015 17:25:48 +1100 Subject: [PATCH 4/4] Add sanity breakout of worker instance looping in statsupdate --- src/stratifier.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/stratifier.c b/src/stratifier.c index b0351cba..c3f795c6 100644 --- a/src/stratifier.c +++ b/src/stratifier.c @@ -3513,10 +3513,18 @@ static void *statsupdate(void *arg) HASH_ITER(hh, sdata->user_instances, instance, tmpuser) { worker_instance_t *worker; + int iterations = 0; bool idle = false; /* Decay times per worker */ DL_FOREACH(instance->worker_instances, worker) { + /* FIXME: This shouldn't happen and is purely a sanity + * breakout till the real issue is found fixed. */ + if (unlikely(iterations++ > instance->workers)) { + LOGWARNING("Statsupdate trying to iterate more than %d existing workers for worker %s", + instance->workers, worker->workername); + break; + } per_tdiff = tvdiff(&now, &worker->last_share); if (per_tdiff > 60) { decay_time(&worker->dsps1, 0, per_tdiff, 60);