[Pvfs2-cvs] commit by pcarns in pvfs2-1/src/kernel/linux-2.6: pvfs2-bufmap.c

CVS commit program cvs at parl.clemson.edu
Thu Jan 17 18:59:21 EST 2008


Update of /projects/cvsroot/pvfs2-1/src/kernel/linux-2.6
In directory parlweb1:/tmp/cvs-serv21234/src/kernel/linux-2.6

Modified Files:
	pvfs2-bufmap.c 
Log Message:
This keeps dbench from hard locking pvfs2 by fixing one class of race
conditions between pvfs2-client-core dying while in flight operations
attempt to use mapped buffers.  Could be triggered in other cases, though; 
this isn't dbench specific:
- added safety checks to bufmap_get() and friends that were missing
  before, though most other bufmap functions were already doing checks
- fixed the existing safety checks so that they don't just use an unlocked
  integer flag on entry- bufmap functions now hold rw semaphores to prevent
  the buffers from disappearing while the function is running
More in depth discussion coming to a mailing list near you...


Index: pvfs2-bufmap.c
===================================================================
RCS file: /projects/cvsroot/pvfs2-1/src/kernel/linux-2.6/pvfs2-bufmap.c,v
diff -p -u -r1.54 -r1.55
--- pvfs2-bufmap.c	8 Nov 2007 21:48:23 -0000	1.54
+++ pvfs2-bufmap.c	17 Jan 2008 23:59:21 -0000	1.55
@@ -24,6 +24,7 @@ inline int pvfs_bufmap_shift_query(void)
 }
 
 static int bufmap_init = 0;
+DECLARE_RWSEM(bufmap_init_sem);
 static struct page **bufmap_page_array = NULL;
 
 /* array to track usage of buffer descriptors */
@@ -108,6 +109,7 @@ int pvfs_bufmap_initialize(struct PVFS_d
                  "(ptr (%p) sz (%d) cnt(%d).\n",
                  user_desc->ptr, user_desc->size, user_desc->count);
 
+    down_write(&bufmap_init_sem);
     if (bufmap_init == 1)
     {
         gossip_err("pvfs2: error: bufmap already initialized.\n");
@@ -171,13 +173,13 @@ int pvfs_bufmap_initialize(struct PVFS_d
     }
 
     /* map the pages */
-    down_read(&current->mm->mmap_sem);
+    down_write(&current->mm->mmap_sem);
 
     ret = get_user_pages(
         current, current->mm, (unsigned long)user_desc->ptr,
         bufmap_page_count, 1, 0, bufmap_page_array, NULL);
 
-    up_read(&current->mm->mmap_sem);
+    up_write(&current->mm->mmap_sem);
 
     if (ret < 0)
     {
@@ -245,12 +247,14 @@ int pvfs_bufmap_initialize(struct PVFS_d
     spin_unlock(&readdir_index_lock);
 
     bufmap_init = 1;
+    up_write(&bufmap_init_sem);
 
     gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs2_bufmap_initialize: exiting normally\n");
     return 0;
 
 init_failure:
     finalize_bufmap_descriptors();
+    up_write(&bufmap_init_sem);
     return ret;
 }
 
@@ -267,10 +271,12 @@ void pvfs_bufmap_finalize(void)
 
     gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs2_bufmap_finalize: called\n");
 
+    down_write(&bufmap_init_sem);
     if (bufmap_init == 0)
     {
         gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs2_bufmap_finalize: not yet "
                     "initialized; returning\n");
+        up_write(&bufmap_init_sem);
         return;
     }
 
@@ -284,6 +290,7 @@ void pvfs_bufmap_finalize(void)
     bufmap_init = 0;
 
     finalize_bufmap_descriptors();
+    up_write(&bufmap_init_sem);
     gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs2_bufmap_finalize: exiting normally\n");
 }
 
