Date: Tue, 2 Nov 2010 22:49:20 +0000 (UTC) From: Pawel Jakub Dawidek <pjd@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org Subject: svn commit: r214697 - stable/8/sbin/hastd Message-ID: <201011022249.oA2MnKad094760@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: pjd Date: Tue Nov 2 22:49:20 2010 New Revision: 214697 URL: http://svn.freebsd.org/changeset/base/214697 Log: MFC r214119,r214273,r214274,r214275,r214276,r214282,r214283,r214284,r214692: r214119: Use closefrom(2) instead of close(2) in a loop. r214273: Load geom_gate.ko module after parsing arguments. r214274: Plug memory leaks. Found with: valgrind r214275: Plug memory leak. r214276: Simplify code a bit. r214282: Move all NV defines into nv.c, they are not used externally thus there is no need to make then visible from outside. r214283: Implement nv_exists() function that returns true if argument of the given name exists. r214284: Before this change on first connect between primary and secondary we initialize all the data. This is huge waste of time and resources if there were no writes yet, as there is no real data to synchronize. Optimize this by sending "virgin" argument to secondary, which gives it a hint that synchronization is not needed. In the common case (where noth nodes are configured at the same time) instead of synchronizing everything, we don't synchronize at all. r214692: Send packets to remote node only via the send thread to avoid possible races - in this case a keepalive packet was send from wrong thread which lead to connection dropping, because of corrupted packet. Fix it by sending keepalive packets directly from the send thread. As a bonus we now send keepalive packets only when connection is idle. Submitted by: Mikolaj Golub <to.my.trociny@gmail.com> Modified: stable/8/sbin/hastd/hastd.c stable/8/sbin/hastd/hooks.c stable/8/sbin/hastd/nv.c stable/8/sbin/hastd/nv.h stable/8/sbin/hastd/parse.y stable/8/sbin/hastd/primary.c stable/8/sbin/hastd/secondary.c stable/8/sbin/hastd/synch.h Directory Properties: stable/8/sbin/hastd/ (props changed) Modified: stable/8/sbin/hastd/hastd.c ============================================================================== --- stable/8/sbin/hastd/hastd.c Tue Nov 2 22:48:52 2010 (r214696) +++ stable/8/sbin/hastd/hastd.c Tue Nov 2 22:49:20 2010 (r214697) @@ -701,8 +701,6 @@ main(int argc, char *argv[]) int debuglevel; sigset_t mask; - g_gate_load(); - foreground = false; debuglevel = 0; pidfile = HASTD_PIDFILE; @@ -736,6 +734,8 @@ main(int argc, char *argv[]) pjdlog_debug_set(debuglevel); + g_gate_load(); + pfh = pidfile_open(pidfile, 0600, &otherpid); if (pfh == NULL) { if (errno == EEXIST) { Modified: stable/8/sbin/hastd/hooks.c ============================================================================== --- stable/8/sbin/hastd/hooks.c Tue Nov 2 22:48:52 2010 (r214696) +++ stable/8/sbin/hastd/hooks.c Tue Nov 2 22:49:20 2010 (r214697) @@ -88,32 +88,19 @@ static void hook_free(struct hookproc *h static void descriptors(void) { - long maxfd; int fd; /* - * Close all descriptors. + * Close all (or almost all) descriptors. */ - maxfd = sysconf(_SC_OPEN_MAX); - if (maxfd < 0) { - pjdlog_errno(LOG_WARNING, "sysconf(_SC_OPEN_MAX) failed"); - maxfd = 1024; - } - for (fd = 0; fd <= maxfd; fd++) { - switch (fd) { - case STDIN_FILENO: - case STDOUT_FILENO: - case STDERR_FILENO: - if (pjdlog_mode_get() == PJDLOG_MODE_STD) - break; - /* FALLTHROUGH */ - default: - close(fd); - break; - } - } - if (pjdlog_mode_get() == PJDLOG_MODE_STD) + if (pjdlog_mode_get() == PJDLOG_MODE_STD) { + closefrom(MAX(MAX(STDIN_FILENO, STDOUT_FILENO), + STDERR_FILENO) + 1); return; + } + + closefrom(0); + /* * Redirect stdin, stdout and stderr to /dev/null. */ Modified: stable/8/sbin/hastd/nv.c ============================================================================== --- stable/8/sbin/hastd/nv.c Tue Nov 2 22:48:52 2010 (r214696) +++ stable/8/sbin/hastd/nv.c Tue Nov 2 22:49:20 2010 (r214697) @@ -46,6 +46,35 @@ __FBSDID("$FreeBSD$"); #include <ebuf.h> #include <nv.h> +#define NV_TYPE_NONE 0 + +#define NV_TYPE_INT8 1 +#define NV_TYPE_UINT8 2 +#define NV_TYPE_INT16 3 +#define NV_TYPE_UINT16 4 +#define NV_TYPE_INT32 5 +#define NV_TYPE_UINT32 6 +#define NV_TYPE_INT64 7 +#define NV_TYPE_UINT64 8 +#define NV_TYPE_INT8_ARRAY 9 +#define NV_TYPE_UINT8_ARRAY 10 +#define NV_TYPE_INT16_ARRAY 11 +#define NV_TYPE_UINT16_ARRAY 12 +#define NV_TYPE_INT32_ARRAY 13 +#define NV_TYPE_UINT32_ARRAY 14 +#define NV_TYPE_INT64_ARRAY 15 +#define NV_TYPE_UINT64_ARRAY 16 +#define NV_TYPE_STRING 17 + +#define NV_TYPE_MASK 0x7f +#define NV_TYPE_FIRST NV_TYPE_INT8 +#define NV_TYPE_LAST NV_TYPE_STRING + +#define NV_ORDER_NETWORK 0x00 +#define NV_ORDER_HOST 0x80 + +#define NV_ORDER_MASK 0x80 + #define NV_MAGIC 0xaea1e struct nv { int nv_magic; @@ -534,6 +563,29 @@ nv_get_string(struct nv *nv, const char return (str); } +bool +nv_exists(struct nv *nv, const char *namefmt, ...) +{ + struct nvhdr *nvh; + va_list nameap; + int snverror, serrno; + + if (nv == NULL) + return (false); + + serrno = errno; + snverror = nv->nv_error; + + va_start(nameap, namefmt); + nvh = nv_find(nv, NV_TYPE_NONE, namefmt, nameap); + va_end(nameap); + + errno = serrno; + nv->nv_error = snverror; + + return (nvh != NULL); +} + /* * Dump content of the nv structure. */ @@ -770,7 +822,8 @@ nv_find(struct nv *nv, int type, const c assert(size >= NVH_SIZE(nvh)); nv_swap(nvh, true); if (strcmp(nvh->nvh_name, name) == 0) { - if ((nvh->nvh_type & NV_TYPE_MASK) != type) { + if (type != NV_TYPE_NONE && + (nvh->nvh_type & NV_TYPE_MASK) != type) { errno = EINVAL; if (nv->nv_error == 0) nv->nv_error = EINVAL; Modified: stable/8/sbin/hastd/nv.h ============================================================================== --- stable/8/sbin/hastd/nv.h Tue Nov 2 22:48:52 2010 (r214696) +++ stable/8/sbin/hastd/nv.h Tue Nov 2 22:49:20 2010 (r214697) @@ -41,33 +41,6 @@ #include <ebuf.h> -#define NV_TYPE_INT8 1 -#define NV_TYPE_UINT8 2 -#define NV_TYPE_INT16 3 -#define NV_TYPE_UINT16 4 -#define NV_TYPE_INT32 5 -#define NV_TYPE_UINT32 6 -#define NV_TYPE_INT64 7 -#define NV_TYPE_UINT64 8 -#define NV_TYPE_INT8_ARRAY 9 -#define NV_TYPE_UINT8_ARRAY 10 -#define NV_TYPE_INT16_ARRAY 11 -#define NV_TYPE_UINT16_ARRAY 12 -#define NV_TYPE_INT32_ARRAY 13 -#define NV_TYPE_UINT32_ARRAY 14 -#define NV_TYPE_INT64_ARRAY 15 -#define NV_TYPE_UINT64_ARRAY 16 -#define NV_TYPE_STRING 17 - -#define NV_TYPE_MASK 0x7f -#define NV_TYPE_FIRST NV_TYPE_INT8 -#define NV_TYPE_LAST NV_TYPE_STRING - -#define NV_ORDER_NETWORK 0x00 -#define NV_ORDER_HOST 0x80 - -#define NV_ORDER_MASK 0x80 - struct nv; struct nv *nv_alloc(void); @@ -153,6 +126,7 @@ const uint64_t *nv_get_uint64_array(stru const char *nv_get_string(struct nv *nv, const char *namefmt, ...) __printflike(2, 3); +bool nv_exists(struct nv *nv, const char *namefmt, ...) __printflike(2, 3); void nv_dump(struct nv *nv); #endif /* !_NV_H_ */ Modified: stable/8/sbin/hastd/parse.y ============================================================================== --- stable/8/sbin/hastd/parse.y Tue Nov 2 22:48:52 2010 (r214696) +++ stable/8/sbin/hastd/parse.y Tue Nov 2 22:49:20 2010 (r214697) @@ -264,6 +264,7 @@ control_statement: CONTROL STR sizeof(depth0_control)) >= sizeof(depth0_control)) { pjdlog_error("control argument is too long."); + free($2); return (1); } break; @@ -274,12 +275,14 @@ control_statement: CONTROL STR sizeof(lconfig->hc_controladdr)) >= sizeof(lconfig->hc_controladdr)) { pjdlog_error("control argument is too long."); + free($2); return (1); } break; default: assert(!"control at wrong depth level"); } + free($2); } ; @@ -291,6 +294,7 @@ listen_statement: LISTEN STR sizeof(depth0_listen)) >= sizeof(depth0_listen)) { pjdlog_error("listen argument is too long."); + free($2); return (1); } break; @@ -301,12 +305,14 @@ listen_statement: LISTEN STR sizeof(lconfig->hc_listenaddr)) >= sizeof(lconfig->hc_listenaddr)) { pjdlog_error("listen argument is too long."); + free($2); return (1); } break; default: assert(!"listen at wrong depth level"); } + free($2); } ; @@ -357,6 +363,7 @@ exec_statement: EXEC STR if (strlcpy(depth0_exec, $2, sizeof(depth0_exec)) >= sizeof(depth0_exec)) { pjdlog_error("Exec path is too long."); + free($2); return (1); } break; @@ -367,12 +374,14 @@ exec_statement: EXEC STR sizeof(curres->hr_exec)) >= sizeof(curres->hr_exec)) { pjdlog_error("Exec path is too long."); + free($2); return (1); } break; default: assert(!"exec at wrong depth level"); } + free($2); } ; @@ -386,6 +395,7 @@ node_start: STR { switch (isitme($1)) { case -1: + free($1); return (1); case 0: break; @@ -395,6 +405,7 @@ node_start: STR default: assert(!"invalid isitme() return value"); } + free($1); } ; @@ -482,14 +493,17 @@ resource_start: STR curres = calloc(1, sizeof(*curres)); if (curres == NULL) { pjdlog_error("Unable to allocate memory for resource."); + free($1); return (1); } if (strlcpy(curres->hr_name, $1, sizeof(curres->hr_name)) >= sizeof(curres->hr_name)) { pjdlog_error("Resource name is too long."); + free($1); return (1); } + free($1); curres->hr_role = HAST_ROLE_INIT; curres->hr_previous_role = HAST_ROLE_INIT; curres->hr_replication = -1; @@ -530,6 +544,7 @@ name_statement: NAME STR sizeof(depth1_provname)) >= sizeof(depth1_provname)) { pjdlog_error("name argument is too long."); + free($2); return (1); } break; @@ -541,12 +556,14 @@ name_statement: NAME STR sizeof(curres->hr_provname)) >= sizeof(curres->hr_provname)) { pjdlog_error("name argument is too long."); + free($2); return (1); } break; default: assert(!"name at wrong depth level"); } + free($2); } ; @@ -558,6 +575,7 @@ local_statement: LOCAL STR sizeof(depth1_localpath)) >= sizeof(depth1_localpath)) { pjdlog_error("local argument is too long."); + free($2); return (1); } break; @@ -569,12 +587,14 @@ local_statement: LOCAL STR sizeof(curres->hr_localpath)) >= sizeof(curres->hr_localpath)) { pjdlog_error("local argument is too long."); + free($2); return (1); } break; default: assert(!"local at wrong depth level"); } + free($2); } ; @@ -589,6 +609,7 @@ resource_node_start: STR if (curres != NULL) { switch (isitme($1)) { case -1: + free($1); return (1); case 0: break; @@ -599,6 +620,7 @@ resource_node_start: STR assert(!"invalid isitme() return value"); } } + free($1); } ; @@ -624,8 +646,10 @@ remote_statement: REMOTE STR sizeof(curres->hr_remoteaddr)) >= sizeof(curres->hr_remoteaddr)) { pjdlog_error("remote argument is too long."); + free($2); return (1); } } + free($2); } ; Modified: stable/8/sbin/hastd/primary.c ============================================================================== --- stable/8/sbin/hastd/primary.c Tue Nov 2 22:48:52 2010 (r214696) +++ stable/8/sbin/hastd/primary.c Tue Nov 2 22:49:20 2010 (r214697) @@ -180,14 +180,21 @@ static pthread_mutex_t metadata_lock; if (_wakeup) \ cv_signal(&hio_##name##_list_cond); \ } while (0) -#define QUEUE_TAKE1(hio, name, ncomp) do { \ +#define QUEUE_TAKE1(hio, name, ncomp, timeout) do { \ + bool _last; \ + \ mtx_lock(&hio_##name##_list_lock[(ncomp)]); \ - while (((hio) = TAILQ_FIRST(&hio_##name##_list[(ncomp)])) == NULL) { \ - cv_wait(&hio_##name##_list_cond[(ncomp)], \ - &hio_##name##_list_lock[(ncomp)]); \ + _last = false; \ + while (((hio) = TAILQ_FIRST(&hio_##name##_list[(ncomp)])) == NULL && !_last) { \ + cv_timedwait(&hio_##name##_list_cond[(ncomp)], \ + &hio_##name##_list_lock[(ncomp)], (timeout)); \ + if ((timeout) != 0) \ + _last = true; \ + } \ + if (hio != NULL) { \ + TAILQ_REMOVE(&hio_##name##_list[(ncomp)], (hio), \ + hio_next[(ncomp)]); \ } \ - TAILQ_REMOVE(&hio_##name##_list[(ncomp)], (hio), \ - hio_next[(ncomp)]); \ mtx_unlock(&hio_##name##_list_lock[(ncomp)]); \ } while (0) #define QUEUE_TAKE2(hio, name) do { \ @@ -417,6 +424,24 @@ init_environment(struct hast_resource *r } } +static bool +init_resuid(struct hast_resource *res) +{ + + mtx_lock(&metadata_lock); + if (res->hr_resuid != 0) { + mtx_unlock(&metadata_lock); + return (false); + } else { + /* Initialize unique resource identifier. */ + arc4random_buf(&res->hr_resuid, sizeof(res->hr_resuid)); + mtx_unlock(&metadata_lock); + if (metadata_write(res) < 0) + exit(EX_NOINPUT); + return (true); + } +} + static void init_local(struct hast_resource *res) { @@ -452,10 +477,12 @@ init_local(struct hast_resource *res) if (res->hr_resuid != 0) return; /* - * We're using provider for the first time, so we have to generate - * resource unique identifier and initialize local and remote counts. + * We're using provider for the first time. Initialize local and remote + * counters. We don't initialize resuid here, as we want to do it just + * in time. The reason for this is that we want to inform secondary + * that there were no writes yet, so there is no need to synchronize + * anything. */ - arc4random_buf(&res->hr_resuid, sizeof(res->hr_resuid)); res->hr_primary_localcnt = 1; res->hr_primary_remotecnt = 0; if (metadata_write(res) < 0) @@ -566,6 +593,19 @@ init_remote(struct hast_resource *res, s nv_add_string(nvout, res->hr_name, "resource"); nv_add_uint8_array(nvout, res->hr_token, sizeof(res->hr_token), "token"); + if (res->hr_resuid == 0) { + /* + * The resuid field was not yet initialized. + * Because we do synchronization inside init_resuid(), it is + * possible that someone already initialized it, the function + * will return false then, but if we successfully initialized + * it, we will get true. True means that there were no writes + * to this resource yet and we want to inform secondary that + * synchronization is not needed by sending "virgin" argument. + */ + if (init_resuid(res)) + nv_add_int8(nvout, 1, "virgin"); + } nv_add_uint64(nvout, res->hr_resuid, "resuid"); nv_add_uint64(nvout, res->hr_primary_localcnt, "localcnt"); nv_add_uint64(nvout, res->hr_primary_remotecnt, "remotecnt"); @@ -646,6 +686,7 @@ init_remote(struct hast_resource *res, s */ (void)hast_activemap_flush(res); } + nv_free(nvin); pjdlog_info("Connected to %s.", res->hr_remoteaddr); if (inp != NULL && outp != NULL) { *inp = in; @@ -1005,6 +1046,10 @@ ggate_recv_thread(void *arg) QUEUE_INSERT1(hio, send, ncomp); break; case BIO_WRITE: + if (res->hr_resuid == 0) { + /* This is first write, initialize resuid. */ + (void)init_resuid(res); + } for (;;) { mtx_lock(&range_lock); if (rangelock_islocked(range_sync, @@ -1074,7 +1119,7 @@ local_send_thread(void *arg) for (;;) { pjdlog_debug(2, "local_send: Taking request."); - QUEUE_TAKE1(hio, send, ncomp); + QUEUE_TAKE1(hio, send, ncomp, 0); pjdlog_debug(2, "local_send: (%p) Got request.", hio); ggio = &hio->hio_ggio; switch (ggio->gctl_cmd) { @@ -1138,6 +1183,38 @@ local_send_thread(void *arg) return (NULL); } +static void +keepalive_send(struct hast_resource *res, unsigned int ncomp) +{ + struct nv *nv; + + if (!ISCONNECTED(res, ncomp)) + return; + + assert(res->hr_remotein != NULL); + assert(res->hr_remoteout != NULL); + + nv = nv_alloc(); + nv_add_uint8(nv, HIO_KEEPALIVE, "cmd"); + if (nv_error(nv) != 0) { + nv_free(nv); + pjdlog_debug(1, + "keepalive_send: Unable to prepare header to send."); + return; + } + if (hast_proto_send(res, res->hr_remoteout, nv, NULL, 0) < 0) { + pjdlog_common(LOG_DEBUG, 1, errno, + "keepalive_send: Unable to send request"); + nv_free(nv); + rw_unlock(&hio_remote_lock[ncomp]); + remote_close(res, ncomp); + rw_rlock(&hio_remote_lock[ncomp]); + return; + } + nv_free(nv); + pjdlog_debug(2, "keepalive_send: Request sent."); +} + /* * Thread sends request to secondary node. */ @@ -1146,6 +1223,7 @@ remote_send_thread(void *arg) { struct hast_resource *res = arg; struct g_gate_ctl_io *ggio; + time_t lastcheck, now; struct hio *hio; struct nv *nv; unsigned int ncomp; @@ -1156,10 +1234,19 @@ remote_send_thread(void *arg) /* Remote component is 1 for now. */ ncomp = 1; + lastcheck = time(NULL); for (;;) { pjdlog_debug(2, "remote_send: Taking request."); - QUEUE_TAKE1(hio, send, ncomp); + QUEUE_TAKE1(hio, send, ncomp, RETRY_SLEEP); + if (hio == NULL) { + now = time(NULL); + if (lastcheck + RETRY_SLEEP <= now) { + keepalive_send(res, ncomp); + lastcheck = now; + } + continue; + } pjdlog_debug(2, "remote_send: (%p) Got request.", hio); ggio = &hio->hio_ggio; switch (ggio->gctl_cmd) { @@ -1845,32 +1932,6 @@ failed: } static void -keepalive_send(struct hast_resource *res, unsigned int ncomp) -{ - struct nv *nv; - - nv = nv_alloc(); - nv_add_uint8(nv, HIO_KEEPALIVE, "cmd"); - if (nv_error(nv) != 0) { - nv_free(nv); - pjdlog_debug(1, - "keepalive_send: Unable to prepare header to send."); - return; - } - if (hast_proto_send(res, res->hr_remoteout, nv, NULL, 0) < 0) { - pjdlog_common(LOG_DEBUG, 1, errno, - "keepalive_send: Unable to send request"); - nv_free(nv); - rw_unlock(&hio_remote_lock[ncomp]); - remote_close(res, ncomp); - rw_rlock(&hio_remote_lock[ncomp]); - return; - } - nv_free(nv); - pjdlog_debug(2, "keepalive_send: Request sent."); -} - -static void guard_one(struct hast_resource *res, unsigned int ncomp) { struct proto_conn *in, *out; @@ -1888,12 +1949,6 @@ guard_one(struct hast_resource *res, uns if (ISCONNECTED(res, ncomp)) { assert(res->hr_remotein != NULL); assert(res->hr_remoteout != NULL); - keepalive_send(res, ncomp); - } - - if (ISCONNECTED(res, ncomp)) { - assert(res->hr_remotein != NULL); - assert(res->hr_remoteout != NULL); rw_unlock(&hio_remote_lock[ncomp]); pjdlog_debug(2, "remote_guard: Connection to %s is ok.", res->hr_remoteaddr); Modified: stable/8/sbin/hastd/secondary.c ============================================================================== --- stable/8/sbin/hastd/secondary.c Tue Nov 2 22:48:52 2010 (r214696) +++ stable/8/sbin/hastd/secondary.c Tue Nov 2 22:49:20 2010 (r214697) @@ -243,13 +243,22 @@ init_remote(struct hast_resource *res, s */ if (res->hr_resuid == 0) { /* - * Provider is used for the first time. Initialize everything. + * Provider is used for the first time. If primary node done no + * writes yet as well (we will find "virgin" argument) then + * there is no need to synchronize anything. If primary node + * done any writes already we have to synchronize everything. */ assert(res->hr_secondary_localcnt == 0); res->hr_resuid = resuid; if (metadata_write(res) < 0) exit(EX_NOINPUT); - memset(map, 0xff, mapsize); + if (nv_exists(nvin, "virgin")) { + free(map); + map = NULL; + mapsize = 0; + } else { + memset(map, 0xff, mapsize); + } nv_add_uint8(nvout, HAST_SYNCSRC_PRIMARY, "syncsrc"); } else if ( /* Is primary is out-of-date? */ @@ -318,11 +327,11 @@ init_remote(struct hast_resource *res, s (uintmax_t)res->hr_secondary_remotecnt); } if (hast_proto_send(res, res->hr_remotein, nvout, map, mapsize) < 0) { - pjdlog_errno(LOG_WARNING, "Unable to send activemap to %s", + pjdlog_exit(EX_TEMPFAIL, "Unable to send activemap to %s", res->hr_remoteaddr); - nv_free(nvout); - exit(EX_TEMPFAIL); } + if (map != NULL) + free(map); nv_free(nvout); if (res->hr_secondary_localcnt > res->hr_primary_remotecnt && res->hr_primary_localcnt > res->hr_secondary_remotecnt) { Modified: stable/8/sbin/hastd/synch.h ============================================================================== --- stable/8/sbin/hastd/synch.h Tue Nov 2 22:48:52 2010 (r214696) +++ stable/8/sbin/hastd/synch.h Tue Nov 2 22:49:20 2010 (r214697) @@ -140,6 +140,8 @@ cv_init(pthread_cond_t *cv) assert(error == 0); error = pthread_cond_init(cv, &attr); assert(error == 0); + error = pthread_condattr_destroy(&attr); + assert(error == 0); } static __inline void cv_wait(pthread_cond_t *cv, pthread_mutex_t *lock)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201011022249.oA2MnKad094760>