diff options
| author | Soumya Koduri <skoduri@redhat.com> | 2020-04-06 12:36:44 +0530 | 
|---|---|---|
| committer | Rinku Kothiya <rkothiya@redhat.com> | 2020-05-09 12:01:07 +0000 | 
| commit | d48de09adaffdf6b75f30d78f8a1cd287e6fa8ae (patch) | |
| tree | 95939a4a47e5b250bec8295be19bb4485bf9ab09 | |
| parent | 41f0dbd6d9176bc65b5a39287ba490f734f39035 (diff) | |
gfapi: Suspend synctasks instead of blocking them
There are certain conditions which blocks the current
execution thread (like waiting on mutex lock or condition
variable or I/O response). In such cases, if it is a
synctask thread, we should suspend the task instead
of blocking it (like done in SYNCOP using synctask_yield)
This is to avoid deadlock like the one mentioned below -
1) synctaskA sets fs->migration_in_progress to 1 and
   does I/O (LOOKUP)
2) Other synctask threads wait for fs->migration_in_progress
  to be reset to 0 by synctaskA and hence blocked
3) but synctaskA cannot resume as all synctask threads are blocked
   on (2).
Note: this same approach is already used by few other components
like syncbarrier etc.
Change-Id: If90f870d663bb242c702a5b86ac52eeda67c6f0d
Fixes: #1146
Signed-off-by: Soumya Koduri <skoduri@redhat.com>
(cherry picked from commit 55914f968d907ed747774da15285b42653afda61)
| -rw-r--r-- | api/src/glfs-internal.h | 34 | ||||
| -rw-r--r-- | api/src/glfs-resolve.c | 9 | ||||
| -rw-r--r-- | api/src/glfs.c | 9 | 
3 files changed, 50 insertions, 2 deletions
diff --git a/api/src/glfs-internal.h b/api/src/glfs-internal.h index 55401b2910e..ecee473a9d8 100644 --- a/api/src/glfs-internal.h +++ b/api/src/glfs-internal.h @@ -16,6 +16,7 @@  #include <glusterfs/upcall-utils.h>  #include "glfs-handles.h"  #include <glusterfs/refcount.h> +#include <glusterfs/syncop.h>  #define GLFS_SYMLINK_MAX_FOLLOW 2048 @@ -207,6 +208,7 @@ struct glfs {      glfs_upcall_cbk up_cbk; /* upcall cbk function to be registered */      void *up_data;          /* Opaque data provided by application                               * during upcall registration */ +    struct list_head waitq; /* waiting synctasks */  };  /* This enum is used to maintain the state of glfd. In case of async fops @@ -442,6 +444,34 @@ glfs_process_upcall_event(struct glfs *fs, void *data)          THIS = glfd->fd->inode->table->xl->ctx->master;                        \      } while (0) +#define __GLFS_LOCK_WAIT(fs)                                                   \ +    do {                                                                       \ +        struct synctask *task = NULL;                                          \ +                                                                               \ +        task = synctask_get();                                                 \ +                                                                               \ +        if (task) {                                                            \ +            list_add_tail(&task->waitq, &fs->waitq);                           \ +            pthread_mutex_unlock(&fs->mutex);                                  \ +            synctask_yield(task);                                              \ +            pthread_mutex_lock(&fs->mutex);                                    \ +        } else {                                                               \ +            /* non-synctask */                                                 \ +            pthread_cond_wait(&fs->cond, &fs->mutex);                          \ +        }                                                                      \ +    } while (0) + +#define __GLFS_SYNCTASK_WAKE(fs)                                               \ +    do {                                                                       \ +        struct synctask *waittask = NULL;                                      \ +                                                                               \ +        while (!list_empty(&fs->waitq)) {                                      \ +            waittask = list_entry(fs->waitq.next, struct synctask, waitq);     \ +            list_del_init(&waittask->waitq);                                   \ +            synctask_wake(waittask);                                           \ +        }                                                                      \ +    } while (0) +  /*    By default all lock attempts from user context must    use glfs_lock() and glfs_unlock(). This allows @@ -466,10 +496,10 @@ glfs_lock(struct glfs *fs, gf_boolean_t wait_for_migration)      pthread_mutex_lock(&fs->mutex);      while (!fs->init) -        pthread_cond_wait(&fs->cond, &fs->mutex); +        __GLFS_LOCK_WAIT(fs);      while (wait_for_migration && fs->migration_in_progress) -        pthread_cond_wait(&fs->cond, &fs->mutex); +        __GLFS_LOCK_WAIT(fs);      return 0;  } diff --git a/api/src/glfs-resolve.c b/api/src/glfs-resolve.c index a79f4905749..cde6270590a 100644 --- a/api/src/glfs-resolve.c +++ b/api/src/glfs-resolve.c @@ -65,6 +65,9 @@ __glfs_first_lookup(struct glfs *fs, xlator_t *subvol)      fs->migration_in_progress = 0;      pthread_cond_broadcast(&fs->cond); +    /* wake up other waiting tasks */ +    __GLFS_SYNCTASK_WAKE(fs); +      return ret;  } @@ -154,6 +157,9 @@ __glfs_refresh_inode(struct glfs *fs, xlator_t *subvol, inode_t *inode,      fs->migration_in_progress = 0;      pthread_cond_broadcast(&fs->cond); +    /* wake up other waiting tasks */ +    __GLFS_SYNCTASK_WAKE(fs); +      return newinode;  } @@ -829,6 +835,9 @@ __glfs_migrate_fd(struct glfs *fs, xlator_t *newsubvol, struct glfs_fd *glfd)      fs->migration_in_progress = 0;      pthread_cond_broadcast(&fs->cond); +    /* wake up other waiting tasks */ +    __GLFS_SYNCTASK_WAKE(fs); +      return newfd;  } diff --git a/api/src/glfs.c b/api/src/glfs.c index 98054b6bdb6..ab77f6c1eae 100644 --- a/api/src/glfs.c +++ b/api/src/glfs.c @@ -741,6 +741,7 @@ glfs_new_fs(const char *volname)      INIT_LIST_HEAD(&fs->openfds);      INIT_LIST_HEAD(&fs->upcall_list); +    INIT_LIST_HEAD(&fs->waitq);      PTHREAD_MUTEX_INIT(&fs->mutex, NULL, fs->pthread_flags, GLFS_INIT_MUTEX,                         err); @@ -1229,6 +1230,7 @@ pub_glfs_fini(struct glfs *fs)      call_pool_t *call_pool = NULL;      int fs_init = 0;      int err = -1; +    struct synctask *waittask = NULL;      DECLARE_OLD_THIS; @@ -1250,6 +1252,13 @@ pub_glfs_fini(struct glfs *fs)      call_pool = fs->ctx->pool; +    /* Wake up any suspended synctasks */ +    while (!list_empty(&fs->waitq)) { +        waittask = list_entry(fs->waitq.next, struct synctask, waitq); +        list_del_init(&waittask->waitq); +        synctask_wake(waittask); +    } +      while (countdown--) {          /* give some time for background frames to finish */          pthread_mutex_lock(&fs->mutex);  | 
