nfsd: fix double-locks of directory mutex

A number of nfsd operations depend on the i_mutex to cover more code than just the fsync, so the approach of 4c728ef583b3d8 "add a vfs_fsync helper" doesn't work for nfsd. Revert the parts of those patches that touch nfsd. Note: we can't, however, remove the logic from vfs_fsync that was needed only for the special case of nfsd, because a vfs_fsync(NULL,...) call can still result indirectly from a stackable filesystem that was called by nfsd. (Thanks to Christoph Hellwig for pointing this out.) Reported-by: Eric Sesterhenn <snakebyte@gmx.de> Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>

nfsd: fix double-locks of directory mutex
A number of nfsd operations depend on the i_mutex to cover more code than just the fsync, so the approach of 4c728ef583b3d8 "add a vfs_fsync helper" doesn't work for nfsd. Revert the parts of those patches that touch nfsd. Note: we can't, however, remove the logic from vfs_fsync that was needed only for the special case of nfsd, because a vfs_fsync(NULL,...) call can still result indirectly from a stackable filesystem that was called by nfsd. (Thanks to Christoph Hellwig for pointing this out.) Reported-by: Eric Sesterhenn <snakebyte@gmx.de> Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
J. Bruce Fields
1 parent 2779e3ae39
Showing 1 changed file with 31 additions and 3 deletions Side-by-side Diff
fs/nfsd/vfs.c
@@ -744,16 +744,44 @@
 	fput(filp);
 }
  
+/*
+ * Sync a file
+ * As this calls fsync (not fdatasync) there is no need for a write_inode
+ * after it.
+ */
+static inline int nfsd_dosync(struct file *filp, struct dentry *dp,
+			      const struct file_operations *fop)
+{
+	struct inode *inode = dp->d_inode;
+	int (*fsync) (struct file *, struct dentry *, int);
+	int err;
+
+	err = filemap_fdatawrite(inode->i_mapping);
+	if (err == 0 && fop && (fsync = fop->fsync))
+		err = fsync(filp, dp, 0);
+	if (err == 0)
+		err = filemap_fdatawait(inode->i_mapping);
+
+	return err;
+}
+
 static int
 nfsd_sync(struct file *filp)
 {
-	return vfs_fsync(filp, filp->f_path.dentry, 0);
+        int err;
+	struct inode *inode = filp->f_path.dentry->d_inode;
+	dprintk("nfsd: sync file %s\n", filp->f_path.dentry->d_name.name);
+	mutex_lock(&inode->i_mutex);
+	err=nfsd_dosync(filp, filp->f_path.dentry, filp->f_op);
+	mutex_unlock(&inode->i_mutex);
+
+	return err;
 }
  
 int
-nfsd_sync_dir(struct dentry *dentry)
+nfsd_sync_dir(struct dentry *dp)
 {
-	return vfs_fsync(NULL, dentry, 0);
+	return nfsd_dosync(NULL, dp, dp->d_inode->i_fop);
 }
  
 /*
...	...	@@ -744,16 +744,44 @@
744	744	fput(filp);
745	745	}
746	746
	747	+/*
	748	+ * Sync a file
	749	+ * As this calls fsync (not fdatasync) there is no need for a write_inode
	750	+ * after it.
	751	+ */
	752	+static inline int nfsd_dosync(struct file filp, struct dentry dp,
	753	+ const struct file_operations *fop)
	754	+{
	755	+ struct inode *inode = dp->d_inode;
	756	+ int (fsync) (struct file , struct dentry *, int);
	757	+ int err;
	758	+
	759	+ err = filemap_fdatawrite(inode->i_mapping);
	760	+ if (err == 0 && fop && (fsync = fop->fsync))
	761	+ err = fsync(filp, dp, 0);
	762	+ if (err == 0)
	763	+ err = filemap_fdatawait(inode->i_mapping);
	764	+
	765	+ return err;
	766	+}
	767	+
747	768	static int
748	769	nfsd_sync(struct file *filp)
749	770	{
750		- return vfs_fsync(filp, filp->f_path.dentry, 0);
	771	+ int err;
	772	+ struct inode *inode = filp->f_path.dentry->d_inode;
	773	+ dprintk("nfsd: sync file %s\n", filp->f_path.dentry->d_name.name);
	774	+ mutex_lock(&inode->i_mutex);
	775	+ err=nfsd_dosync(filp, filp->f_path.dentry, filp->f_op);
	776	+ mutex_unlock(&inode->i_mutex);
	777	+
	778	+ return err;
751	779	}
752	780
753	781	int
754		-nfsd_sync_dir(struct dentry *dentry)
	782	+nfsd_sync_dir(struct dentry *dp)
755	783	{
756		- return vfs_fsync(NULL, dentry, 0);
	784	+ return nfsd_dosync(NULL, dp, dp->d_inode->i_fop);
757	785	}
758	786
759	787	/*