cluster: RHEL55 - gfs-kernel: assertion "!get_transaction" fails on mmaps between gfs filesystems

Benjamin Marzinski bmarzins at fedoraproject.org
Wed Jul 28 20:59:07 UTC 2010


Gitweb:        http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=48194711384ab9a6af2701fd7313e05c8c0ce1d7
Commit:        48194711384ab9a6af2701fd7313e05c8c0ce1d7
Parent:        e44f2175836fbc9ec95b761781911ca2a6cc570a
Author:        Benjamin Marzinski <bmarzins at redhat.com>
AuthorDate:    Wed Jul 28 15:54:25 2010 -0500
Committer:     Benjamin Marzinski <bmarzins at redhat.com>
CommitterDate: Wed Jul 28 15:54:25 2010 -0500

gfs-kernel: assertion "!get_transaction" fails on mmaps between gfs filesystems

The gfs walk_vm() code only checked that the buffer which got passed in was not
mmaped to a file on the same filesystem as the file being written to.  However,
if that buffer was from a file on another GFS filesystem, and that file did
not have blocks allocated for all of the area mapped to the buffer, GFS
would need to start a transaction when it tried to read from the buffer.
Since gfs had already started a transaction, the !get_transaction assert
would fail.  Furthermore, gfs is not able to safely hold glocks on files in
two seperate filesystems at the same time, since inode numbers are used for
lock ordering, and these are not unique across filesystems. Because of this
there is no way to guarantee that the necessary blocks won't be removed from
the mmaped file via a truncate before gfs needs to read them in.  This fix
simply reads the buffer in before doing any locking, if gfs notices that the
buffer is mmaped to a file on another gfs filesystem. This should fix the
problem except in corner cases, where the mmaped file is being truncated at
the same time.

Resolves: bz617339

Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>
---
 gfs-kernel/src/gfs/ops_file.c |   70 +++++++++++++++++++++++------------------
 1 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/gfs-kernel/src/gfs/ops_file.c b/gfs-kernel/src/gfs/ops_file.c
index 150f328..44a73e5 100644
--- a/gfs-kernel/src/gfs/ops_file.c
+++ b/gfs-kernel/src/gfs/ops_file.c
@@ -192,6 +192,37 @@ walk_vm_hard(struct file *file, char *buf, size_t size, loff_t *offset,
 }
 
 /**
+ * grope_mapping - feel up a mapping that needs to be written
+ * @buf: the start of the memory to be written
+ * @size: the size of the memory to be written
+ *
+ * We do this after acquiring the locks on the mapping,
+ * but before starting the write transaction.  We need to make
+ * sure that we don't cause recursive transactions if blocks
+ * need to be allocated to the file backing the mapping.
+ *
+ * Returns: errno
+ */
+
+static int
+grope_mapping(char *buf, size_t size)
+{
+	unsigned long start = (unsigned long)buf;
+	unsigned long stop = start + size;
+	char c;
+
+	while (start < stop) {
+		if (copy_from_user(&c, (char *)start, 1))
+			return -EFAULT;
+
+		start += PAGE_CACHE_SIZE;
+		start &= PAGE_CACHE_MASK;
+	}
+
+	return 0;
+}
+
+/**
  * walk_vm - Walk the vmas associated with a buffer for read or write.
  *    If any of them are gfs, pass the gfs inode down to the read/write
  *    worker function so that locks can be acquired in the correct order.
@@ -211,8 +242,11 @@ walk_vm(struct file *file, char *buf, size_t size, loff_t *offset,
 	struct kiocb *iocb,	
 	do_rw_t operation)
 {
+	int needs_groping = 0;
+
 	if (current->mm) {
 		struct super_block *sb = file->f_dentry->d_inode->i_sb;
+		struct file_system_type *type = file->f_dentry->d_inode->i_sb->s_type;
 		struct mm_struct *mm = current->mm;
 		struct vm_area_struct *vma;
 		unsigned long start = (unsigned long)buf;
@@ -228,6 +262,9 @@ walk_vm(struct file *file, char *buf, size_t size, loff_t *offset,
 			if (vma->vm_file &&
 			    vma->vm_file->f_dentry->d_inode->i_sb == sb)
 				goto do_locks;
+			else if (vma->vm_file &&
+				 vma->vm_file->f_dentry->d_inode->i_sb->s_type == type)
+				needs_groping = 1;
 		}
 
 		if (!dumping)
@@ -236,6 +273,8 @@ walk_vm(struct file *file, char *buf, size_t size, loff_t *offset,
 
 	{
 		struct gfs_holder gh;
+		if (needs_groping)
+			grope_mapping(buf, size);
 		return operation(file, buf, size, offset, iocb, 0, &gh);
 	}
 
@@ -284,37 +323,6 @@ do_read_readi(struct file *file, char *buf, size_t size, loff_t *offset,
 }
 
 /**
- * grope_mapping - feel up a mapping that needs to be written
- * @buf: the start of the memory to be written
- * @size: the size of the memory to be written
- *
- * We do this after acquiring the locks on the mapping,
- * but before starting the write transaction.  We need to make
- * sure that we don't cause recursive transactions if blocks
- * need to be allocated to the file backing the mapping.
- *
- * Returns: errno
- */
-
-static int
-grope_mapping(char *buf, size_t size)
-{
-	unsigned long start = (unsigned long)buf;
-	unsigned long stop = start + size;
-	char c;
-
-	while (start < stop) {
-		if (copy_from_user(&c, (char *)start, 1))
-			return -EFAULT;
-
-		start += PAGE_CACHE_SIZE;
-		start &= PAGE_CACHE_MASK;
-	}
-
-	return 0;
-}
-
-/**
  * do_read_direct - Read bytes from a file
  * @file: The file to read from
  * @buf: The buffer to copy into


More information about the cluster-commits mailing list