Patchwork [v2,2/2] Replace rwlocks with RCU variants of interfaces.

login
register
mail settings
Submitter Harsh Prateek Bora
Date Oct. 13, 2011, 8:35 p.m.
Message ID <1318538102-6982-2-git-send-email-harsh@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/119608/
State New
Headers show

Comments

Harsh Prateek Bora - Oct. 13, 2011, 8:35 p.m.
Use QLIST_INSERT_HEAD_RCU and rcu_read_lock/unlock instead of rwlocks.
Use v9fs_synth_mutex as a write-only mutex to handle concurrent writers.

Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p-synth.c |   26 ++++++++++++--------------
 1 files changed, 12 insertions(+), 14 deletions(-)
Harsh Prateek Bora - Oct. 13, 2011, 10:09 p.m.
On 10/14/2011 02:05 AM, Harsh Prateek Bora wrote:
> Use QLIST_INSERT_HEAD_RCU and rcu_read_lock/unlock instead of rwlocks.
> Use v9fs_synth_mutex as a write-only mutex to handle concurrent writers.

This patch is based on top of:
http://repo.or.cz/w/qemu/v9fs.git/commitdiff/d647a2337e6e010c9f733d1ff68f7f97b27c5f80
>
> Signed-off-by: Harsh Prateek Bora<harsh@linux.vnet.ibm.com>
> ---
>   hw/9pfs/virtio-9p-synth.c |   26 ++++++++++++--------------
>   1 files changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p-synth.c b/hw/9pfs/virtio-9p-synth.c
> index cbf74e4..567611e 100644
> --- a/hw/9pfs/virtio-9p-synth.c
> +++ b/hw/9pfs/virtio-9p-synth.c
> @@ -30,8 +30,7 @@ V9fsSynthNode v9fs_synth_root = {
>       .attr =&v9fs_synth_root.actual_attr,
>   };
>
> -/*FIXME!! should be converted to qemu_rwlock_t */
> -static pthread_rwlock_t v9fs_synth_mutex;
> +static QemuMutex  v9fs_synth_mutex;
>   static int v9fs_synth_node_count;
>   /* set to 1 when the synth fs is ready */
>   static int v9fs_synth_fs;
> @@ -79,7 +78,7 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
>       if (!parent) {
>           parent =&v9fs_synth_root;
>       }
> -    pthread_rwlock_wrlock(&v9fs_synth_mutex);
> +    qemu_mutex_lock(&v9fs_synth_mutex);
>       QLIST_FOREACH(tmp,&parent->child, sibling) {
>           if (!strcmp(tmp->name, name)) {
>               ret = EEXIST;
> @@ -95,7 +94,7 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
>       *result = node;
>       ret = 0;
>   err_out:
> -    pthread_rwlock_unlock(&v9fs_synth_mutex);
> +    qemu_mutex_unlock(&v9fs_synth_mutex);
>       return ret;
>   }
>
> @@ -116,7 +115,7 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int mode,
>           parent =&v9fs_synth_root;
>       }
>
> -    pthread_rwlock_wrlock(&v9fs_synth_mutex);
> +    qemu_mutex_lock(&v9fs_synth_mutex);
>       QLIST_FOREACH(tmp,&parent->child, sibling) {
>           if (!strcmp(tmp->name, name)) {
>               ret = EEXIST;
> @@ -134,10 +133,10 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int mode,
>       node->attr->mode   = mode;
>       node->private      = arg;
>       strncpy(node->name, name, sizeof(node->name));
> -    QLIST_INSERT_HEAD(&parent->child, node, sibling);
> +    QLIST_INSERT_HEAD_RCU(&parent->child, node, sibling);
>       ret = 0;
>   err_out:
> -    pthread_rwlock_unlock(&v9fs_synth_mutex);
> +    qemu_mutex_unlock(&v9fs_synth_mutex);
>       return ret;
>   }
>
> @@ -230,7 +229,7 @@ static int v9fs_synth_get_dentry(V9fsSynthNode *dir, struct dirent *entry,
>       int i = 0;
>       V9fsSynthNode *node;
>
> -    pthread_rwlock_rdlock(&v9fs_synth_mutex);
> +    rcu_read_lock();
>       QLIST_FOREACH(node,&dir->child, sibling) {
>           /* This is the off child of the directory */
>           if (i == off) {
> @@ -238,7 +237,7 @@ static int v9fs_synth_get_dentry(V9fsSynthNode *dir, struct dirent *entry,
>           }
>           i++;
>       }
> -    pthread_rwlock_unlock(&v9fs_synth_mutex);
> +    rcu_read_unlock();
>       if (!node) {
>           /* end of directory */
>           *result = NULL;
> @@ -483,13 +482,14 @@ static int v9fs_synth_name_to_path(FsContext *ctx, V9fsPath *dir_path,
>           goto out;
>       }
>       /* search for the name in the childern */
> -    pthread_rwlock_rdlock(&v9fs_synth_mutex);
> +    rcu_read_lock();
>       QLIST_FOREACH(node,&dir_node->child, sibling) {
>           if (!strcmp(node->name, name)) {
>               break;
>           }
>       }
> -    pthread_rwlock_unlock(&v9fs_synth_mutex);
> +    rcu_read_unlock();
> +
>       if (!node) {
>           errno = ENOENT;
>           return -1;
> @@ -532,11 +532,9 @@ static ssize_t my_test_read(void *in_buf, int len, off_t offset, void *arg)
>   static int v9fs_synth_init(FsContext *ctx)
>   {
>       V9fsSynthNode *node;
> -    pthread_rwlockattr_t rwlockattr;
>
>       QLIST_INIT(&v9fs_synth_root.child);
> -    pthread_rwlockattr_init(&rwlockattr);
> -    pthread_rwlock_init(&v9fs_synth_mutex,&rwlockattr);
> +    qemu_mutex_init(&v9fs_synth_mutex);
>
>       /* Add "." and ".." entries for root */
>       v9fs_add_dir_node(&v9fs_synth_root, v9fs_synth_root.attr->mode,
Aneesh Kumar K.V - Oct. 14, 2011, 4:56 a.m.
On Fri, 14 Oct 2011 02:05:02 +0530, Harsh Prateek Bora <harsh@linux.vnet.ibm.com> wrote:
> Use QLIST_INSERT_HEAD_RCU and rcu_read_lock/unlock instead of rwlocks.
> Use v9fs_synth_mutex as a write-only mutex to handle concurrent writers.
> 
> Signed-off-by: Harsh Prateek Bora <harsh@linux.vnet.ibm.com>
> ---
>  hw/9pfs/virtio-9p-synth.c |   26 ++++++++++++--------------
>  1 files changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/9pfs/virtio-9p-synth.c b/hw/9pfs/virtio-9p-synth.c
> index cbf74e4..567611e 100644
> --- a/hw/9pfs/virtio-9p-synth.c
> +++ b/hw/9pfs/virtio-9p-synth.c
> @@ -30,8 +30,7 @@ V9fsSynthNode v9fs_synth_root = {
>      .attr = &v9fs_synth_root.actual_attr,
>  };
> 
> -/*FIXME!! should be converted to qemu_rwlock_t */
> -static pthread_rwlock_t v9fs_synth_mutex;
> +static QemuMutex  v9fs_synth_mutex;
>  static int v9fs_synth_node_count;
>  /* set to 1 when the synth fs is ready */
>  static int v9fs_synth_fs;
> @@ -79,7 +78,7 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
>      if (!parent) {
>          parent = &v9fs_synth_root;
>      }
> -    pthread_rwlock_wrlock(&v9fs_synth_mutex);
> +    qemu_mutex_lock(&v9fs_synth_mutex);
>      QLIST_FOREACH(tmp, &parent->child, sibling) {
>          if (!strcmp(tmp->name, name)) {
>              ret = EEXIST;
> @@ -95,7 +94,7 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
>      *result = node;
>      ret = 0;
>  err_out:
> -    pthread_rwlock_unlock(&v9fs_synth_mutex);
> +    qemu_mutex_unlock(&v9fs_synth_mutex);
>      return ret;


We sould be using QLIST_INSERT_HEAD_RCU even in v9fs_add_dir_node


>  }
> 
> @@ -116,7 +115,7 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int mode,
>          parent = &v9fs_synth_root;
>      }
> 

-aneesh

Patch

diff --git a/hw/9pfs/virtio-9p-synth.c b/hw/9pfs/virtio-9p-synth.c
index cbf74e4..567611e 100644
--- a/hw/9pfs/virtio-9p-synth.c
+++ b/hw/9pfs/virtio-9p-synth.c
@@ -30,8 +30,7 @@  V9fsSynthNode v9fs_synth_root = {
     .attr = &v9fs_synth_root.actual_attr,
 };
 
-/*FIXME!! should be converted to qemu_rwlock_t */
-static pthread_rwlock_t v9fs_synth_mutex;
+static QemuMutex  v9fs_synth_mutex;
 static int v9fs_synth_node_count;
 /* set to 1 when the synth fs is ready */
 static int v9fs_synth_fs;
@@ -79,7 +78,7 @@  int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
     if (!parent) {
         parent = &v9fs_synth_root;
     }
-    pthread_rwlock_wrlock(&v9fs_synth_mutex);
+    qemu_mutex_lock(&v9fs_synth_mutex);
     QLIST_FOREACH(tmp, &parent->child, sibling) {
         if (!strcmp(tmp->name, name)) {
             ret = EEXIST;
@@ -95,7 +94,7 @@  int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
     *result = node;
     ret = 0;
 err_out:
-    pthread_rwlock_unlock(&v9fs_synth_mutex);
+    qemu_mutex_unlock(&v9fs_synth_mutex);
     return ret;
 }
 
@@ -116,7 +115,7 @@  int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int mode,
         parent = &v9fs_synth_root;
     }
 
-    pthread_rwlock_wrlock(&v9fs_synth_mutex);
+    qemu_mutex_lock(&v9fs_synth_mutex);
     QLIST_FOREACH(tmp, &parent->child, sibling) {
         if (!strcmp(tmp->name, name)) {
             ret = EEXIST;
@@ -134,10 +133,10 @@  int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int mode,
     node->attr->mode   = mode;
     node->private      = arg;
     strncpy(node->name, name, sizeof(node->name));
-    QLIST_INSERT_HEAD(&parent->child, node, sibling);
+    QLIST_INSERT_HEAD_RCU(&parent->child, node, sibling);
     ret = 0;
 err_out:
-    pthread_rwlock_unlock(&v9fs_synth_mutex);
+    qemu_mutex_unlock(&v9fs_synth_mutex);
     return ret;
 }
 
@@ -230,7 +229,7 @@  static int v9fs_synth_get_dentry(V9fsSynthNode *dir, struct dirent *entry,
     int i = 0;
     V9fsSynthNode *node;
 
-    pthread_rwlock_rdlock(&v9fs_synth_mutex);
+    rcu_read_lock();
     QLIST_FOREACH(node, &dir->child, sibling) {
         /* This is the off child of the directory */
         if (i == off) {
@@ -238,7 +237,7 @@  static int v9fs_synth_get_dentry(V9fsSynthNode *dir, struct dirent *entry,
         }
         i++;
     }
-    pthread_rwlock_unlock(&v9fs_synth_mutex);
+    rcu_read_unlock();
     if (!node) {
         /* end of directory */
         *result = NULL;
@@ -483,13 +482,14 @@  static int v9fs_synth_name_to_path(FsContext *ctx, V9fsPath *dir_path,
         goto out;
     }
     /* search for the name in the childern */
-    pthread_rwlock_rdlock(&v9fs_synth_mutex);
+    rcu_read_lock();
     QLIST_FOREACH(node, &dir_node->child, sibling) {
         if (!strcmp(node->name, name)) {
             break;
         }
     }
-    pthread_rwlock_unlock(&v9fs_synth_mutex);
+    rcu_read_unlock();
+
     if (!node) {
         errno = ENOENT;
         return -1;
@@ -532,11 +532,9 @@  static ssize_t my_test_read(void *in_buf, int len, off_t offset, void *arg)
 static int v9fs_synth_init(FsContext *ctx)
 {
     V9fsSynthNode *node;
-    pthread_rwlockattr_t rwlockattr;
 
     QLIST_INIT(&v9fs_synth_root.child);
-    pthread_rwlockattr_init(&rwlockattr);
-    pthread_rwlock_init(&v9fs_synth_mutex, &rwlockattr);
+    qemu_mutex_init(&v9fs_synth_mutex);
 
     /* Add "." and ".." entries for root */
     v9fs_add_dir_node(&v9fs_synth_root, v9fs_synth_root.attr->mode,