summaryrefslogtreecommitdiffstats
path: root/api/src
diff options
context:
space:
mode:
authorHumble Devassy Chirammal <hchiramm@redhat.com>2015-05-18 17:37:14 +0530
committerNiels de Vos <ndevos@redhat.com>2015-06-22 02:36:24 -0700
commit5afaa6e64f46a32f2b86512236677066adc36510 (patch)
tree43317b98f38f848496b9a6e106309b641d4a760f /api/src
parentdb4beb00c6dfb45367b8869fcaa813c83f1aa8eb (diff)
libgfapi: introduce bit flags for pthread mutex and cond variables
At present stage, the initialization and destroy of conditional and mutex variables of glfs struct happen in different places and there is no uniform way of destroying these variables incase there is a failure when initializing these. The fs mutex and conditional variables are getting destroyed from glfs_free_from_ctx(). However this destroy is not done by a conditional check inside destroy function. By introducing bit flags in glfs object, we can make use of the same (if there is a failure in initialization) in glfs_fini() which is evolving as one and only function to free fs and ctx resources. This patch introduce the flags field and set the flags according to the initialization flow of the mutex and conditional variables of struct glfs members. Without this patch we are compelled to use the goto lables and other hacks in init functions to make sure the resources are freed if there is an error path. Change-Id: I86e2719fb7ce437419a05699b4f06c14b02d0e69 BUG: 1233651 Signed-off-by: Humble Devassy Chirammal <hchiramm@redhat.com> Signed-off-by: Poornima G <pgurusid@redhat.com> Reviewed-on: http://review.gluster.org/10120 Reviewed-by: Niels de Vos <ndevos@redhat.com> Tested-by: Gluster Build System <jenkins@build.gluster.com> Tested-by: NetBSD Build System Reviewed-by: Shyamsundar Ranganathan <srangana@redhat.com> (cherry picked from commit bc858473db1e1091b15d3f3d69ac6ba5d20b58e7) Reviewed-on: http://review.gluster.org/10971 Tested-by: NetBSD Build System <jenkins@build.gluster.org> Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com>
Diffstat (limited to 'api/src')
-rw-r--r--api/src/glfs-internal.h50
-rw-r--r--api/src/glfs.c123
2 files changed, 100 insertions, 73 deletions
diff --git a/api/src/glfs-internal.h b/api/src/glfs-internal.h
index 84935f50cc9..8b57bfede75 100644
--- a/api/src/glfs-internal.h
+++ b/api/src/glfs-internal.h
@@ -21,6 +21,55 @@
#define DEFAULT_REVAL_COUNT 1
+/*
+ * According to pthread mutex and conditional variable ( cond, child_down_count,
+ * upcall mutex and mutex) initialization of struct glfs members,
+ * below GLFS_INIT_* flags are set in 'pthread_flags' member of struct glfs.
+ * The flags are set from glfs_init() and glfs_new_from_ctx() functions
+ * as part of fs inititialization.
+ *
+ * These flag bits are validated in glfs_fini() to destroy all or partially
+ * initialized mutex and conditional variables of glfs object.
+ * If you introduce new pthread mutex or conditional variable in glfs object,
+ * please make sure you have a flag bit intorduced here for proper cleanup
+ * in glfs_fini().
+ *
+ */
+
+#define PTHREAD_MUTEX_INIT(mutex, attr, flags, mask, label) do { \
+ int __ret = -1; \
+ __ret = pthread_mutex_init (mutex, attr); \
+ if (__ret == 0) \
+ flags |= mask; \
+ else \
+ goto label; \
+} while (0)
+
+#define PTHREAD_MUTEX_DESTROY(mutex, flags, mask) do { \
+ if (flags & mask) \
+ (void) pthread_mutex_destroy (mutex); \
+} while (0)
+
+#define PTHREAD_COND_INIT(cond, attr, flags, mask, label) do { \
+ int __ret = -1; \
+ __ret = pthread_cond_init (cond, attr); \
+ if (__ret == 0) \
+ flags |= mask; \
+ else \
+ goto label; \
+} while (0)
+
+#define PTHREAD_COND_DESTROY(cond, flags, mask) do { \
+ if (flags & mask) \
+ (void) pthread_cond_destroy (cond); \
+} while (0)
+
+#define GLFS_INIT_MUTEX 0x00000001 /* pthread_mutex_flag */
+#define GLFS_INIT_COND 0x00000002 /* pthread_cond_flag */
+#define GLFS_INIT_COND_CHILD 0x00000004 /* pthread_cond_child_down_flag */
+#define GLFS_INIT_MUTEX_UPCALL 0x00000008 /* pthread_mutex_upcall_flag */
+
+
#ifndef GF_DARWIN_HOST_OS
#ifndef GFAPI_PUBLIC
#define GFAPI_PUBLIC(sym, ver) /**/
@@ -154,6 +203,7 @@ struct glfs {
pthread_mutex_t upcall_list_mutex; /* mutex for upcall entry list */
uint32_t pin_refcnt;
+ uint32_t pthread_flags; /* GLFS_INIT_* # defines set this flag */
};
struct glfs_fd {
diff --git a/api/src/glfs.c b/api/src/glfs.c
index 897d3eab809..5653f4d46c0 100644
--- a/api/src/glfs.c
+++ b/api/src/glfs.c
@@ -581,12 +581,42 @@ glfs_poller (void *data)
return NULL;
}
-/*
- * please note all the variable initializations done here probably
- * need to be added in 'glfs_new_from_ctx()' as well, so that,
- * we do not miss out while adding new members in 'fs'.
-*/
+static struct glfs *
+glfs_new_fs (const char *volname)
+{
+ struct glfs *fs = NULL;
+
+ fs = CALLOC (1, sizeof (*fs));
+ if (!fs)
+ return NULL;
+
+ INIT_LIST_HEAD (&fs->openfds);
+ INIT_LIST_HEAD (&fs->upcall_list);
+
+ PTHREAD_MUTEX_INIT (&fs->mutex, NULL, fs->pthread_flags,
+ GLFS_INIT_MUTEX, err);
+
+ PTHREAD_COND_INIT (&fs->cond, NULL, fs->pthread_flags,
+ GLFS_INIT_COND, err);
+
+ PTHREAD_COND_INIT (&fs->child_down_cond, NULL, fs->pthread_flags,
+ GLFS_INIT_COND_CHILD, err);
+
+ PTHREAD_MUTEX_INIT (&fs->upcall_list_mutex, NULL, fs->pthread_flags,
+ GLFS_INIT_MUTEX_UPCALL, err);
+
+ fs->volname = strdup (volname);
+ if (!fs->volname)
+ goto err;
+
+ fs->pin_refcnt = 0;
+ return fs;
+
+err:
+ glfs_free_from_ctx (fs);
+ return NULL;
+}
struct glfs *
pub_glfs_new (const char *volname)
@@ -601,26 +631,10 @@ pub_glfs_new (const char *volname)
return NULL;
}
- fs = CALLOC (1, sizeof (*fs));
+ fs = glfs_new_fs (volname);
if (!fs)
return NULL;
- ret = pthread_mutex_init (&fs->mutex, NULL);
- if (ret != 0)
- goto freefs;
-
- ret = pthread_cond_init (&fs->cond, NULL);
- if (ret != 0)
- goto mutex_destroy;
-
- ret = pthread_cond_init (&fs->child_down_cond, NULL);
- if (ret != 0)
- goto cond_destroy;
-
- ret = pthread_mutex_init (&fs->upcall_list_mutex, NULL);
- if (ret != 0)
- goto cond_child_destroy;
-
ctx = glusterfs_ctx_new ();
if (!ctx)
goto fini;
@@ -654,39 +668,9 @@ pub_glfs_new (const char *volname)
if (!(fs->ctx->cmd_args.volfile_id))
goto fini;
- fs->volname = strdup (volname);
- if (!fs->volname)
- goto fini;
-
- INIT_LIST_HEAD (&fs->openfds);
- INIT_LIST_HEAD (&fs->upcall_list);
-
- fs->pin_refcnt = 0;
-
goto out;
-cond_child_destroy:
- pthread_cond_destroy (&fs->child_down_cond);
-
-cond_destroy:
- pthread_cond_destroy (&fs->cond);
-
-mutex_destroy:
- pthread_mutex_destroy (&fs->mutex);
-
-freefs:
- FREE (fs);
- fs = NULL;
- goto out;
fini:
- /*
- * When pthread_*init() fails there is no way for other cleanup
- * funtions (glfs_fini/glfs_free_from_ctx) to know which of them succeded
- * and which did not(unless there is a flag). Hence pthread cleanup is done
- * in this funtion.Anything that fails after pthread_*_init() succeeds, should
- * directly call glfs_fini() to cleanup the resources.
- */
-
glfs_fini (fs);
fs = NULL;
out:
@@ -705,25 +689,15 @@ priv_glfs_new_from_ctx (glusterfs_ctx_t *ctx)
struct glfs *fs = NULL;
if (!ctx)
- return NULL;
+ goto out;
- fs = CALLOC (1, sizeof (*fs));
+ fs = glfs_new_fs ("");
if (!fs)
- return NULL;
- fs->ctx = ctx;
-
- (void) pthread_cond_init (&fs->cond, NULL);
- (void) pthread_cond_init (&fs->child_down_cond, NULL);
-
- (void) pthread_mutex_init (&fs->mutex, NULL);
-
- INIT_LIST_HEAD (&fs->openfds);
-
- INIT_LIST_HEAD (&fs->upcall_list);
- pthread_mutex_init (&fs->upcall_list_mutex, NULL);
+ goto out;
- fs->pin_refcnt = 0;
+ fs->ctx = ctx;
+out:
return fs;
}
@@ -747,15 +721,18 @@ priv_glfs_free_from_ctx (struct glfs *fs)
GF_FREE (u_list->upcall_data.data);
GF_FREE (u_list);
}
- (void) pthread_mutex_destroy (&fs->upcall_list_mutex);
- (void) pthread_cond_destroy (&fs->cond);
- (void) pthread_cond_destroy (&fs->child_down_cond);
+ PTHREAD_MUTEX_DESTROY (&fs->mutex, fs->pthread_flags, GLFS_INIT_MUTEX);
+
+ PTHREAD_COND_DESTROY (&fs->cond, fs->pthread_flags, GLFS_INIT_COND);
+
+ PTHREAD_COND_DESTROY (&fs->child_down_cond, fs->pthread_flags,
+ GLFS_INIT_COND_CHILD);
- (void) pthread_mutex_destroy (&fs->mutex);
+ PTHREAD_MUTEX_DESTROY (&fs->upcall_list_mutex, fs->pthread_flags,
+ GLFS_INIT_MUTEX_UPCALL);
- if (fs->volname)
- FREE (fs->volname);
+ FREE (fs->volname);
FREE (fs);
}