Package: fusedav Version: 0.2-3 Severity: normal Tags: patch upstream [PATCH] file_cache: prevent duplicate entries on concurrent open
Calling file_cache_open() concurrently with the same path can result in duplicate entries. Instead, hold onto the files_mutex lock through the check and insertion. To maximize concurrency with slow HEAD requests, we only insert a locked dummy entry while files_mutex, and slowly populate the new entry when files_mutex is unlocked (but fi->mutex is still held). Holding fi->mutex for the new entry will prevent thundering herds on the WebDAV server if we are hit by many opens at once.
>From 8ddaa7a0a9d19d913b9acf92f6056aea72267d83 Mon Sep 17 00:00:00 2001 From: Eric Wong <normalper...@yhbt.net> Date: Thu, 25 Oct 2012 01:56:27 +0000 Subject: [PATCH 09/13] file_cache: prevent duplicate entries on concurrent open Calling file_cache_open() concurrently with the same path can result in duplicate entries. Instead, hold onto the files_mutex lock through the check and insertion. To maximize concurrency with slow HEAD requests, we only insert a locked dummy entry while files_mutex, and slowly populate the new entry when files_mutex is unlocked (but fi->mutex is still held). Holding fi->mutex for the new entry will prevent thundering herds on the WebDAV server if we are hit by many opens at once. --- src/filecache.c | 81 ++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 51 insertions(+), 30 deletions(-) diff --git a/src/filecache.c b/src/filecache.c index 59e1a9c..08071fe 100644 --- a/src/filecache.c +++ b/src/filecache.c @@ -70,10 +70,8 @@ static pthread_mutex_t files_mutex = PTHREAD_MUTEX_INITIALIZER; static int file_cache_sync_unlocked(struct file_info *fi); -void* file_cache_get(const char *path) { +static void* file_cache_get_unlocked(const char *path) { struct file_info *f, *r = NULL; - - pthread_mutex_lock(&files_mutex); for (f = files; f; f = f->next) { @@ -85,10 +83,19 @@ void* file_cache_get(const char *path) { pthread_mutex_unlock(&f->mutex); if (r) - break; + return r; } - + + return NULL; +} + +void* file_cache_get(const char *path) { + struct file_info *f; + + pthread_mutex_lock(&files_mutex); + f = file_cache_get_unlocked(path); pthread_mutex_unlock(&files_mutex); + return f; } @@ -157,29 +164,47 @@ int file_cache_close(void *f) { return r; } +static void file_cache_update_flags_unlocked(struct file_info *fi, int flags) +{ + if (flags & O_RDONLY || flags & O_RDWR) fi->readable = 1; + if (flags & O_WRONLY || flags & O_RDWR) fi->writable = 1; +} + void* file_cache_open(const char *path, int flags) { struct file_info *fi = NULL; char tempfile[PATH_MAX]; const char *length = NULL; ne_request *req = NULL; ne_session *session; + int cached = 0; if (!(session = session_get(1))) { errno = EIO; - goto fail; + return NULL; } - if ((fi = file_cache_get(path))) { - if (flags & O_RDONLY || flags & O_RDWR) fi->readable = 1; - if (flags & O_WRONLY || flags & O_RDWR) fi->writable = 1; - return fi; + pthread_mutex_lock(&files_mutex); + + if ((fi = file_cache_get_unlocked(path))) { + pthread_mutex_lock(&fi->mutex); + file_cache_update_flags_unlocked(fi, flags); + pthread_mutex_unlock(&fi->mutex); + cached = 1; + } else { + fi = calloc(1, sizeof(struct file_info)); + assert(fi); + fi->filename = strdup(path); + assert(fi->filename); + pthread_mutex_init(&fi->mutex, NULL); + pthread_mutex_lock(&fi->mutex); + fi->next = files; + files = fi; } - fi = malloc(sizeof(struct file_info)); - memset(fi, 0, sizeof(struct file_info)); - fi->fd = -1; + pthread_mutex_unlock(&files_mutex); - fi->filename = strdup(path); + if (cached) + return fi; snprintf(tempfile, sizeof(tempfile), "%s/fusedav-cache-XXXXXX", "/tmp"); if ((fi->fd = mkstemp(tempfile)) < 0) @@ -202,19 +227,11 @@ void* file_cache_open(const char *path, int flags) { fi->server_length = fi->length = (off_t)atoll(length); ne_request_destroy(req); - - if (flags & O_RDONLY || flags & O_RDWR) fi->readable = 1; - if (flags & O_WRONLY || flags & O_RDWR) fi->writable = 1; - - pthread_mutex_init(&fi->mutex, NULL); - - pthread_mutex_lock(&files_mutex); - fi->next = files; - files = fi; - pthread_mutex_unlock(&files_mutex); + file_cache_update_flags_unlocked(fi, flags); fi->ref = 1; - + pthread_mutex_unlock(&fi->mutex); + return fi; fail: @@ -223,10 +240,9 @@ fail: ne_request_destroy(req); if (fi) { - if (fi->fd >= 0) - close(fi->fd); - free(fi->filename); - free(fi); + pthread_mutex_unlock(&fi->mutex); + fi->dead = 1; + file_cache_free_unlocked(fi); } return NULL; @@ -423,8 +439,13 @@ int file_cache_close_all(void) { off_t file_cache_get_size(void *f) { struct file_info *fi = f; + off_t length; assert(fi); - return fi->length; + pthread_mutex_lock(&fi->mutex); + length = fi->length; + pthread_mutex_unlock(&fi->mutex); + + return length; } -- 1.8.0.3.gdd57fab.dirty