@@ -373,12 +380,25 @@ static void put_back_slot(struct slot_ar
 int pvfs_bufmap_get(int *buffer_index)
 {
     struct slot_args slargs;
+    int ret;
+
+    down_read(&bufmap_init_sem);
+    if (bufmap_init == 0)
+    {
+        gossip_err("pvfs_bufmap_get: not yet "
+                    "initialized.\n");
+        gossip_err("pvfs2: please confirm that pvfs2-client daemon is running.\n");
+        up_read(&bufmap_init_sem);
+        return -EIO;
+    }
 
     slargs.slot_count = pvfs2_bufmap_desc_count;
     slargs.slot_array = buffer_index_array;
     slargs.slot_lock  = &buffer_index_lock;
     slargs.slot_wq    = &bufmap_waitq;
-    return wait_for_a_slot(&slargs, buffer_index);
+    ret = wait_for_a_slot(&slargs, buffer_index);
+    up_read(&bufmap_init_sem);
+    return(ret);
 }
 
 /* pvfs_bufmap_put()
@@ -391,11 +411,22 @@ void pvfs_bufmap_put(int buffer_index)
 {
     struct slot_args slargs;
 
+    down_read(&bufmap_init_sem);
+    if (bufmap_init == 0)
+    {
+        gossip_err("pvfs_bufmap_put: not yet "
+                    "initialized.\n");
+        gossip_err("pvfs2: please confirm that pvfs2-client daemon is running.\n");
+        up_read(&bufmap_init_sem);
+        return;
+    }
+
     slargs.slot_count = pvfs2_bufmap_desc_count;
     slargs.slot_array = buffer_index_array;
     slargs.slot_lock  = &buffer_index_lock;
     slargs.slot_wq    = &bufmap_waitq;
     put_back_slot(&slargs, buffer_index);
+    up_read(&bufmap_init_sem);
     return;
 }
 
@@ -412,23 +443,48 @@ void pvfs_bufmap_put(int buffer_index)
 int readdir_index_get(int *buffer_index)
 {
     struct slot_args slargs;
+    int ret;
+
+    down_read(&bufmap_init_sem);
+    if (bufmap_init == 0)
+    {
+        gossip_err("pvfs_bufmap_get: not yet "
+                    "initialized.\n");
+        gossip_err("pvfs2: please confirm that pvfs2-client daemon is running.\n");
+        up_read(&bufmap_init_sem);
+        return -EIO;
+    }
 
     slargs.slot_count = PVFS2_READDIR_DEFAULT_DESC_COUNT;
     slargs.slot_array = readdir_index_array;
     slargs.slot_lock  = &readdir_index_lock;
     slargs.slot_wq    = &readdir_waitq;
-    return wait_for_a_slot(&slargs, buffer_index);
+    ret = wait_for_a_slot(&slargs, buffer_index);
+    up_read(&bufmap_init_sem);
+    return(ret);
 }
 
 void readdir_index_put(int buffer_index)
 {
     struct slot_args slargs;
 
+    down_read(&bufmap_init_sem);
+    if (bufmap_init == 0)
+    {
+        gossip_err("pvfs_bufmap_get: not yet "
+                    "initialized.\n");
+        gossip_err("pvfs2: please confirm that pvfs2-client daemon is running.\n");
+        up_read(&bufmap_init_sem);
+        return;
+    }
+
+
     slargs.slot_count = PVFS2_READDIR_DEFAULT_DESC_COUNT;
     slargs.slot_array = readdir_index_array;
     slargs.slot_lock  = &readdir_index_lock;
     slargs.slot_wq    = &readdir_waitq;
     put_back_slot(&slargs, buffer_index);
+    up_read(&bufmap_init_sem);
     return;
 }
 
@@ -449,11 +505,13 @@ int pvfs_bufmap_copy_to_user(void __user
     gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs_bufmap_copy_to_user: to %p, from %p, index %d, "
                 "size %zd\n", to, from, buffer_index, size);
 
+    down_read(&bufmap_init_sem);
     if (bufmap_init == 0)
     {
         gossip_err("pvfs_bufmap_copy_to_user: not yet "
                     "initialized.\n");
         gossip_err("pvfs2: please confirm that pvfs2-client daemon is running.\n");
+        up_read(&bufmap_init_sem);
         return -EIO;
     }
 
@@ -470,6 +528,7 @@ int pvfs_bufmap_copy_to_user(void __user
         if (ret)
         {
             gossip_debug(GOSSIP_BUFMAP_DEBUG, "Failed to copy data to user space %zd\n", ret);
+            up_read(&bufmap_init_sem);
             return -EFAULT;
         }
 
@@ -477,6 +536,7 @@ int pvfs_bufmap_copy_to_user(void __user
         amt_copied += cur_copy_size;
         index++;
     }
+    up_read(&bufmap_init_sem);
     return 0;
 }
 
@@ -491,11 +551,13 @@ int pvfs_bufmap_copy_to_kernel(
     gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs_bufmap_copy_to_kernel: to %p, index %d, size %zd\n",
                 to, buffer_index, size);
 
+    down_read(&bufmap_init_sem);
     if (bufmap_init == 0)
     {
         gossip_err("pvfs_bufmap_copy_to_kernel: not yet "
                     "initialized.\n");
         gossip_err("pvfs2: please confirm that pvfs2-client daemon is running.\n");
+        up_read(&bufmap_init_sem);
         return -EIO;
     }
 
@@ -513,6 +575,7 @@ int pvfs_bufmap_copy_to_kernel(
         amt_copied += cur_copy_size;
         index++;
     }
+    up_read(&bufmap_init_sem);
     return 0;
 }
 
@@ -536,11 +599,13 @@ int pvfs_bufmap_copy_from_user(
     gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs_bufmap_copy_from_user: from %p, index %d, "
                 "size %zd\n", from, buffer_index, size);
 
+    down_read(&bufmap_init_sem);
     if (bufmap_init == 0)
     {
         gossip_err("pvfs_bufmap_copy_from_user: not yet "
                     "initialized.\n");
         gossip_err("pvfs2: please confirm that pvfs2-client daemon is running.\n");
+        up_read(&bufmap_init_sem);
         return -EIO;
     }
 
@@ -564,6 +629,7 @@ int pvfs_bufmap_copy_from_user(
         if (ret)
         {
             gossip_debug(GOSSIP_BUFMAP_DEBUG, "Failed to copy data from user space\n");
+            up_read(&bufmap_init_sem);
             return -EFAULT;
         }
 
@@ -571,6 +637,7 @@ int pvfs_bufmap_copy_from_user(
         amt_copied += cur_copy_size;
         index++;
     }
+    up_read(&bufmap_init_sem);
     return 0;
 }
 
@@ -596,11 +663,13 @@ int pvfs_bufmap_copy_to_pages(int buffer
     gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs_bufmap_copy_to_pages: nr_pages %lu,"
                  "index %d, size %zd\n", nr_segs, buffer_index, size);
 
+    down_read(&bufmap_init_sem);
     if (bufmap_init == 0)
     {
         gossip_err("pvfs_bufmap_copy_to_pages: not yet "
                     "initialized.\n");
         gossip_err("pvfs2: please confirm that pvfs2-client is running.\n");
+        up_read(&bufmap_init_sem);
         return -EIO;
     }
 
@@ -610,12 +679,14 @@ int pvfs_bufmap_copy_to_pages(int buffer
         {
             gossip_err("pvfs_bufmap_copy_to_pages: count cannot exceed "
                        "number of pages(%lu)\n", nr_segs);
+            up_read(&bufmap_init_sem);
             return -EIO;
         }
         page = (struct page *) vec[index].iov_base;
         if (page == NULL) {
             gossip_err("pvfs_bufmap_copy_to_pages: invalid page pointer %d\n",
                         index);
+            up_read(&bufmap_init_sem);
             return -EIO;
         }
         amt_remaining = (size - amt_copied);
@@ -643,6 +714,7 @@ int pvfs_bufmap_copy_to_pages(int buffer
         amt_copied += cur_copy_size;
         index++;
     }
+    up_read(&bufmap_init_sem);
     return 0;
 }
 
@@ -668,11 +740,13 @@ int pvfs_bufmap_copy_from_pages(int buff
     gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs_bufmap_copy_from_pages: nr_pages %lu "
             "index %d, size %zd\n", nr_segs, buffer_index, size);
 
+    down_read(&bufmap_init_sem);
     if (bufmap_init == 0)
     {
         gossip_err("pvfs_bufmap_copy_from_pages: not yet "
                     "initialized.\n");
         gossip_err("pvfs2: please confirm that pvfs2-client is running.\n");
+        up_read(&bufmap_init_sem);
         return -EIO;
     }
 
@@ -681,11 +755,13 @@ int pvfs_bufmap_copy_from_pages(int buff
         if (index >= nr_segs) {
             gossip_err("pvfs_bufmap_copy_from_pages: count cannot exceed number of"
                        "pages(%lu)\n", nr_segs);
+            up_read(&bufmap_init_sem);
             return -EIO;
         }
         page = (struct page *) vec[index].iov_base;
         if (page == NULL) {
             gossip_err("pvfs_bufmap_copy_from_pages: invalid page pointer\n");
+            up_read(&bufmap_init_sem);
             return -EIO;
         }
         amt_remaining = (size - amt_copied);
@@ -706,6 +782,7 @@ int pvfs_bufmap_copy_from_pages(int buff
         amt_copied += cur_copy_size;
         index++;
     }
+    up_read(&bufmap_init_sem);
     return 0;
 }
 
@@ -741,10 +818,12 @@ int pvfs_bufmap_copy_iovec_from_user(
     gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs_bufmap_copy_iovec_from_user: index %d, "
                 "size %zd\n", buffer_index, size);
 
+    down_read(&bufmap_init_sem);
     if (bufmap_init == 0)
     {
         gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs_bufmap_copy_iovec_from_user: not yet "
                     "initialized; returning\n");
+        up_read(&bufmap_init_sem);
         return -EIO;
     }
     /*
@@ -755,6 +834,7 @@ int pvfs_bufmap_copy_iovec_from_user(
     if (copied_iovec == NULL)
     {
         gossip_err("pvfs2_bufmap_copy_iovec_from_user: failed allocating memory\n");
+        up_read(&bufmap_init_sem);
         return -ENOMEM;
     }
     memcpy(copied_iovec, iov, nr_segs * sizeof(struct iovec));
@@ -771,6 +851,7 @@ int pvfs_bufmap_copy_iovec_from_user(
         gossip_err("pvfs2_bufmap_copy_iovec_from_user: computed total (%zd) is not equal to (%zd)\n",
                 amt_copied, size);
         kfree(copied_iovec);
+        up_read(&bufmap_init_sem);
         return -EINVAL;
     }
 
@@ -827,6 +908,7 @@ int pvfs_bufmap_copy_iovec_from_user(
         {
             gossip_err("Failed to copy data from user space\n");
             kfree(copied_iovec);
+            up_read(&bufmap_init_sem);
             return -EFAULT;
         }
 
@@ -840,6 +922,7 @@ int pvfs_bufmap_copy_iovec_from_user(
         }
     }
     kfree(copied_iovec);
+    up_read(&bufmap_init_sem);
     return 0;
 }
 
@@ -872,10 +955,12 @@ int pvfs_bufmap_copy_iovec_from_kernel(
     gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs_bufmap_copy_iovec_from_kernel: index %d, "
                 "size %zd\n", buffer_index, size);
 
+    down_read(&bufmap_init_sem);
     if (bufmap_init == 0)
     {
         gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs_bufmap_copy_iovec_from_kernel: not yet "
                     "initialized; returning\n");
+        up_read(&bufmap_init_sem);
         return -EIO;
     }
     /*
@@ -886,6 +971,7 @@ int pvfs_bufmap_copy_iovec_from_kernel(
     if (copied_iovec == NULL)
     {
         gossip_err("pvfs2_bufmap_copy_iovec_from_kernel: failed allocating memory\n");
+        up_read(&bufmap_init_sem);
         return -ENOMEM;
     }
     memcpy(copied_iovec, iov, nr_segs * sizeof(struct iovec));
@@ -902,6 +988,7 @@ int pvfs_bufmap_copy_iovec_from_kernel(
         gossip_err("pvfs2_bufmap_copy_iovec_from_kernel: computed total (%zd) is not equal to (%zd)\n",
                 amt_copied, size);
         kfree(copied_iovec);
+        up_read(&bufmap_init_sem);
         return -EINVAL;
     }
 
@@ -956,6 +1043,7 @@ int pvfs_bufmap_copy_iovec_from_kernel(
         }
     }
     kfree(copied_iovec);
+    up_read(&bufmap_init_sem);
     return 0;
 }
 
@@ -986,10 +1074,12 @@ int pvfs_bufmap_copy_to_user_iovec(
     gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs_bufmap_copy_to_user_iovec: index %d, "
                 "size %zd\n", buffer_index, size);
 
+    down_read(&bufmap_init_sem);
     if (bufmap_init == 0)
     {
         gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs2_bufmap_copy_to_user_iovec: not yet "
                     "initialized; returning\n");
+        up_read(&bufmap_init_sem);
         return -EIO;
     }
     /*
@@ -1000,6 +1090,7 @@ int pvfs_bufmap_copy_to_user_iovec(
     if (copied_iovec == NULL)
     {
         gossip_err("pvfs2_bufmap_copy_to_user_iovec: failed allocating memory\n");
+        up_read(&bufmap_init_sem);
         return -ENOMEM;
     }
     memcpy(copied_iovec, iov, nr_segs * sizeof(struct iovec));
@@ -1016,6 +1107,7 @@ int pvfs_bufmap_copy_to_user_iovec(
         gossip_err("pvfs2_bufmap_copy_to_user_iovec: computed total (%zd) is less than (%zd)\n",
                 amt_copied, size);
         kfree(copied_iovec);
+        up_read(&bufmap_init_sem);
         return -EINVAL;
     }
 
@@ -1071,6 +1163,7 @@ int pvfs_bufmap_copy_to_user_iovec(
         {
             gossip_err("Failed to copy data to user space\n");
             kfree(copied_iovec);
+            up_read(&bufmap_init_sem);
             return -EFAULT;
         }
 
@@ -1084,6 +1177,7 @@ int pvfs_bufmap_copy_to_user_iovec(
         }
     }
     kfree(copied_iovec);
+    up_read(&bufmap_init_sem);
     return 0;
 }
 
@@ -1112,10 +1206,12 @@ int pvfs_bufmap_copy_to_kernel_iovec(
     gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs_bufmap_copy_to_kernel_iovec: index %d, "
                 "size %zd\n", buffer_index, size);
 
+    down_read(&bufmap_init_sem);
     if (bufmap_init == 0)
     {
         gossip_debug(GOSSIP_BUFMAP_DEBUG, "pvfs2_bufmap_copy_to_kernel_iovec: not yet "
                     "initialized; returning\n");
+        up_read(&bufmap_init_sem);
         return -EIO;
     }
     /*
@@ -1126,6 +1222,7 @@ int pvfs_bufmap_copy_to_kernel_iovec(
     if (copied_iovec == NULL)
     {
         gossip_err("pvfs2_bufmap_copy_to_kernel_iovec: failed allocating memory\n");
+        up_read(&bufmap_init_sem);
         return -ENOMEM;
     }
     memcpy(copied_iovec, iov, nr_segs * sizeof(struct iovec));
@@ -1142,6 +1239,7 @@ int pvfs_bufmap_copy_to_kernel_iovec(
         gossip_err("pvfs2_bufmap_copy_to_kernel_iovec: computed total (%zd) is less than (%zd)\n",
                 amt_copied, size);
         kfree(copied_iovec);
+        up_read(&bufmap_init_sem);
         return -EINVAL;
     }
 
@@ -1197,6 +1295,7 @@ int pvfs_bufmap_copy_to_kernel_iovec(
         }
     }
     kfree(copied_iovec);
+    up_read(&bufmap_init_sem);
     return 0;
 }
 
@@ -1237,17 +1336,20 @@ size_t pvfs_bufmap_copy_to_user_task(
             " PID: %d, to %p, from %p, index %d, "
             " size %zd\n", tsk->pid, to, from, buffer_index, size);
 
+    down_read(&bufmap_init_sem);
     if (bufmap_init == 0)
     {
         gossip_err("pvfs2_bufmap_copy_to_user: not yet "
                     "initialized.\n");
         gossip_err("pvfs2: please confirm that pvfs2-client "
                 "daemon is running.\n");
+        up_read(&bufmap_init_sem);
         return -EIO;
     }
     mm = get_task_mm(tsk);
     if (!mm) 
     {
+        up_read(&bufmap_init_sem);
         return -EIO;
     }
     /* 
@@ -1312,6 +1414,7 @@ size_t pvfs_bufmap_copy_to_user_task(
     }
     up_read(&mm->mmap_sem);
     mmput(mm);
+    up_read(&bufmap_init_sem);
     return (amt_copied < size) ? -EFAULT: amt_copied;
 }
 



More information about the Pvfs2-cvs mailing list