[PVFS2-CVS] commit by neill in pvfs2-1/src/kernel/linux-2.6: file.c

CVS commit program cvs at parl.clemson.edu
Mon Mar 22 18:28:19 EST 2004


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

Modified Files:
	file.c 
Log Message:
- fix a nasty race on the I/O write case (though it was fixed on the read case
  apparently a while back) where we access the pvfs2_kernel_op_t object after
  waking up the device file (who is responsible for freeing the object).  in
  the very rare case that the object was freed by the device before we read
  from it for the last time, slab corruption occurs.  can't believe this wasn't
  found earlier...


Index: file.c
===================================================================
RCS file: /projects/cvsroot/pvfs2-1/src/kernel/linux-2.6/file.c,v
diff -p -u -r1.59 -r1.60
--- file.c	16 Mar 2004 23:27:28 -0000	1.59
+++ file.c	22 Mar 2004 23:28:19 -0000	1.60
@@ -176,8 +176,10 @@ ssize_t pvfs2_inode_read(
         amt_complete = new_op->downcall.resp.io.amt_complete;
 
         /*
-          tell the device file owner waiting on I/O that
-          this read has completed and it can return now
+          tell the device file owner waiting on I/O that this read has
+          completed and it can return now.  in this exact case, on
+          wakeup the device will free the op, so we *cannot* touch it
+          after this.
         */
         wake_up_device_for_return(new_op);
 
@@ -224,6 +226,7 @@ static ssize_t pvfs2_file_write(
     size_t each_count = 0, total_count = 0;
     struct inode *inode = file->f_dentry->d_inode;
     pvfs2_inode_t *pvfs2_inode = PVFS2_I(inode);
+    size_t amt_complete = 0;
 
     pvfs2_print("pvfs2: pvfs2_file_write called on %s\n",
 		(file && file->f_dentry && file->f_dentry->d_name.name ?
@@ -299,29 +302,29 @@ static ssize_t pvfs2_file_write(
 	current_buf += new_op->downcall.resp.io.amt_complete;
 	*offset += new_op->downcall.resp.io.amt_complete;
 	total_count += new_op->downcall.resp.io.amt_complete;
+        amt_complete = new_op->downcall.resp.io.amt_complete;
 
         /* adjust inode size if applicable */
-        if ((original_offset + new_op->downcall.resp.io.amt_complete) >
-            inode->i_size)
+        if ((original_offset + amt_complete) > inode->i_size)
         {
-            i_size_write(inode, (original_offset +
-                                 new_op->downcall.resp.io.amt_complete));
+            i_size_write(inode, (original_offset + amt_complete));
         }
 
         /*
-          tell the device file owner waiting on I/O that
-          this read has completed and it can return now
+          tell the device file owner waiting on I/O that this read has
+          completed and it can return now.  in this exact case, on
+          wakeup the device will free the op, so we *cannot* touch it
+          after this.
         */
         wake_up_device_for_return(new_op);
 
 	pvfs_bufmap_put(buffer_index);
 
-	/* if we got a short write, fall out and return what we
-	 * got so far
-	 * TODO: define semantics here- kind of depends on pvfs2
+	/* if we got a short write, fall out and return what we got so
+	 * far TODO: define semantics here- kind of depends on pvfs2
 	 * semantics that don't really exist yet
 	 */
-	if (new_op->downcall.resp.io.amt_complete < each_count)
+	if (amt_complete < each_count)
 	{
 	    break;
 	}



More information about the PVFS2-CVS mailing list