summaryrefslogtreecommitdiffstats
path: root/booster/src/booster.c
diff options
context:
space:
mode:
authorShehjar Tikoo <shehjart@gluster.com>2009-05-24 23:05:18 +0000
committerAnand V. Avati <avati@dev.gluster.com>2009-06-02 23:12:32 -0700
commitf888b3f5be4be893323b644dba0668ae3d40228e (patch)
tree3d5fff0a18759f6eaa3651bbe89f961c21b69ada /booster/src/booster.c
parent380cd4e6d296cde9d203ce6c4fa01be189fbf34c (diff)
booster: Eliminate gluster context creation race
When multiple threads try to create a glusterfs context using the glusterfs_init function, those threads end up using the global vairables in the vol file parser in an non-synchronized manner, resulting in a seg-fault. There is now a big lock around searches and additions from the mount table in do_open. This lock granularity could be reduced. Signed-off-by: Anand V. Avati <avati@dev.gluster.com>
Diffstat (limited to 'booster/src/booster.c')
-rw-r--r--booster/src/booster.c225
1 files changed, 133 insertions, 92 deletions
diff --git a/booster/src/booster.c b/booster/src/booster.c
index 40dac43eed7..b51cc25cae3 100644
--- a/booster/src/booster.c
+++ b/booster/src/booster.c
@@ -249,10 +249,10 @@ booster_get_process_fd ()
#define BOOSTER_DEFAULT_LOG CONFDIR"/booster.log"
#define BOOSTER_LOG_ENV_VAR "GLUSTERFS_BOOSTER_LOG"
-static int32_t
-booster_put_handle (booster_mount_table_t *table,
- dev_t st_dev,
- glusterfs_handle_t handle)
+
+static int32_t
+__booster_put_handle (booster_mount_table_t *table, dev_t st_dev,
+ glusterfs_handle_t handle)
{
int32_t hash = 0;
booster_mount_t *mount = NULL, *tmp = NULL;
@@ -269,22 +269,32 @@ booster_put_handle (booster_mount_table_t *table,
mount->handle = handle;
hash = st_dev % table->hash_size;
+ list_for_each_entry (tmp, &table->mounts[hash], device_list) {
+ if (tmp->st_dev == st_dev) {
+ ret = -1;
+ errno = EEXIST;
+ goto out;
+ }
+ }
+ list_add (&mount->device_list, &table->mounts[hash]);
+out:
+ if (ret == -1)
+ free (mount);
+ return ret;
+}
+
+static int32_t
+booster_put_handle (booster_mount_table_t *table,dev_t st_dev,
+ glusterfs_handle_t handle)
+{
+ int32_t ret = 0;
pthread_mutex_lock (&table->lock);
{
- list_for_each_entry (tmp, &table->mounts[hash], device_list) {
- if (tmp->st_dev == st_dev) {
- ret = -1;
- errno = EEXIST;
- goto unlock;
- }
- }
-
- list_add (&mount->device_list, &table->mounts[hash]);
+ ret = __booster_put_handle (table, st_dev, handle);
}
-unlock:
pthread_mutex_unlock (&table->lock);
-
+
return ret;
}
@@ -322,23 +332,32 @@ booster_put_fd (fdtable_t *fdtable, int fd)
}
-static glusterfs_handle_t
-booster_get_handle (booster_mount_table_t *table, dev_t st_dev)
+static glusterfs_handle_t
+__booster_get_handle (booster_mount_table_t *table, dev_t st_dev)
{
int32_t hash = 0;
booster_mount_t *mount = NULL;
glusterfs_handle_t handle = NULL;
hash = st_dev % table->hash_size;
+ list_for_each_entry (mount, &table->mounts[hash], device_list) {
+ if (mount->st_dev == st_dev) {
+ handle = mount->handle;
+ break;
+ }
+ }
+
+ return handle;
+}
+
+static glusterfs_handle_t
+booster_get_handle (booster_mount_table_t *table, dev_t st_dev)
+{
+ glusterfs_handle_t handle = NULL;
pthread_mutex_lock (&table->lock);
{
- list_for_each_entry (mount, &table->mounts[hash], device_list) {
- if (mount->st_dev == st_dev) {
- handle = mount->handle;
- break;
- }
- }
+ handle = __booster_get_handle (table, st_dev);
}
pthread_mutex_unlock (&table->lock);
@@ -346,12 +365,82 @@ booster_get_handle (booster_mount_table_t *table, dev_t st_dev)
}
+/* MBP - Mount Point Bypass,
+ * the approach used to bypass the already mounted glusterfs and instead
+ * use libglusterfsclient.
+ */
+glusterfs_handle_t
+mbp_open (int fd, dev_t file_devno)
+{
+ char *specfile = NULL;
+ FILE *specfp = NULL;
+ int32_t file_size = -1;
+ int ret = -1;
+ glusterfs_handle_t handle = NULL;
+
+ glusterfs_init_params_t ctx = {
+ .loglevel = "critical",
+ .lookup_timeout = 600,
+ .stat_timeout = 600,
+ };
+
+ file_size = fgetxattr (fd, "user.glusterfs-booster-volfile", NULL, 0);
+ if (file_size == -1)
+ return NULL;
+
+ specfile = calloc (1, file_size);
+ if (!specfile) {
+ fprintf (stderr, "cannot allocate memory: %s\n",
+ strerror (errno));
+ goto out;
+ }
+
+ ret = fgetxattr (fd, "user.glusterfs-booster-volfile", specfile,
+ file_size);
+ if (ret == -1)
+ goto out;
+
+ specfp = tmpfile ();
+ if (!specfp)
+ goto out;
+
+ ret = fwrite (specfile, file_size, 1, specfp);
+ if (ret != 1)
+ goto out;
+
+ fseek (specfp, 0L, SEEK_SET);
+ ctx.logfile = strdup (getenv (BOOSTER_LOG_ENV_VAR));
+ if (!ctx.logfile)
+ ctx.logfile = strdup (BOOSTER_DEFAULT_LOG);
+
+ ctx.specfp = specfp;
+ handle = glusterfs_init (&ctx);
+ if (!handle)
+ goto out;
+
+ ret = __booster_put_handle (booster_mount_table, file_devno, handle);
+ if (ret == -1) {
+ glusterfs_fini (handle);
+ goto out;
+ }
+
+out:
+ if (specfile)
+ free (specfile);
+
+ if (specfp)
+ fclose (specfp);
+
+ if (ctx.logfile)
+ free (ctx.logfile);
+
+ return handle;
+}
+
void
do_open (int fd, int flags, mode_t mode)
{
- char *specfile = NULL;
glusterfs_handle_t handle;
- int32_t file_size;
struct stat st = {0,};
int32_t ret = -1;
@@ -364,75 +453,27 @@ do_open (int fd, int flags, mode_t mode)
return;
}
- handle = booster_get_handle (booster_mount_table, st.st_dev);
- if (!handle) {
- FILE *specfp = NULL;
-
- glusterfs_init_params_t ctx = {
- .loglevel = "critical",
- .lookup_timeout = 600,
- .stat_timeout = 600,
- };
-
- file_size = fgetxattr (fd, "user.glusterfs-booster-volfile",
- NULL, 0);
- if (file_size == -1) {
- return;
- }
-
- specfile = calloc (1, file_size);
- if (!specfile) {
- fprintf (stderr, "cannot allocate memory: %s\n",
- strerror (errno));
- return;
- }
-
- ret = fgetxattr (fd, "user.glusterfs-booster-volfile", specfile,
- file_size);
- if (ret == -1) {
- free (specfile);
- return ;
- }
-
- specfp = tmpfile ();
- if (!specfp) {
- free (specfile);
- return;
- }
- ret = fwrite (specfile, file_size, 1, specfp);
- if (ret != 1) {
- fclose (specfp);
- free (specfile);
- }
-
- fseek (specfp, 0L, SEEK_SET);
-
- ctx.logfile = getenv (BOOSTER_LOG_ENV_VAR);
- if (!ctx.logfile)
- ctx.logfile = strdup (BOOSTER_DEFAULT_LOG);
-
- ctx.specfp = specfp;
-
- handle = glusterfs_init (&ctx);
-
- free (specfile);
- fclose (specfp);
-
- if (!handle) {
- return;
- }
+ /* We need to have this big lock to prevent multiple threads
+ * trying to create glusterfs contexts in parallel. The vol file
+ * parser uses global variables which end up in inconsistent
+ * state when modified by different threads without a sync mech
+ * among them.
+ *
+ * We could reduce the lock granularity by locking only when the
+ * glusterfs_init is called inside mbp_open but that requires a
+ * new global lock, that I am not comfortable with. We're better off
+ * using the mount table lock that fits in with the purpose of
+ * maintaining multiple glusterfs contexts in a single process.
+ */
+ pthread_mutex_lock (&booster_mount_table->lock);
+ {
+ handle = __booster_get_handle (booster_mount_table, st.st_dev);
+ if (!handle)
+ handle = mbp_open (fd, st.st_dev);
+ }
+ pthread_mutex_unlock (&booster_mount_table->lock);
- ret = booster_put_handle (booster_mount_table, st.st_dev,
- handle);
- if (ret == -1) {
- glusterfs_fini (handle);
- if (errno != EEXIST) {
- return;
- }
- }
- }
-
if (handle) {
glusterfs_file_t glfs_fd;
char path [UNIX_PATH_MAX];