From 80ad63dc319298c87af1df9edef55d36b665b7bb Mon Sep 17 00:00:00 2001 From: Jim Warner Date: Fri, 1 Jul 2016 00:00:00 -0500 Subject: [PATCH] library: refactor exposed pointers management, 3rd gen This commit brings all of those 'fetch' type functions (supporting some form of 'reap') into closer alignment with one another. The biggest impact is to be found in the module, which now provides for the separate copy of stack pointers which will be exposed to users. The reason such a copy was not employed initially with , unlike those for and , is due to the fact that such stacks were never sortable. Thus the original raw consolidated extent pointers wouldn't have been disturbed. But that meant no NULL delimiter. So with this commit, all reap/fetch operations now use pointer copies when returning results to callers. And, all such arrays are now NULL delimited meaning callers can choose their own access fencepost: totals or NULL. Signed-off-by: Jim Warner --- proc/pids.c | 10 +++++---- proc/slabinfo.c | 23 +++++++++++---------- proc/stat.c | 54 ++++++++++++++++++++++++++++++++++++------------- 3 files changed, 58 insertions(+), 29 deletions(-) diff --git a/proc/pids.c b/proc/pids.c index 23c97af8..0e36ed14 100644 --- a/proc/pids.c +++ b/proc/pids.c @@ -1090,14 +1090,16 @@ static int stacks_fetch ( // finalize stuff ------------------------------------- /* note: we go to this trouble of maintaining a duplicate of the consolidated | extent stacks addresses represented as our 'anchor' since these ptrs | - are exposed to users ( um, not that we don't trust 'em or anything ) | */ - if (n_saved < n_alloc + 1) { - n_saved = n_alloc + 1; + are exposed to a user (um, not that we don't trust 'em or anything). | + plus, we can NULL delimit these ptrs which we couldn't do otherwise. | */ + if (n_saved < n_inuse + 1) { + n_saved = n_inuse + 1; if (!(info->fetch.results.stacks = realloc(info->fetch.results.stacks, sizeof(void *) * n_saved))) - return -1; + return -ENOMEM; } memcpy(info->fetch.results.stacks, info->fetch.anchor, sizeof(void *) * n_inuse); info->fetch.results.stacks[n_inuse] = NULL; + return n_inuse; // callers beware, this might be zero ! #undef n_alloc #undef n_inuse diff --git a/proc/slabinfo.c b/proc/slabinfo.c index f5dda453..7633ad79 100644 --- a/proc/slabinfo.c +++ b/proc/slabinfo.c @@ -45,6 +45,8 @@ #define SLABINFO_LINE_LEN 2048 #define SLAB_INFO_NAME_LEN 128 +#define STACKS_INCR 128 + /* Because 'select' could, at most, return only node[0] values and since 'reap' | would be forced to duplicate global slabs stuff in every node results stack, | @@ -682,7 +684,6 @@ static struct stacks_extent *stacks_alloc ( static int stacks_fetch ( struct procps_slabinfo *info) { - #define memINCR 64 #define n_alloc info->fetch.n_alloc #define n_inuse info->fetch.n_inuse #define n_saved info->fetch.n_alloc_save @@ -690,9 +691,9 @@ static int stacks_fetch ( // initialize stuff ----------------------------------- if (!info->fetch.anchor) { - if (!(info->fetch.anchor = calloc(sizeof(void *), memINCR))) + if (!(info->fetch.anchor = calloc(sizeof(void *), STACKS_INCR))) return -ENOMEM; - n_alloc = memINCR; + n_alloc = STACKS_INCR; } if (!info->fetch_ext.extents) { if (!(ext = stacks_alloc(&info->fetch_ext, n_alloc))) @@ -707,11 +708,11 @@ static int stacks_fetch ( n_inuse = 0; while (n_inuse < info->nodes_used) { if (!(n_inuse < n_alloc)) { - n_alloc += memINCR; + n_alloc += STACKS_INCR; if ((!(info->fetch.anchor = realloc(info->fetch.anchor, sizeof(void *) * n_alloc))) - || (!(ext = stacks_alloc(&info->fetch_ext, memINCR)))) + || (!(ext = stacks_alloc(&info->fetch_ext, STACKS_INCR)))) return -1; - memcpy(info->fetch.anchor + n_inuse, ext->stacks, sizeof(void *) * memINCR); + memcpy(info->fetch.anchor + n_inuse, ext->stacks, sizeof(void *) * STACKS_INCR); } assign_results(info->fetch.anchor[n_inuse], &info->hist, &info->nodes[n_inuse]); ++n_inuse; @@ -720,18 +721,18 @@ static int stacks_fetch ( // finalize stuff ------------------------------------- /* note: we go to this trouble of maintaining a duplicate of the consolidated | extent stacks addresses represented as our 'anchor' since these ptrs | - are exposed to users ( um, not that we don't trust 'em or anything ) | */ - if (n_saved < n_alloc + 1) { - n_saved = n_alloc + 1; + are exposed to a user (um, not that we don't trust 'em or anything). | + plus, we can NULL delimit these ptrs which we couldn't do otherwise. | */ + if (n_saved < n_inuse + 1) { + n_saved = n_inuse + 1; if (!(info->fetch.results.stacks = realloc(info->fetch.results.stacks, sizeof(void *) * n_saved))) - return -1; + return -ENOMEM; } memcpy(info->fetch.results.stacks, info->fetch.anchor, sizeof(void *) * n_inuse); info->fetch.results.stacks[n_inuse] = NULL; info->fetch.results.total = n_inuse; return n_inuse; - #undef memINCR #undef n_alloc #undef n_inuse #undef n_saved diff --git a/proc/stat.c b/proc/stat.c index a1a4a2df..f0437fa6 100644 --- a/proc/stat.c +++ b/proc/stat.c @@ -105,8 +105,9 @@ struct reap_support { int total; // independently obtained # of cpus/nodes struct ext_support fetch; // extents plus items details struct tic_support hist; // cpu and node jiffies management - int n_anchor_alloc; // last known anchor pointers allocation + int n_alloc; // last known anchor pointers allocation struct stat_stack **anchor; // reapable stacks (consolidated extents) + int n_alloc_save; // last known results.stacks allocation struct stat_reap result; // summary + stacks returned to caller }; @@ -649,6 +650,9 @@ static int stacks_fetch_tics ( struct procps_statinfo *info, struct reap_support *this) { + #define n_alloc this->n_alloc + #define n_inuse this->hist.n_inuse + #define n_saved this->n_alloc_save struct stacks_extent *ext; int i; @@ -659,21 +663,21 @@ static int stacks_fetch_tics ( if (!this->anchor) { if (!(this->anchor = calloc(sizeof(void *), STACKS_INCR))) return -ENOMEM; - this->n_anchor_alloc = STACKS_INCR; + n_alloc = STACKS_INCR; } if (!this->fetch.extents) { - if (!(ext = stacks_alloc(&this->fetch, this->n_anchor_alloc))) + if (!(ext = stacks_alloc(&this->fetch, n_alloc))) return -ENOMEM; - memcpy(this->anchor, ext->stacks, sizeof(void *) * this->n_anchor_alloc); + memcpy(this->anchor, ext->stacks, sizeof(void *) * n_alloc); } if (this->fetch.dirty_stacks) cleanup_stacks_all(&this->fetch); // iterate stuff -------------------------------------- - for (i = 0; i < this->hist.n_inuse; i++) { - if (!(i < this->n_anchor_alloc)) { - this->n_anchor_alloc += STACKS_INCR; - if ((!(this->anchor = realloc(this->anchor, sizeof(void *) * this->n_anchor_alloc))) + for (i = 0; i < n_inuse; i++) { + if (!(i < n_alloc)) { + n_alloc += STACKS_INCR; + if ((!(this->anchor = realloc(this->anchor, sizeof(void *) * n_alloc))) || (!(ext = stacks_alloc(&this->fetch, STACKS_INCR)))) { return -ENOMEM; } @@ -683,12 +687,25 @@ static int stacks_fetch_tics ( } // finalize stuff ------------------------------------- + /* note: we go to this trouble of maintaining a duplicate of the consolidated | + extent stacks addresses represented as our 'anchor' since these ptrs | + are exposed to a user (um, not that we don't trust 'em or anything). | + plus, we can NULL delimit these ptrs which we couldn't do otherwise. | */ + if (n_saved < i + 1) { + n_saved = i + 1; + if (!(this->result.stacks = realloc(this->result.stacks, sizeof(void *) * n_saved))) + return -ENOMEM; + } + memcpy(this->result.stacks, this->anchor, sizeof(void *) * i); + this->result.stacks[i] = NULL; this->result.total = i; - this->result.stacks = this->anchor; this->fetch.dirty_stacks = 1; // callers beware, this might be zero (maybe no libnuma.so) ... return this->result.total; + #undef n_alloc + #undef n_inuse + #undef n_saved } // end: stacks_fetch_tics @@ -766,6 +783,7 @@ PROCPS_EXPORT int procps_stat_new ( p->refcount = 1; p->stat_fd = -1; + p->results.cpus = &p->cpus.result; p->results.nodes = &p->nodes.result; p->cpus.total = procps_cpu_count(); @@ -824,6 +842,8 @@ PROCPS_EXPORT int procps_stat_unref ( if ((*info)->refcount == 0) { if ((*info)->cpus.anchor) free((*info)->cpus.anchor); + if ((*info)->cpus.result.stacks) + free((*info)->cpus.result.stacks); if ((*info)->cpus.hist.tics) free((*info)->cpus.hist.tics); if ((*info)->cpus.fetch.extents) @@ -831,6 +851,8 @@ PROCPS_EXPORT int procps_stat_unref ( if ((*info)->nodes.anchor) free((*info)->nodes.anchor); + if ((*info)->nodes.result.stacks) + free((*info)->nodes.result.stacks); if ((*info)->nodes.hist.tics) free((*info)->nodes.hist.tics); if ((*info)->nodes.fetch.extents) @@ -911,11 +933,6 @@ PROCPS_EXPORT struct stat_reaped *procps_stat_reap ( if (info == NULL || items == NULL) return NULL; - - info->results.summary = NULL; - info->cpus.result.total = info->nodes.result.total = 0; - info->cpus.result.stacks = info->nodes.result.stacks = NULL; - if (what != STAT_REAP_CPUS_ONLY && what != STAT_REAP_CPUS_AND_NODES) return NULL; @@ -940,6 +957,15 @@ PROCPS_EXPORT struct stat_reaped *procps_stat_reap ( return NULL; info->results.summary = update_single_stack(info, &info->cpu_summary); + /* unlike the other 'reap' functions, provides for two separate + stacks pointer arrays exposed to callers. Thus, to keep our promise + of NULL delimit we must ensure a minimal array for the optional one */ + if (!info->nodes.result.stacks + && (!(info->nodes.result.stacks = malloc(sizeof(void *))))) + return NULL; + info->nodes.result.total = 0; + info->nodes.result.stacks[0] = NULL; + switch (what) { case STAT_REAP_CPUS_ONLY: if (!stacks_fetch_tics(info, &info->cpus))