All of lore.kernel.org

* Re: possible deadlock in JFFS2
From: David Woodhouse @ 2004-01-26 12:44 UTC (permalink / raw)
  To: Pierre Vandwalle; +Cc: linux-fsdevel
In-Reply-To: <3FFBC907DD03A34CA4410C5C745DEB120339A735@wnimail.WoodsideNet.Com>

On Sat, 2004-01-24 at 03:46 -0800, Pierre Vandwalle wrote:
> The deadlock is between:
> 1- a first process in jffs2_setattr doing an open/create which require truncating the inode pages
> 2- a second process in do_generic_file_read on the same file

Thanks for the report. At first glance, I suspect that a quick fix would
be to move the vmtruncate() down to the end of jffs2_setattr(), after
we've dropped the jffs2-private inode semaphore.

This increases the likelihood that we'll get a simultaneous call to
readpage() which ends up attempting to read a page from an offset
_greater_ than the new end-of-file, but as far as I can tell, nothing in
generic_file_read() actually prevents that anyway, and we handle it OK
by returning a page full of zeroes.

Equivalent fix for CVS HEAD and 2.6 version, if you're using that, is at
http://lists.infradead.org/pipermail/linux-mtd-cvs/2004-January/003460.html

Index: fs/jffs2/file.c
===================================================================
RCS file: /home/cvs/mtd/fs/jffs2/file.c,v
retrieving revision 1.58.2.7
retrieving revision 1.58.2.8
diff -u -p -r1.58.2.7 -r1.58.2.8

--- fs/jffs2/file.c	2 Nov 2003 13:51:17 -0000	1.58.2.7
+++ fs/jffs2/file.c	26 Jan 2004 12:40:32 -0000	1.58.2.8
@@ -1,7 +1,7 @@
 /*
  * JFFS2 -- Journalling Flash File System, Version 2.
  *
- * Copyright (C) 2001 Red Hat, Inc.
+ * Copyright (C) 2001-2004 Red Hat, Inc.
  *
  * Created by David Woodhouse <dwmw2@cambridge.redhat.com>
  *
@@ -31,7 +31,7 @@
  * provisions above, a recipient may use your version of this file
  * under either the RHEPL or the GPL.
  *
- * $Id: file.c,v 1.58.2.7 2003/11/02 13:51:17 dwmw2 Exp $
+ * $Id: file.c,v 1.58.2.8 2004/01/26 12:40:32 dwmw2 Exp $
  *
  */
 
@@ -197,10 +197,8 @@ int jffs2_setattr (struct dentry *dentry
 
 	old_metadata = f->metadata;
 
-	if (inode->i_size > ri->isize) {
-		vmtruncate(inode, ri->isize);
+	if (inode->i_size > ri->isize)
 		jffs2_truncate_fraglist (c, &f->fraglist, ri->isize);
-	}
 
 	if (inode->i_size < ri->isize) {
 		jffs2_add_full_dnode_to_inode(c, f, new_metadata);
@@ -215,6 +213,15 @@ int jffs2_setattr (struct dentry *dentry
 	}
 	jffs2_free_raw_inode(ri);
 	up(&f->sem);
+
+	/* We have to do the vmtruncate() without f->sem held, since
+	   some pages may be locked and waiting for it in readpage().
+	   We are protected from a simultaneous write() extending i_size
+	   back past iattr->ia_size, because do_truncate() holds the
+	   generic inode semaphore. */
+	if (inode->i_size > ri->isize)
+		vmtruncate(inode, ri->isize);
+
 	return 0;
 }
 


-- 
dwmw2


^ permalink raw reply