2320. [func] Make statistics couters thread-safe for platforms

			that support certain atomic operations. [RT #17466]
parent fcb738fc
2320. [func] Make statistics couters thread-safe for platforms
that support certain atomic operations. [RT #17466]
2319. [bug] Silence Coverity warnings in
lib/dns/rdata/in_1/apl_42.c. [RT #17469]
......
......@@ -15,7 +15,7 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: server.h,v 1.91 2008/01/18 23:46:57 tbox Exp $ */
/* $Id: server.h,v 1.92 2008/01/24 02:00:44 jinmei Exp $ */
#ifndef NAMED_SERVER_H
#define NAMED_SERVER_H 1
......@@ -91,7 +91,7 @@ struct ns_server {
isc_boolean_t flushonshutdown;
isc_boolean_t log_queries; /*%< For BIND 8 compatibility */
isc_uint64_t * querystats; /*%< Query statistics counters */
dns_stats_t * querystats; /*%< Query statistics counters */
ns_controls_t * controls; /*%< Control channels */
unsigned int dispatchgen;
......
......@@ -15,7 +15,7 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: query.c,v 1.302 2008/01/18 23:46:57 tbox Exp $ */
/* $Id: query.c,v 1.303 2008/01/24 02:00:43 jinmei Exp $ */
/*! \file */
......@@ -130,12 +130,12 @@ inc_stats(ns_client_t *client, dns_statscounter_t counter) {
REQUIRE(counter < DNS_STATS_NCOUNTERS);
ns_g_server->querystats[counter]++;
dns_stats_incrementcounter(ns_g_server->querystats, counter);
if (zone != NULL) {
isc_uint64_t *zonestats = dns_zone_getstatscounters(zone);
dns_stats_t *zonestats = dns_zone_getstats(zone);
if (zonestats != NULL)
zonestats[counter]++;
dns_stats_incrementcounter(zonestats, counter);
}
}
......
......@@ -15,7 +15,7 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: server.c,v 1.500 2008/01/22 00:29:03 jinmei Exp $ */
/* $Id: server.c,v 1.501 2008/01/24 02:00:44 jinmei Exp $ */
/*! \file */
......@@ -3800,8 +3800,8 @@ ns_server_create(isc_mem_t *mctx, ns_server_t **serverp) {
server->server_usehostname = ISC_FALSE;
server->server_id = NULL;
CHECKFATAL(dns_stats_alloccounters(ns_g_mctx, &server->querystats),
"dns_stats_alloccounters");
CHECKFATAL(dns_stats_create(ns_g_mctx, &server->querystats),
"dns_stats_create");
server->flushonshutdown = ISC_FALSE;
server->log_queries = ISC_FALSE;
......@@ -3825,7 +3825,7 @@ ns_server_destroy(ns_server_t **serverp) {
ns_controls_destroy(&server->controls);
dns_stats_freecounters(server->mctx, &server->querystats);
dns_stats_destroy(server->mctx, &server->querystats);
isc_mem_free(server->mctx, server->statsfile);
isc_mem_free(server->mctx, server->dumpfile);
......@@ -4394,6 +4394,7 @@ ns_server_dumpstats(ns_server_t *server) {
FILE *fp = NULL;
int i;
int ncounters;
isc_uint64_t counters[DNS_STATS_NCOUNTERS];
isc_stdtime_get(&now);
......@@ -4403,22 +4404,23 @@ ns_server_dumpstats(ns_server_t *server) {
ncounters = DNS_STATS_NCOUNTERS;
fprintf(fp, "+++ Statistics Dump +++ (%lu)\n", (unsigned long)now);
dns_stats_copy(server->querystats, counters);
for (i = 0; i < ncounters; i++)
fprintf(fp, "%s %" ISC_PRINT_QUADFORMAT "u\n",
dns_statscounter_names[i],
server->querystats[i]);
dns_statscounter_names[i], counters[i]);
zone = NULL;
for (result = dns_zone_first(server->zonemgr, &zone);
result == ISC_R_SUCCESS;
next = NULL, result = dns_zone_next(zone, &next), zone = next)
{
isc_uint64_t *zonestats = dns_zone_getstatscounters(zone);
dns_stats_t *zonestats = dns_zone_getstats(zone);
if (zonestats != NULL) {
char zonename[DNS_NAME_FORMATSIZE];
dns_view_t *view;
char *viewname;
dns_stats_copy(zonestats, counters);
dns_name_format(dns_zone_getorigin(zone),
zonename, sizeof(zonename));
view = dns_zone_getview(zone);
......@@ -4427,7 +4429,7 @@ ns_server_dumpstats(ns_server_t *server) {
fprintf(fp, "%s %" ISC_PRINT_QUADFORMAT
"u %s",
dns_statscounter_names[i],
zonestats[i],
counters[i],
zonename);
if (strcmp(viewname, "_default") != 0)
fprintf(fp, " %s", viewname);
......
......@@ -14,7 +14,7 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: statschannel.c,v 1.4 2008/01/18 23:46:57 tbox Exp $ */
/* $Id: statschannel.c,v 1.5 2008/01/24 02:00:44 jinmei Exp $ */
/*! \file */
......@@ -71,6 +71,7 @@ generatexml(ns_server_t *server, int *buflen, xmlChar **buf) {
int xmlrc;
dns_view_t *view;
int i;
isc_uint64_t counters[DNS_STATS_NCOUNTERS];
isc_time_now(&now);
isc_time_formatISO8601(&ns_g_boottime, boottime, sizeof boottime);
......@@ -117,12 +118,13 @@ generatexml(ns_server_t *server, int *buflen, xmlChar **buf) {
xmlTextWriterWriteString(writer, ISC_XMLCHAR nowstr);
xmlTextWriterEndElement(writer);
TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "counters"));
dns_stats_copy(server->querystats, counters);
for (i = 0; i < DNS_STATS_NCOUNTERS; i++) {
xmlTextWriterStartElement(writer,
ISC_XMLCHAR dns_statscounter_names[i]);
xmlTextWriterWriteFormatString(writer,
"%" ISC_PRINT_QUADFORMAT "u",
server->querystats[i]);
counters[i]);
xmlTextWriterEndElement(writer);
}
xmlTextWriterEndElement(writer); /* counters */
......
......@@ -18,7 +18,7 @@ AC_DIVERT_PUSH(1)dnl
esyscmd([sed "s/^/# /" COPYRIGHT])dnl
AC_DIVERT_POP()dnl
AC_REVISION($Revision: 1.436 $)
AC_REVISION($Revision: 1.437 $)
AC_INIT(lib/dns/name.c)
AC_PREREQ(2.59)
......@@ -2123,11 +2123,13 @@ main() {
exit((sizeof(void *) == 8) ? 0 : 1);
}
],
[arch=x86_64],
[arch=x86_64
have_xaddq=yes],
[arch=x86_32],
[arch=x86_32])
;;
x86_64-*)
have_xaddq=yes
arch=x86_64
;;
alpha*-*)
......@@ -2232,7 +2234,14 @@ else
ISC_PLATFORM_HAVEATOMICSTORE="#undef ISC_PLATFORM_HAVEATOMICSTORE"
fi
if test "$have_xaddq" = "yes"; then
ISC_PLATFORM_HAVEXADDQ="#define ISC_PLATFORM_HAVEXADDQ 1"
else
ISC_PLATFORM_HAVEXADDQ="#undef ISC_PLATFORM_HAVEXADDQ"
fi
AC_SUBST(ISC_PLATFORM_HAVEXADD)
AC_SUBST(ISC_PLATFORM_HAVEXADDQ)
AC_SUBST(ISC_PLATFORM_HAVECMPXCHG)
AC_SUBST(ISC_PLATFORM_HAVEATOMICSTORE)
......
......@@ -15,7 +15,7 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: stats.h,v 1.13 2007/06/19 23:47:17 tbox Exp $ */
/* $Id: stats.h,v 1.14 2008/01/24 02:00:44 jinmei Exp $ */
#ifndef DNS_STATS_H
#define DNS_STATS_H 1
......@@ -42,11 +42,66 @@ typedef enum {
LIBDNS_EXTERNAL_DATA extern const char *dns_statscounter_names[];
isc_result_t
dns_stats_create(isc_mem_t *mctx, dns_stats_t **statsp);
/*%<
* Create a statistics counter structure.
*
* Requires:
*
*\li 'mctx' must be a valid memory context.
*
*\li 'statsp' != NULL && '*statsp' == NULL.
*/
void
dns_stats_destroy(isc_mem_t *mctx, dns_stats_t **statsp);
/*%<
* Destroy a statistics counter structure.
*
* Requires:
*
*\li 'mctx' must be a valid memory context.
*
*\li 'statsp' != NULL and '*statsp' be valid dns_stats_t.
*
* Ensures:
*
*\li '*statsp' == NULL
*/
void
dns_stats_incrementcounter(dns_stats_t *stat, dns_statscounter_t counter);
/*%<
* Increment a counter field of 'stat' specified by 'counter'.
*
* Requires:
*
*\li 'stat' be a valid dns_stats_t.
*
*\li counter < DNS_STATS_NCOUNTERS
*/
void
dns_stats_copy(dns_stats_t *src, isc_uint64_t *dst);
/*%<
* Copy statistics counter fields of 'src' to the 'dst' array.
*
* Requires:
*
*\li 'src' be a valid dns_stats_t.
*
*\li 'dst' be sufficiently large to store DNS_STATS_NCOUNTERS 64-bit
* integers.
*/
isc_result_t
dns_stats_alloccounters(isc_mem_t *mctx, isc_uint64_t **ctrp);
/*%<
* Allocate an array of query statistics counters from the memory
* context 'mctx'.
*
* This function is obsoleted. Use dns_stats_create() instead.
*/
void
......@@ -54,6 +109,8 @@ dns_stats_freecounters(isc_mem_t *mctx, isc_uint64_t **ctrp);
/*%<
* Free an array of query statistics counters allocated from the memory
* context 'mctx'.
*
* This function is obsoleted. Use dns_stats_destroy() instead.
*/
ISC_LANG_ENDDECLS
......
......@@ -15,7 +15,7 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: types.h,v 1.126 2007/09/12 01:09:08 each Exp $ */
/* $Id: types.h,v 1.127 2008/01/24 02:00:44 jinmei Exp $ */
#ifndef DNS_TYPES_H
#define DNS_TYPES_H 1
......@@ -106,6 +106,7 @@ typedef isc_uint8_t dns_secproto_t;
typedef struct dns_signature dns_signature_t;
typedef struct dns_ssurule dns_ssurule_t;
typedef struct dns_ssutable dns_ssutable_t;
typedef struct dns_stats dns_stats_t;
typedef struct dns_tkeyctx dns_tkeyctx_t;
typedef isc_uint16_t dns_trust_t;
typedef struct dns_tsig_keyring dns_tsig_keyring_t;
......
......@@ -15,7 +15,7 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: zone.h,v 1.153 2007/09/18 00:22:31 marka Exp $ */
/* $Id: zone.h,v 1.154 2008/01/24 02:00:44 jinmei Exp $ */
#ifndef DNS_ZONE_H
#define DNS_ZONE_H 1
......@@ -1488,6 +1488,12 @@ dns_zone_setstatistics(dns_zone_t *zone, isc_boolean_t on);
isc_uint64_t *
dns_zone_getstatscounters(dns_zone_t *zone);
/*%<
* This function is obsoleted by dns_zone_getstats().
*/
dns_stats_t *
dns_zone_getstats(dns_zone_t *zone);
/*%<
* Requires:
* zone be a valid zone.
......
......@@ -15,13 +15,19 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: stats.c,v 1.12 2007/06/19 23:47:16 tbox Exp $ */
/* $Id: stats.c,v 1.13 2008/01/24 02:00:44 jinmei Exp $ */
/*! \file */
#include <config.h>
#include <string.h>
#include <isc/atomic.h>
#include <isc/mem.h>
#include <isc/platform.h>
#include <isc/rwlock.h>
#include <isc/util.h>
#include <dns/stats.h>
......@@ -37,6 +43,143 @@ LIBDNS_EXTERNAL_DATA const char *dns_statscounter_names[DNS_STATS_NCOUNTERS] =
"dropped"
};
#ifndef DNS_STATS_USEMULTIFIELDS
#if defined(ISC_RWLOCK_USEATOMIC) && defined(ISC_PLATFORM_HAVEXADD) && !defined(ISC_PLATFORM_HAVEXADDQ)
#define DNS_STATS_USEMULTIFIELDS 1
#else
#define DNS_STATS_USEMULTIFIELDS 0
#endif
#endif /* DNS_STATS_USEMULTIFIELDS */
#if DNS_STATS_USEMULTIFIELDS
typedef struct {
isc_uint32_t hi;
isc_uint32_t lo;
} dns_stat_t;
#else
typedef isc_uint64_t dns_stat_t;
#endif
struct dns_stats {
/* XXXJT: do we need a magic? */
#ifdef ISC_RWLOCK_USEATOMIC
isc_rwlock_t lock;
#endif
dns_stat_t counters[DNS_STATS_NCOUNTERS];
};
isc_result_t
dns_stats_create(isc_mem_t *mctx, dns_stats_t **statsp) {
dns_stats_t *stats;
isc_result_t result = ISC_R_SUCCESS;
REQUIRE(statsp != NULL && *statsp == NULL);
stats = isc_mem_get(mctx, sizeof(*stats));
if (stats == NULL)
return (ISC_R_NOMEMORY);
#ifdef ISC_RWLOCK_USEATOMIC
result = isc_rwlock_init(&stats->lock, 0, 0);
if (result != ISC_R_SUCCESS) {
isc_mem_put(mctx, stats, sizeof(*stats));
return (result);
}
#endif
memset(stats->counters, 0, sizeof(dns_stat_t) * DNS_STATS_NCOUNTERS);
*statsp = stats;
return (result);
}
void
dns_stats_destroy(isc_mem_t *mctx, dns_stats_t **statsp) {
dns_stats_t *stats;
REQUIRE(statsp != NULL && *statsp != NULL);
stats = *statsp;
#ifdef ISC_RWLOCK_USEATOMIC
isc_rwlock_destroy(&stats->lock);
#endif
isc_mem_put(mctx, stats, sizeof(*stats));
*statsp = NULL;
}
void
dns_stats_incrementcounter(dns_stats_t *stats, dns_statscounter_t counter) {
isc_int32_t prev;
REQUIRE(counter < DNS_STATS_NCOUNTERS);
#ifdef ISC_RWLOCK_USEATOMIC
/*
* We use a "read" lock to prevent other threads from reading the
* counter while we "writing" a counter field. The write access itself
* is protected by the atomic operation.
*/
isc_rwlock_lock(&stats->lock, isc_rwlocktype_read);
#endif
#if DNS_STATS_USEMULTIFIELDS
prev = isc_atomic_xadd((isc_int32_t *)&stats->counters[counter].lo, 1);
/*
* If the lower 32-bit field overflows, increment the higher field.
* Note that it's *theoretically* possible that the lower field
* overlaps again before the higher field is incremented. It doesn't
* matter, however, because we don't read the value until
* dns_stats_copy() is called where the whole process is protected
* by the write (exclusive) lock.
*/
if (prev == (isc_int32_t)0xffffffff)
isc_atomic_xadd((isc_int32_t *)&stats->counters[counter].hi, 1);
#elif defined(ISC_PLATFORM_HAVEXADDQ)
UNUSED(prev);
isc_atomic_xaddq((isc_int64_t *)&stats->counters[counter], 1);
#else
UNUSED(prev);
stats->counters[counter]++;
#endif
#ifdef ISC_RWLOCK_USEATOMIC
isc_rwlock_unlock(&stats->lock, isc_rwlocktype_read);
#endif
}
void
dns_stats_copy(dns_stats_t *src, isc_uint64_t *dst) {
int i;
#ifdef ISC_RWLOCK_USEATOMIC
/*
* We use a "write" lock before "reading" the statistics counters as
* an exclusive lock.
*/
isc_rwlock_lock(&src->lock, isc_rwlocktype_write);
#endif
#if DNS_STATS_USEMULTIFIELDS
for (i = 0; i < DNS_STATS_NCOUNTERS; i++) {
dst[i] = ((isc_uint64_t)src->counters[i].hi) << 32 |
src->counters[i].lo;
}
#else
UNUSED(i);
memcpy(dst, src->counters, DNS_STATS_NCOUNTERS * sizeof(dst[0]));
#endif
#ifdef ISC_RWLOCK_USEATOMIC
isc_rwlock_unlock(&src->lock, isc_rwlocktype_write);
#endif
}
/***
*** Obsolete functions follow
***/
isc_result_t
dns_stats_alloccounters(isc_mem_t *mctx, isc_uint64_t **ctrp) {
int i;
......
......@@ -15,7 +15,7 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: zone.c,v 1.470 2007/12/02 22:27:54 marka Exp $ */
/* $Id: zone.c,v 1.471 2008/01/24 02:00:44 jinmei Exp $ */
/*! \file */
......@@ -251,7 +251,7 @@ struct dns_zone {
/*%
* Optional per-zone statistics counters (NULL if not present).
*/
isc_uint64_t *counters;
dns_stats_t *counters;
isc_uint32_t notifydelay;
dns_isselffunc_t isself;
void *isselfarg;
......@@ -736,7 +736,7 @@ zone_free(dns_zone_t *zone) {
isc_mem_free(zone->mctx, zone->journal);
zone->journal = NULL;
if (zone->counters != NULL)
dns_stats_freecounters(zone->mctx, &zone->counters);
dns_stats_destroy(zone->mctx, &zone->counters);
if (zone->db != NULL)
zone_detachdb(zone);
if (zone->acache != NULL)
......@@ -8092,11 +8092,11 @@ dns_zone_setstatistics(dns_zone_t *zone, isc_boolean_t on) {
if (on) {
if (zone->counters != NULL)
goto done;
result = dns_stats_alloccounters(zone->mctx, &zone->counters);
result = dns_stats_create(zone->mctx, &zone->counters);
} else {
if (zone->counters == NULL)
goto done;
dns_stats_freecounters(zone->mctx, &zone->counters);
dns_stats_destroy(zone->mctx, &zone->counters);
}
done:
UNLOCK_ZONE(zone);
......@@ -8105,6 +8105,15 @@ dns_zone_setstatistics(dns_zone_t *zone, isc_boolean_t on) {
isc_uint64_t *
dns_zone_getstatscounters(dns_zone_t *zone) {
/*
* This function is obsoleted by dns_zone_getstats().
*/
UNUSED(zone);
return (NULL);
}
dns_stats_t *
dns_zone_getstats(dns_zone_t *zone) {
return (zone->counters);
}
......@@ -8342,13 +8351,16 @@ dns_zone_xmlrender(dns_zone_t *zone, xmlTextWriterPtr xml, int flags)
xmlTextWriterEndElement(xml);
if (zone->counters != NULL) {
isc_uint64_t counters[DNS_STATS_NCOUNTERS];
xmlTextWriterStartElement(xml, ISC_XMLCHAR "counters");
dns_stats_copy(zone->counters, counters);
for (i = 0 ; i < DNS_STATS_NCOUNTERS ; i++) {
xmlTextWriterStartElement(xml,
ISC_XMLCHAR dns_statscounter_names[i]);
xmlTextWriterWriteFormatString(xml,
"%" ISC_PRINT_QUADFORMAT "u",
zone->counters[i]);
counters[i]);
xmlTextWriterEndElement(xml);
}
xmlTextWriterEndElement(xml); /* counters */
......
......@@ -15,7 +15,7 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: platform.h.in,v 1.45 2007/09/13 04:45:18 each Exp $ */
/* $Id: platform.h.in,v 1.46 2008/01/24 02:00:44 jinmei Exp $ */
#ifndef ISC_PLATFORM_H
#define ISC_PLATFORM_H 1
......@@ -236,6 +236,12 @@
*/
@ISC_PLATFORM_HAVEXADD@
/*
* If the "xaddq" operation (64bit xadd) is available on this architecture,
* ISC_PLATFORM_HAVEXADDQ will be defined.
*/
@ISC_PLATFORM_HAVEXADDQ@
/*
* If the "atomic swap" operation is available on this architecture,
* ISC_PLATFORM_HAVEATOMICSTORE" will be defined.
......
......@@ -14,7 +14,7 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: atomic.h,v 1.8 2007/07/27 14:22:53 explorer Exp $ */
/* $Id: atomic.h,v 1.9 2008/01/24 02:00:44 jinmei Exp $ */
#ifndef ISC_ATOMIC_H
#define ISC_ATOMIC_H 1
......@@ -43,6 +43,24 @@ isc_atomic_xadd(isc_int32_t *p, isc_int32_t val) {
return (prev);
}
#ifdef ISC_PLATFORM_HAVEXADDQ
static __inline__ isc_int64_t
isc_atomic_xaddq(isc_int64_t *p, isc_int64_t val) {
isc_int64_t prev = val;
__asm__ volatile(
#ifdef ISC_PLATFORM_USETHREADS
"lock;"
#endif
"xaddq %0, %1"
:"=q"(prev)
:"m"(*p), "0"(prev)
:"memory", "cc");
return (prev);
}
#endif /* ISC_PLATFORM_HAVEXADDQ */
/*
* This routine atomically stores the value 'val' in 'p'.
*/
......
......@@ -14,7 +14,7 @@
* PERFORMANCE OF THIS SOFTWARE.
*/
/* $Id: atomic.h,v 1.4 2007/06/19 23:47:21 tbox Exp $ */
/* $Id: atomic.h,v 1.5 2008/01/24 02:00:44 jinmei Exp $ */
#ifndef ISC_ATOMIC_H
#define ISC_ATOMIC_H 1
......@@ -49,14 +49,31 @@ isc_atomic_xadd(isc_int32_t *p, isc_int32_t val) {
"lock;"
#endif
"xadd %eax, (%rdx)\n"
/*
* XXX: assume %eax will be used as the return value.
*/
);
}
#ifdef ISC_PLATFORM_HAVEXADDQ
static isc_int64_t
isc_atomic_xaddq(isc_int64_t *p, isc_int64_t val) {
UNUSED(p);
UNUSED(val);
__asm (
"movq %rdi, %rdx\n"
"movq %rsi, %rax\n"
#ifdef ISC_PLATFORM_USETHREADS
"lock;"
#endif
"xaddq %rax, (%rdx)\n"
/*
* set the return value directly in the register so that we
* can avoid guessing the correct position in the stack for a
* local variable.
* XXX: assume %rax will be used as the return value.
*/
);
}
#endif
static void
isc_atomic_store(isc_int32_t *p, isc_int32_t val) {
......@@ -70,6 +87,9 @@ isc_atomic_store(isc_int32_t *p, isc_int32_t val) {
"lock;"
#endif
"xchgl (%rax), %edx\n"
/*
* XXX: assume %rax will be used as the return value.
*/
);
}
......@@ -89,7 +109,7 @@ isc_atomic_cmpxchg(isc_int32_t *p, isc_int32_t cmpval, isc_int32_t val) {
#endif
/*
* If (%rdi) == %eax then (%rdi) := %edx.
% %eax is set to old (%ecx), which will be the return value.
* %eax is set to old (%ecx), which will be the return value.
*/
"cmpxchgl %ecx, (%rdx)"