[PATCH 0/6] first step toward the new truncate sequence

[PATCH 0/6] first step toward the new truncate sequence

[Christoph Hellwig]

This series is something that I see as the first major step towards
a broad switch to the new truncate sequence.  The patches get rid
of the _newtrunc variant of blockdev_direct_IO & friends and
*_write_begin, and clean up some bits in that area that make the
switch easier.  After this we have all vmtruncate instances except
for inode_setattr in filesystem code.  A second series to deal
with ->setattr will follow and after that we can easily switch
over one filesystem after another.

I think this is still 2.6.34 material as it will make the fs
switches a lot easier and avoid introducing the _newtrunc variants
for one kernel release.

[PATCH 1/6] sort out blockdev_direct_IO variants

[Christoph Hellwig]

Move the call to vmtruncate to get rid of accessive blocks to the callers
in prepearation of the new truncate calling sequence.  This was only done
for DIO_LOCKING filesystems, so the __blockdev_direct_IO_newtrunc variant
was not needed anyway.  Get rid of blockdev_direct_IO_no_locking and
it's _newtrunc variant while at it as just opencoding the two additional
paramters is shorted than the name suffix.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/fs/block_dev.c
===================================================================
--- linux-2.6.orig/fs/block_dev.c	2010-05-30 15:10:19.692004346 +0200
+++ linux-2.6/fs/block_dev.c	2010-05-30 15:15:00.753273377 +0200
@@ -172,9 +172,8 @@ blkdev_direct_IO(int rw, struct kiocb *i
 	struct file *file = iocb->ki_filp;
 	struct inode *inode = file->f_mapping->host;
 
-	return blockdev_direct_IO_no_locking_newtrunc(rw, iocb, inode,
-				I_BDEV(inode), iov, offset, nr_segs,
-				blkdev_get_blocks, NULL);
+	return __blockdev_direct_IO(rw, iocb, inode, I_BDEV(inode), iov, offset,
+				    nr_segs, blkdev_get_blocks, NULL, NULL, 0);
 }
 
 int __sync_blockdev(struct block_device *bdev, int wait)
Index: linux-2.6/fs/direct-io.c
===================================================================
--- linux-2.6.orig/fs/direct-io.c	2010-05-30 15:10:19.711004136 +0200
+++ linux-2.6/fs/direct-io.c	2010-05-30 15:15:00.754273587 +0200
@@ -1134,8 +1134,27 @@ direct_io_worker(int rw, struct kiocb *i
 	return ret;
 }
 
+/*
+ * This is a library function for use by filesystem drivers.
+ *
+ * The locking rules are governed by the flags parameter:
+ *  - if the flags value contains DIO_LOCKING we use a fancy locking
+ *    scheme for dumb filesystems.
+ *    For writes this function is called under i_mutex and returns with
+ *    i_mutex held, for reads, i_mutex is not held on entry, but it is
+ *    taken and dropped again before returning.
+ *    For reads and writes i_alloc_sem is taken in shared mode and released
+ *    on I/O completion (which may happen asynchronously after returning to
+ *    the caller).
+ *
+ *  - if the flags value does NOT contain DIO_LOCKING we don't use any
+ *    internal locking but rather rely on the filesystem to synchronize
+ *    direct I/O reads/writes versus each other and truncate.
+ *    For reads and writes both i_mutex and i_alloc_sem are not held on
+ *    entry and are never taken.
+ */
 ssize_t
-__blockdev_direct_IO_newtrunc(int rw, struct kiocb *iocb, struct inode *inode,
+__blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
 	struct block_device *bdev, const struct iovec *iov, loff_t offset, 
 	unsigned long nr_segs, get_block_t get_block, dio_iodone_t end_io,
 	dio_submit_t submit_io,	int flags)
@@ -1231,57 +1250,4 @@ __blockdev_direct_IO_newtrunc(int rw, st
 out:
 	return retval;
 }
-EXPORT_SYMBOL(__blockdev_direct_IO_newtrunc);
-
-/*
- * This is a library function for use by filesystem drivers.
- *
- * The locking rules are governed by the flags parameter:
- *  - if the flags value contains DIO_LOCKING we use a fancy locking
- *    scheme for dumb filesystems.
- *    For writes this function is called under i_mutex and returns with
- *    i_mutex held, for reads, i_mutex is not held on entry, but it is
- *    taken and dropped again before returning.
- *    For reads and writes i_alloc_sem is taken in shared mode and released
- *    on I/O completion (which may happen asynchronously after returning to
- *    the caller).
- *
- *  - if the flags value does NOT contain DIO_LOCKING we don't use any
- *    internal locking but rather rely on the filesystem to synchronize
- *    direct I/O reads/writes versus each other and truncate.
- *    For reads and writes both i_mutex and i_alloc_sem are not held on
- *    entry and are never taken.
- */
-ssize_t
-__blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
-	struct block_device *bdev, const struct iovec *iov, loff_t offset,
-	unsigned long nr_segs, get_block_t get_block, dio_iodone_t end_io,
-	dio_submit_t submit_io,	int flags)
-{
-	ssize_t retval;
-
-	retval = __blockdev_direct_IO_newtrunc(rw, iocb, inode, bdev, iov,
-			offset, nr_segs, get_block, end_io, submit_io, flags);
-	/*
-	 * In case of error extending write may have instantiated a few
-	 * blocks outside i_size. Trim these off again for DIO_LOCKING.
-	 * NOTE: DIO_NO_LOCK/DIO_OWN_LOCK callers have to handle this in
-	 * their own manner. This is a further example of where the old
-	 * truncate sequence is inadequate.
-	 *
-	 * NOTE: filesystems with their own locking have to handle this
-	 * on their own.
-	 */
-	if (flags & DIO_LOCKING) {
-		if (unlikely((rw & WRITE) && retval < 0)) {
-			loff_t isize = i_size_read(inode);
-			loff_t end = offset + iov_length(iov, nr_segs);
-
-			if (end > isize)
-				vmtruncate(inode, isize);
-		}
-	}
-
-	return retval;
-}
 EXPORT_SYMBOL(__blockdev_direct_IO);
Index: linux-2.6/fs/ext4/inode.c
===================================================================
--- linux-2.6.orig/fs/ext4/inode.c	2010-05-30 15:10:19.719003158 +0200
+++ linux-2.6/fs/ext4/inode.c	2010-05-30 15:15:00.760003927 +0200
@@ -3545,15 +3545,24 @@ static ssize_t ext4_ind_direct_IO(int rw
 
 retry:
 	if (rw == READ && ext4_should_dioread_nolock(inode))
-		ret = blockdev_direct_IO_no_locking(rw, iocb, inode,
+		ret = __blockdev_direct_IO(rw, iocb, inode,
 				 inode->i_sb->s_bdev, iov,
 				 offset, nr_segs,
-				 ext4_get_block, NULL);
-	else
+				 ext4_get_block, NULL, NULL, 0);
+	else {
 		ret = blockdev_direct_IO(rw, iocb, inode,
 				 inode->i_sb->s_bdev, iov,
 				 offset, nr_segs,
 				 ext4_get_block, NULL);
+
+		if (unlikely((rw & WRITE) && ret < 0)) {
+			loff_t isize = i_size_read(inode);
+			loff_t end = offset + iov_length(iov, nr_segs);
+
+			if (end > isize)
+				vmtruncate(inode, isize);
+		}
+	}
 	if (ret == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
 		goto retry;
 
Index: linux-2.6/fs/gfs2/aops.c
===================================================================
--- linux-2.6.orig/fs/gfs2/aops.c	2010-05-30 15:10:19.727004485 +0200
+++ linux-2.6/fs/gfs2/aops.c	2010-05-30 15:15:00.768051419 +0200
@@ -1047,9 +1047,9 @@ static ssize_t gfs2_direct_IO(int rw, st
 	if (rv != 1)
 		goto out; /* dio not valid, fall back to buffered i/o */
 
-	rv = blockdev_direct_IO_no_locking(rw, iocb, inode, inode->i_sb->s_bdev,
-					   iov, offset, nr_segs,
-					   gfs2_get_block_direct, NULL);
+	rv = __blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
+				  offset, nr_segs, gfs2_get_block_direct,
+				  NULL, NULL, 0);
 out:
 	gfs2_glock_dq_m(1, &gh);
 	gfs2_holder_uninit(&gh);
Index: linux-2.6/fs/ocfs2/aops.c
===================================================================
--- linux-2.6.orig/fs/ocfs2/aops.c	2010-05-30 15:10:19.741004625 +0200
+++ linux-2.6/fs/ocfs2/aops.c	2010-05-30 15:15:00.776005603 +0200
@@ -669,11 +669,10 @@ static ssize_t ocfs2_direct_IO(int rw,
 	if (i_size_read(inode) <= offset)
 		return 0;
 
-	ret = blockdev_direct_IO_no_locking(rw, iocb, inode,
-					    inode->i_sb->s_bdev, iov, offset,
-					    nr_segs,
-					    ocfs2_direct_IO_get_blocks,
-					    ocfs2_dio_end_io);
+	ret = __blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev,
+				   iov, offset, nr_segs,
+				   ocfs2_direct_IO_get_blocks,
+				   ocfs2_dio_end_io, NULL, 0);
 
 	mlog_exit(ret);
 	return ret;
Index: linux-2.6/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- linux-2.6.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-05-30 15:10:19.752024390 +0200
+++ linux-2.6/fs/xfs/linux-2.6/xfs_aops.c	2010-05-30 15:15:00.781005533 +0200
@@ -1658,10 +1658,9 @@ xfs_vm_direct_IO(
 	iocb->private = xfs_alloc_ioend(inode, rw == WRITE ?
 					IO_UNWRITTEN : IO_READ);
 
-	ret = blockdev_direct_IO_no_locking(rw, iocb, inode, bdev, iov,
-					    offset, nr_segs,
-					    xfs_get_blocks_direct,
-					    xfs_end_io_direct);
+	ret = __blockdev_direct_IO(rw, iocb, inode, bdev, iov, offset,
+				   nr_segs, xfs_get_blocks_direct,
+				   xfs_end_io_direct, NULL, 0);
 
 	if (unlikely(ret != -EIOCBQUEUED && iocb->private))
 		xfs_destroy_ioend(iocb->private);
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2010-05-30 15:10:19.861004207 +0200
+++ linux-2.6/include/linux/fs.h	2010-05-30 15:15:00.787005812 +0200
@@ -2255,16 +2255,6 @@ static inline int xip_truncate_page(stru
 struct bio;
 typedef void (dio_submit_t)(int rw, struct bio *bio, struct inode *inode,
 			    loff_t file_offset);
-void dio_end_io(struct bio *bio, int error);
-
-ssize_t __blockdev_direct_IO_newtrunc(int rw, struct kiocb *iocb, struct inode *inode,
-	struct block_device *bdev, const struct iovec *iov, loff_t offset,
-	unsigned long nr_segs, get_block_t get_block, dio_iodone_t end_io,
-	dio_submit_t submit_io, int lock_type);
-ssize_t __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
-	struct block_device *bdev, const struct iovec *iov, loff_t offset,
-	unsigned long nr_segs, get_block_t get_block, dio_iodone_t end_io,
-	dio_submit_t submit_io,	int lock_type);
 
 enum {
 	/* need locking between buffered and direct access */
@@ -2274,24 +2264,13 @@ enum {
 	DIO_SKIP_HOLES	= 0x02,
 };
 
-static inline ssize_t blockdev_direct_IO_newtrunc(int rw, struct kiocb *iocb,
-	struct inode *inode, struct block_device *bdev, const struct iovec *iov,
-	loff_t offset, unsigned long nr_segs, get_block_t get_block,
-	dio_iodone_t end_io)
-{
-	return __blockdev_direct_IO_newtrunc(rw, iocb, inode, bdev, iov, offset,
-				    nr_segs, get_block, end_io, NULL,
-				    DIO_LOCKING | DIO_SKIP_HOLES);
-}
+void dio_end_io(struct bio *bio, int error);
+
+ssize_t __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
+	struct block_device *bdev, const struct iovec *iov, loff_t offset,
+	unsigned long nr_segs, get_block_t get_block, dio_iodone_t end_io,
+	dio_submit_t submit_io,	int flags);
 
-static inline ssize_t blockdev_direct_IO_no_locking_newtrunc(int rw, struct kiocb *iocb,
-	struct inode *inode, struct block_device *bdev, const struct iovec *iov,
-	loff_t offset, unsigned long nr_segs, get_block_t get_block,
-	dio_iodone_t end_io)
-{
-	return __blockdev_direct_IO_newtrunc(rw, iocb, inode, bdev, iov, offset,
-				nr_segs, get_block, end_io, NULL, 0);
-}
 static inline ssize_t blockdev_direct_IO(int rw, struct kiocb *iocb,
 	struct inode *inode, struct block_device *bdev, const struct iovec *iov,
 	loff_t offset, unsigned long nr_segs, get_block_t get_block,
@@ -2301,15 +2280,6 @@ static inline ssize_t blockdev_direct_IO
 				    nr_segs, get_block, end_io, NULL,
 				    DIO_LOCKING | DIO_SKIP_HOLES);
 }
-
-static inline ssize_t blockdev_direct_IO_no_locking(int rw, struct kiocb *iocb,
-	struct inode *inode, struct block_device *bdev, const struct iovec *iov,
-	loff_t offset, unsigned long nr_segs, get_block_t get_block,
-	dio_iodone_t end_io)
-{
-	return __blockdev_direct_IO(rw, iocb, inode, bdev, iov, offset,
-				    nr_segs, get_block, end_io, NULL, 0);
-}
 #endif
 
 extern const struct file_operations generic_ro_fops;
Index: linux-2.6/fs/ext2/inode.c
===================================================================
--- linux-2.6.orig/fs/ext2/inode.c	2010-05-30 15:10:26.785274006 +0200
+++ linux-2.6/fs/ext2/inode.c	2010-05-30 15:15:00.792005673 +0200
@@ -831,7 +831,7 @@ ext2_direct_IO(int rw, struct kiocb *ioc
 	struct inode *inode = mapping->host;
 	ssize_t ret;
 
-	ret = blockdev_direct_IO_newtrunc(rw, iocb, inode, inode->i_sb->s_bdev,
+	ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev,
 				iov, offset, nr_segs, ext2_get_block, NULL);
 	if (ret < 0 && (rw & WRITE))
 		ext2_write_failed(mapping, offset + iov_length(iov, nr_segs));
Index: linux-2.6/fs/ext3/inode.c
===================================================================
--- linux-2.6.orig/fs/ext3/inode.c	2010-05-30 15:10:19.774004346 +0200
+++ linux-2.6/fs/ext3/inode.c	2010-05-30 15:15:00.798005533 +0200
@@ -1785,6 +1785,17 @@ retry:
 	ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
 				 offset, nr_segs,
 				 ext3_get_block, NULL);
+	/*
+	 * In case of error extending write may have instantiated a few
+	 * blocks outside i_size. Trim these off again.
+	 */
+	if (unlikely((rw & WRITE) && ret < 0)) {
+		loff_t isize = i_size_read(inode);
+		loff_t end = offset + iov_length(iov, nr_segs);
+
+		if (end > isize)
+			vmtruncate(inode, isize);
+	}
 	if (ret == -ENOSPC && ext3_should_retry_alloc(inode->i_sb, &retries))
 		goto retry;
 
Index: linux-2.6/fs/fat/inode.c
===================================================================
--- linux-2.6.orig/fs/fat/inode.c	2010-05-30 15:10:19.795006092 +0200
+++ linux-2.6/fs/fat/inode.c	2010-05-30 15:15:00.806067692 +0200
@@ -212,8 +212,8 @@ static ssize_t fat_direct_IO(int rw, str
 	 * FAT need to use the DIO_LOCKING for avoiding the race
 	 * condition of fat_get_block() and ->truncate().
 	 */
-	ret = blockdev_direct_IO_newtrunc(rw, iocb, inode, inode->i_sb->s_bdev,
-				iov, offset, nr_segs, fat_get_block, NULL);
+	ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev,
+				 iov, offset, nr_segs, fat_get_block, NULL);
 	if (ret < 0 && (rw & WRITE))
 		fat_write_failed(mapping, offset + iov_length(iov, nr_segs));
 
Index: linux-2.6/fs/hfs/inode.c
===================================================================
--- linux-2.6.orig/fs/hfs/inode.c	2010-05-30 15:10:19.808003788 +0200
+++ linux-2.6/fs/hfs/inode.c	2010-05-30 15:15:00.810051978 +0200
@@ -112,9 +112,24 @@ static ssize_t hfs_direct_IO(int rw, str
 {
 	struct file *file = iocb->ki_filp;
 	struct inode *inode = file->f_path.dentry->d_inode->i_mapping->host;
+	ssize_t ret;
 
-	return blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
+	ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
 				  offset, nr_segs, hfs_get_block, NULL);
+
+	/*
+	 * In case of error extending write may have instantiated a few
+	 * blocks outside i_size. Trim these off again.
+	 */
+	if (unlikely((rw & WRITE) && ret < 0)) {
+		loff_t isize = i_size_read(inode);
+		loff_t end = offset + iov_length(iov, nr_segs);
+
+		if (end > isize)
+			vmtruncate(inode, isize);
+	}
+
+	return ret;
 }
 
 static int hfs_writepages(struct address_space *mapping,
Index: linux-2.6/fs/hfsplus/inode.c
===================================================================
--- linux-2.6.orig/fs/hfsplus/inode.c	2010-05-30 15:10:19.816011470 +0200
+++ linux-2.6/fs/hfsplus/inode.c	2010-05-30 15:15:00.817051489 +0200
@@ -105,9 +105,24 @@ static ssize_t hfsplus_direct_IO(int rw,
 {
 	struct file *file = iocb->ki_filp;
 	struct inode *inode = file->f_path.dentry->d_inode->i_mapping->host;
+	ssize_t ret;
 
-	return blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
+	ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
 				  offset, nr_segs, hfsplus_get_block, NULL);
+
+	/*
+	 * In case of error extending write may have instantiated a few
+	 * blocks outside i_size. Trim these off again.
+	 */
+	if (unlikely((rw & WRITE) && ret < 0)) {
+		loff_t isize = i_size_read(inode);
+		loff_t end = offset + iov_length(iov, nr_segs);
+
+		if (end > isize)
+			vmtruncate(inode, isize);
+	}
+
+	return ret;
 }
 
 static int hfsplus_writepages(struct address_space *mapping,
Index: linux-2.6/fs/jfs/inode.c
===================================================================
--- linux-2.6.orig/fs/jfs/inode.c	2010-05-30 15:10:19.826004346 +0200
+++ linux-2.6/fs/jfs/inode.c	2010-05-30 15:15:00.824005463 +0200
@@ -317,9 +317,24 @@ static ssize_t jfs_direct_IO(int rw, str
 {
 	struct file *file = iocb->ki_filp;
 	struct inode *inode = file->f_mapping->host;
+	ssize_t ret;
 
-	return blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
+	ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
 				offset, nr_segs, jfs_get_block, NULL);
+
+	/*
+	 * In case of error extending write may have instantiated a few
+	 * blocks outside i_size. Trim these off again.
+	 */
+	if (unlikely((rw & WRITE) && ret < 0)) {
+		loff_t isize = i_size_read(inode);
+		loff_t end = offset + iov_length(iov, nr_segs);
+
+		if (end > isize)
+			vmtruncate(inode, isize);
+	}
+
+	return ret;
 }
 
 const struct address_space_operations jfs_aops = {
Index: linux-2.6/fs/nilfs2/inode.c
===================================================================
--- linux-2.6.orig/fs/nilfs2/inode.c	2010-05-30 15:10:19.837004346 +0200
+++ linux-2.6/fs/nilfs2/inode.c	2010-05-30 15:15:00.830005533 +0200
@@ -237,6 +237,19 @@ nilfs_direct_IO(int rw, struct kiocb *io
 	/* Needs synchronization with the cleaner */
 	size = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
 				  offset, nr_segs, nilfs_get_block, NULL);
+
+	/*
+	 * In case of error extending write may have instantiated a few
+	 * blocks outside i_size. Trim these off again.
+	 */
+	if (unlikely((rw & WRITE) && size < 0)) {
+		loff_t isize = i_size_read(inode);
+		loff_t end = offset + iov_length(iov, nr_segs);
+
+		if (end > isize)
+			vmtruncate(inode, isize);
+	}
+
 	return size;
 }
 
Index: linux-2.6/fs/reiserfs/inode.c
===================================================================
--- linux-2.6.orig/fs/reiserfs/inode.c	2010-05-30 15:10:19.848012727 +0200
+++ linux-2.6/fs/reiserfs/inode.c	2010-05-30 15:15:00.835005812 +0200
@@ -3059,10 +3059,25 @@ static ssize_t reiserfs_direct_IO(int rw
 {
 	struct file *file = iocb->ki_filp;
 	struct inode *inode = file->f_mapping->host;
+	ssize_t ret;
 
-	return blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
+	ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
 				  offset, nr_segs,
 				  reiserfs_get_blocks_direct_io, NULL);
+
+	/*
+	 * In case of error extending write may have instantiated a few
+	 * blocks outside i_size. Trim these off again.
+	 */
+	if (unlikely((rw & WRITE) && ret < 0)) {
+		loff_t isize = i_size_read(inode);
+		loff_t end = offset + iov_length(iov, nr_segs);
+
+		if (end > isize)
+			vmtruncate(inode, isize);
+	}
+
+	return ret;
 }
 
 int reiserfs_setattr(struct dentry *dentry, struct iattr *attr)

Re: [PATCH 1/6] sort out blockdev_direct_IO variants

[Jan Kara]

On Sun 30-05-10 22:49:42, Christoph Hellwig wrote:
> Move the call to vmtruncate to get rid of accessive blocks to the callers
> in prepearation of the new truncate calling sequence.  This was only done
> for DIO_LOCKING filesystems, so the __blockdev_direct_IO_newtrunc variant
> was not needed anyway.  Get rid of blockdev_direct_IO_no_locking and
> it's _newtrunc variant while at it as just opencoding the two additional
> paramters is shorted than the name suffix.
  Hmm, I've noticed that you didn't add truncate code to btrfs. I see that
it already does some recovery after direct IO error but I'd like to make
sure that it won't be broken by your patches... Chris?

								Honza
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: linux-2.6/fs/block_dev.c
> ===================================================================
> --- linux-2.6.orig/fs/block_dev.c	2010-05-30 15:10:19.692004346 +0200
> +++ linux-2.6/fs/block_dev.c	2010-05-30 15:15:00.753273377 +0200
> @@ -172,9 +172,8 @@ blkdev_direct_IO(int rw, struct kiocb *i
>  	struct file *file = iocb->ki_filp;
>  	struct inode *inode = file->f_mapping->host;
>  
> -	return blockdev_direct_IO_no_locking_newtrunc(rw, iocb, inode,
> -				I_BDEV(inode), iov, offset, nr_segs,
> -				blkdev_get_blocks, NULL);
> +	return __blockdev_direct_IO(rw, iocb, inode, I_BDEV(inode), iov, offset,
> +				    nr_segs, blkdev_get_blocks, NULL, NULL, 0);
>  }
>  
>  int __sync_blockdev(struct block_device *bdev, int wait)
> Index: linux-2.6/fs/direct-io.c
> ===================================================================
> --- linux-2.6.orig/fs/direct-io.c	2010-05-30 15:10:19.711004136 +0200
> +++ linux-2.6/fs/direct-io.c	2010-05-30 15:15:00.754273587 +0200
> @@ -1134,8 +1134,27 @@ direct_io_worker(int rw, struct kiocb *i
>  	return ret;
>  }
>  
> +/*
> + * This is a library function for use by filesystem drivers.
> + *
> + * The locking rules are governed by the flags parameter:
> + *  - if the flags value contains DIO_LOCKING we use a fancy locking
> + *    scheme for dumb filesystems.
> + *    For writes this function is called under i_mutex and returns with
> + *    i_mutex held, for reads, i_mutex is not held on entry, but it is
> + *    taken and dropped again before returning.
> + *    For reads and writes i_alloc_sem is taken in shared mode and released
> + *    on I/O completion (which may happen asynchronously after returning to
> + *    the caller).
> + *
> + *  - if the flags value does NOT contain DIO_LOCKING we don't use any
> + *    internal locking but rather rely on the filesystem to synchronize
> + *    direct I/O reads/writes versus each other and truncate.
> + *    For reads and writes both i_mutex and i_alloc_sem are not held on
> + *    entry and are never taken.
> + */
>  ssize_t
> -__blockdev_direct_IO_newtrunc(int rw, struct kiocb *iocb, struct inode *inode,
> +__blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
>  	struct block_device *bdev, const struct iovec *iov, loff_t offset, 
>  	unsigned long nr_segs, get_block_t get_block, dio_iodone_t end_io,
>  	dio_submit_t submit_io,	int flags)
> @@ -1231,57 +1250,4 @@ __blockdev_direct_IO_newtrunc(int rw, st
>  out:
>  	return retval;
>  }
> -EXPORT_SYMBOL(__blockdev_direct_IO_newtrunc);
> -
> -/*
> - * This is a library function for use by filesystem drivers.
> - *
> - * The locking rules are governed by the flags parameter:
> - *  - if the flags value contains DIO_LOCKING we use a fancy locking
> - *    scheme for dumb filesystems.
> - *    For writes this function is called under i_mutex and returns with
> - *    i_mutex held, for reads, i_mutex is not held on entry, but it is
> - *    taken and dropped again before returning.
> - *    For reads and writes i_alloc_sem is taken in shared mode and released
> - *    on I/O completion (which may happen asynchronously after returning to
> - *    the caller).
> - *
> - *  - if the flags value does NOT contain DIO_LOCKING we don't use any
> - *    internal locking but rather rely on the filesystem to synchronize
> - *    direct I/O reads/writes versus each other and truncate.
> - *    For reads and writes both i_mutex and i_alloc_sem are not held on
> - *    entry and are never taken.
> - */
> -ssize_t
> -__blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
> -	struct block_device *bdev, const struct iovec *iov, loff_t offset,
> -	unsigned long nr_segs, get_block_t get_block, dio_iodone_t end_io,
> -	dio_submit_t submit_io,	int flags)
> -{
> -	ssize_t retval;
> -
> -	retval = __blockdev_direct_IO_newtrunc(rw, iocb, inode, bdev, iov,
> -			offset, nr_segs, get_block, end_io, submit_io, flags);
> -	/*
> -	 * In case of error extending write may have instantiated a few
> -	 * blocks outside i_size. Trim these off again for DIO_LOCKING.
> -	 * NOTE: DIO_NO_LOCK/DIO_OWN_LOCK callers have to handle this in
> -	 * their own manner. This is a further example of where the old
> -	 * truncate sequence is inadequate.
> -	 *
> -	 * NOTE: filesystems with their own locking have to handle this
> -	 * on their own.
> -	 */
> -	if (flags & DIO_LOCKING) {
> -		if (unlikely((rw & WRITE) && retval < 0)) {
> -			loff_t isize = i_size_read(inode);
> -			loff_t end = offset + iov_length(iov, nr_segs);
> -
> -			if (end > isize)
> -				vmtruncate(inode, isize);
> -		}
> -	}
> -
> -	return retval;
> -}
>  EXPORT_SYMBOL(__blockdev_direct_IO);
> Index: linux-2.6/fs/ext4/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/ext4/inode.c	2010-05-30 15:10:19.719003158 +0200
> +++ linux-2.6/fs/ext4/inode.c	2010-05-30 15:15:00.760003927 +0200
> @@ -3545,15 +3545,24 @@ static ssize_t ext4_ind_direct_IO(int rw
>  
>  retry:
>  	if (rw == READ && ext4_should_dioread_nolock(inode))
> -		ret = blockdev_direct_IO_no_locking(rw, iocb, inode,
> +		ret = __blockdev_direct_IO(rw, iocb, inode,
>  				 inode->i_sb->s_bdev, iov,
>  				 offset, nr_segs,
> -				 ext4_get_block, NULL);
> -	else
> +				 ext4_get_block, NULL, NULL, 0);
> +	else {
>  		ret = blockdev_direct_IO(rw, iocb, inode,
>  				 inode->i_sb->s_bdev, iov,
>  				 offset, nr_segs,
>  				 ext4_get_block, NULL);
> +
> +		if (unlikely((rw & WRITE) && ret < 0)) {
> +			loff_t isize = i_size_read(inode);
> +			loff_t end = offset + iov_length(iov, nr_segs);
> +
> +			if (end > isize)
> +				vmtruncate(inode, isize);
> +		}
> +	}
>  	if (ret == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
>  		goto retry;
>  
> Index: linux-2.6/fs/gfs2/aops.c
> ===================================================================
> --- linux-2.6.orig/fs/gfs2/aops.c	2010-05-30 15:10:19.727004485 +0200
> +++ linux-2.6/fs/gfs2/aops.c	2010-05-30 15:15:00.768051419 +0200
> @@ -1047,9 +1047,9 @@ static ssize_t gfs2_direct_IO(int rw, st
>  	if (rv != 1)
>  		goto out; /* dio not valid, fall back to buffered i/o */
>  
> -	rv = blockdev_direct_IO_no_locking(rw, iocb, inode, inode->i_sb->s_bdev,
> -					   iov, offset, nr_segs,
> -					   gfs2_get_block_direct, NULL);
> +	rv = __blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
> +				  offset, nr_segs, gfs2_get_block_direct,
> +				  NULL, NULL, 0);
>  out:
>  	gfs2_glock_dq_m(1, &gh);
>  	gfs2_holder_uninit(&gh);
> Index: linux-2.6/fs/ocfs2/aops.c
> ===================================================================
> --- linux-2.6.orig/fs/ocfs2/aops.c	2010-05-30 15:10:19.741004625 +0200
> +++ linux-2.6/fs/ocfs2/aops.c	2010-05-30 15:15:00.776005603 +0200
> @@ -669,11 +669,10 @@ static ssize_t ocfs2_direct_IO(int rw,
>  	if (i_size_read(inode) <= offset)
>  		return 0;
>  
> -	ret = blockdev_direct_IO_no_locking(rw, iocb, inode,
> -					    inode->i_sb->s_bdev, iov, offset,
> -					    nr_segs,
> -					    ocfs2_direct_IO_get_blocks,
> -					    ocfs2_dio_end_io);
> +	ret = __blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev,
> +				   iov, offset, nr_segs,
> +				   ocfs2_direct_IO_get_blocks,
> +				   ocfs2_dio_end_io, NULL, 0);
>  
>  	mlog_exit(ret);
>  	return ret;
> Index: linux-2.6/fs/xfs/linux-2.6/xfs_aops.c
> ===================================================================
> --- linux-2.6.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-05-30 15:10:19.752024390 +0200
> +++ linux-2.6/fs/xfs/linux-2.6/xfs_aops.c	2010-05-30 15:15:00.781005533 +0200
> @@ -1658,10 +1658,9 @@ xfs_vm_direct_IO(
>  	iocb->private = xfs_alloc_ioend(inode, rw == WRITE ?
>  					IO_UNWRITTEN : IO_READ);
>  
> -	ret = blockdev_direct_IO_no_locking(rw, iocb, inode, bdev, iov,
> -					    offset, nr_segs,
> -					    xfs_get_blocks_direct,
> -					    xfs_end_io_direct);
> +	ret = __blockdev_direct_IO(rw, iocb, inode, bdev, iov, offset,
> +				   nr_segs, xfs_get_blocks_direct,
> +				   xfs_end_io_direct, NULL, 0);
>  
>  	if (unlikely(ret != -EIOCBQUEUED && iocb->private))
>  		xfs_destroy_ioend(iocb->private);
> Index: linux-2.6/include/linux/fs.h
> ===================================================================
> --- linux-2.6.orig/include/linux/fs.h	2010-05-30 15:10:19.861004207 +0200
> +++ linux-2.6/include/linux/fs.h	2010-05-30 15:15:00.787005812 +0200
> @@ -2255,16 +2255,6 @@ static inline int xip_truncate_page(stru
>  struct bio;
>  typedef void (dio_submit_t)(int rw, struct bio *bio, struct inode *inode,
>  			    loff_t file_offset);
> -void dio_end_io(struct bio *bio, int error);
> -
> -ssize_t __blockdev_direct_IO_newtrunc(int rw, struct kiocb *iocb, struct inode *inode,
> -	struct block_device *bdev, const struct iovec *iov, loff_t offset,
> -	unsigned long nr_segs, get_block_t get_block, dio_iodone_t end_io,
> -	dio_submit_t submit_io, int lock_type);
> -ssize_t __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
> -	struct block_device *bdev, const struct iovec *iov, loff_t offset,
> -	unsigned long nr_segs, get_block_t get_block, dio_iodone_t end_io,
> -	dio_submit_t submit_io,	int lock_type);
>  
>  enum {
>  	/* need locking between buffered and direct access */
> @@ -2274,24 +2264,13 @@ enum {
>  	DIO_SKIP_HOLES	= 0x02,
>  };
>  
> -static inline ssize_t blockdev_direct_IO_newtrunc(int rw, struct kiocb *iocb,
> -	struct inode *inode, struct block_device *bdev, const struct iovec *iov,
> -	loff_t offset, unsigned long nr_segs, get_block_t get_block,
> -	dio_iodone_t end_io)
> -{
> -	return __blockdev_direct_IO_newtrunc(rw, iocb, inode, bdev, iov, offset,
> -				    nr_segs, get_block, end_io, NULL,
> -				    DIO_LOCKING | DIO_SKIP_HOLES);
> -}
> +void dio_end_io(struct bio *bio, int error);
> +
> +ssize_t __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
> +	struct block_device *bdev, const struct iovec *iov, loff_t offset,
> +	unsigned long nr_segs, get_block_t get_block, dio_iodone_t end_io,
> +	dio_submit_t submit_io,	int flags);
>  
> -static inline ssize_t blockdev_direct_IO_no_locking_newtrunc(int rw, struct kiocb *iocb,
> -	struct inode *inode, struct block_device *bdev, const struct iovec *iov,
> -	loff_t offset, unsigned long nr_segs, get_block_t get_block,
> -	dio_iodone_t end_io)
> -{
> -	return __blockdev_direct_IO_newtrunc(rw, iocb, inode, bdev, iov, offset,
> -				nr_segs, get_block, end_io, NULL, 0);
> -}
>  static inline ssize_t blockdev_direct_IO(int rw, struct kiocb *iocb,
>  	struct inode *inode, struct block_device *bdev, const struct iovec *iov,
>  	loff_t offset, unsigned long nr_segs, get_block_t get_block,
> @@ -2301,15 +2280,6 @@ static inline ssize_t blockdev_direct_IO
>  				    nr_segs, get_block, end_io, NULL,
>  				    DIO_LOCKING | DIO_SKIP_HOLES);
>  }
> -
> -static inline ssize_t blockdev_direct_IO_no_locking(int rw, struct kiocb *iocb,
> -	struct inode *inode, struct block_device *bdev, const struct iovec *iov,
> -	loff_t offset, unsigned long nr_segs, get_block_t get_block,
> -	dio_iodone_t end_io)
> -{
> -	return __blockdev_direct_IO(rw, iocb, inode, bdev, iov, offset,
> -				    nr_segs, get_block, end_io, NULL, 0);
> -}
>  #endif
>  
>  extern const struct file_operations generic_ro_fops;
> Index: linux-2.6/fs/ext2/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/ext2/inode.c	2010-05-30 15:10:26.785274006 +0200
> +++ linux-2.6/fs/ext2/inode.c	2010-05-30 15:15:00.792005673 +0200
> @@ -831,7 +831,7 @@ ext2_direct_IO(int rw, struct kiocb *ioc
>  	struct inode *inode = mapping->host;
>  	ssize_t ret;
>  
> -	ret = blockdev_direct_IO_newtrunc(rw, iocb, inode, inode->i_sb->s_bdev,
> +	ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev,
>  				iov, offset, nr_segs, ext2_get_block, NULL);
>  	if (ret < 0 && (rw & WRITE))
>  		ext2_write_failed(mapping, offset + iov_length(iov, nr_segs));
> Index: linux-2.6/fs/ext3/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/ext3/inode.c	2010-05-30 15:10:19.774004346 +0200
> +++ linux-2.6/fs/ext3/inode.c	2010-05-30 15:15:00.798005533 +0200
> @@ -1785,6 +1785,17 @@ retry:
>  	ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
>  				 offset, nr_segs,
>  				 ext3_get_block, NULL);
> +	/*
> +	 * In case of error extending write may have instantiated a few
> +	 * blocks outside i_size. Trim these off again.
> +	 */
> +	if (unlikely((rw & WRITE) && ret < 0)) {
> +		loff_t isize = i_size_read(inode);
> +		loff_t end = offset + iov_length(iov, nr_segs);
> +
> +		if (end > isize)
> +			vmtruncate(inode, isize);
> +	}
>  	if (ret == -ENOSPC && ext3_should_retry_alloc(inode->i_sb, &retries))
>  		goto retry;
>  
> Index: linux-2.6/fs/fat/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/fat/inode.c	2010-05-30 15:10:19.795006092 +0200
> +++ linux-2.6/fs/fat/inode.c	2010-05-30 15:15:00.806067692 +0200
> @@ -212,8 +212,8 @@ static ssize_t fat_direct_IO(int rw, str
>  	 * FAT need to use the DIO_LOCKING for avoiding the race
>  	 * condition of fat_get_block() and ->truncate().
>  	 */
> -	ret = blockdev_direct_IO_newtrunc(rw, iocb, inode, inode->i_sb->s_bdev,
> -				iov, offset, nr_segs, fat_get_block, NULL);
> +	ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev,
> +				 iov, offset, nr_segs, fat_get_block, NULL);
>  	if (ret < 0 && (rw & WRITE))
>  		fat_write_failed(mapping, offset + iov_length(iov, nr_segs));
>  
> Index: linux-2.6/fs/hfs/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/hfs/inode.c	2010-05-30 15:10:19.808003788 +0200
> +++ linux-2.6/fs/hfs/inode.c	2010-05-30 15:15:00.810051978 +0200
> @@ -112,9 +112,24 @@ static ssize_t hfs_direct_IO(int rw, str
>  {
>  	struct file *file = iocb->ki_filp;
>  	struct inode *inode = file->f_path.dentry->d_inode->i_mapping->host;
> +	ssize_t ret;
>  
> -	return blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
> +	ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
>  				  offset, nr_segs, hfs_get_block, NULL);
> +
> +	/*
> +	 * In case of error extending write may have instantiated a few
> +	 * blocks outside i_size. Trim these off again.
> +	 */
> +	if (unlikely((rw & WRITE) && ret < 0)) {
> +		loff_t isize = i_size_read(inode);
> +		loff_t end = offset + iov_length(iov, nr_segs);
> +
> +		if (end > isize)
> +			vmtruncate(inode, isize);
> +	}
> +
> +	return ret;
>  }
>  
>  static int hfs_writepages(struct address_space *mapping,
> Index: linux-2.6/fs/hfsplus/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/hfsplus/inode.c	2010-05-30 15:10:19.816011470 +0200
> +++ linux-2.6/fs/hfsplus/inode.c	2010-05-30 15:15:00.817051489 +0200
> @@ -105,9 +105,24 @@ static ssize_t hfsplus_direct_IO(int rw,
>  {
>  	struct file *file = iocb->ki_filp;
>  	struct inode *inode = file->f_path.dentry->d_inode->i_mapping->host;
> +	ssize_t ret;
>  
> -	return blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
> +	ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
>  				  offset, nr_segs, hfsplus_get_block, NULL);
> +
> +	/*
> +	 * In case of error extending write may have instantiated a few
> +	 * blocks outside i_size. Trim these off again.
> +	 */
> +	if (unlikely((rw & WRITE) && ret < 0)) {
> +		loff_t isize = i_size_read(inode);
> +		loff_t end = offset + iov_length(iov, nr_segs);
> +
> +		if (end > isize)
> +			vmtruncate(inode, isize);
> +	}
> +
> +	return ret;
>  }
>  
>  static int hfsplus_writepages(struct address_space *mapping,
> Index: linux-2.6/fs/jfs/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/jfs/inode.c	2010-05-30 15:10:19.826004346 +0200
> +++ linux-2.6/fs/jfs/inode.c	2010-05-30 15:15:00.824005463 +0200
> @@ -317,9 +317,24 @@ static ssize_t jfs_direct_IO(int rw, str
>  {
>  	struct file *file = iocb->ki_filp;
>  	struct inode *inode = file->f_mapping->host;
> +	ssize_t ret;
>  
> -	return blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
> +	ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
>  				offset, nr_segs, jfs_get_block, NULL);
> +
> +	/*
> +	 * In case of error extending write may have instantiated a few
> +	 * blocks outside i_size. Trim these off again.
> +	 */
> +	if (unlikely((rw & WRITE) && ret < 0)) {
> +		loff_t isize = i_size_read(inode);
> +		loff_t end = offset + iov_length(iov, nr_segs);
> +
> +		if (end > isize)
> +			vmtruncate(inode, isize);
> +	}
> +
> +	return ret;
>  }
>  
>  const struct address_space_operations jfs_aops = {
> Index: linux-2.6/fs/nilfs2/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/nilfs2/inode.c	2010-05-30 15:10:19.837004346 +0200
> +++ linux-2.6/fs/nilfs2/inode.c	2010-05-30 15:15:00.830005533 +0200
> @@ -237,6 +237,19 @@ nilfs_direct_IO(int rw, struct kiocb *io
>  	/* Needs synchronization with the cleaner */
>  	size = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
>  				  offset, nr_segs, nilfs_get_block, NULL);
> +
> +	/*
> +	 * In case of error extending write may have instantiated a few
> +	 * blocks outside i_size. Trim these off again.
> +	 */
> +	if (unlikely((rw & WRITE) && size < 0)) {
> +		loff_t isize = i_size_read(inode);
> +		loff_t end = offset + iov_length(iov, nr_segs);
> +
> +		if (end > isize)
> +			vmtruncate(inode, isize);
> +	}
> +
>  	return size;
>  }
>  
> Index: linux-2.6/fs/reiserfs/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/reiserfs/inode.c	2010-05-30 15:10:19.848012727 +0200
> +++ linux-2.6/fs/reiserfs/inode.c	2010-05-30 15:15:00.835005812 +0200
> @@ -3059,10 +3059,25 @@ static ssize_t reiserfs_direct_IO(int rw
>  {
>  	struct file *file = iocb->ki_filp;
>  	struct inode *inode = file->f_mapping->host;
> +	ssize_t ret;
>  
> -	return blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
> +	ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
>  				  offset, nr_segs,
>  				  reiserfs_get_blocks_direct_io, NULL);
> +
> +	/*
> +	 * In case of error extending write may have instantiated a few
> +	 * blocks outside i_size. Trim these off again.
> +	 */
> +	if (unlikely((rw & WRITE) && ret < 0)) {
> +		loff_t isize = i_size_read(inode);
> +		loff_t end = offset + iov_length(iov, nr_segs);
> +
> +		if (end > isize)
> +			vmtruncate(inode, isize);
> +	}
> +
> +	return ret;
>  }
>  
>  int reiserfs_setattr(struct dentry *dentry, struct iattr *attr)
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 1/6] sort out blockdev_direct_IO variants

[Christoph Hellwig]

On Wed, Jun 02, 2010 at 01:00:14AM +0200, Jan Kara wrote:
> On Sun 30-05-10 22:49:42, Christoph Hellwig wrote:
> > Move the call to vmtruncate to get rid of accessive blocks to the callers
> > in prepearation of the new truncate calling sequence.  This was only done
> > for DIO_LOCKING filesystems, so the __blockdev_direct_IO_newtrunc variant
> > was not needed anyway.  Get rid of blockdev_direct_IO_no_locking and
> > it's _newtrunc variant while at it as just opencoding the two additional
> > paramters is shorted than the name suffix.
>   Hmm, I've noticed that you didn't add truncate code to btrfs. I see that
> it already does some recovery after direct IO error but I'd like to make
> sure that it won't be broken by your patches... Chris?

As mentioned above __blockdev_direct_IO only called vmtruncate for the
DIO_LOCKING, which does not include btrfs.  And yes, there's quite a bit
of things that need cleanup but didn't have it before due to the way
the cleanup code was hidden, but that's not something I want to do
in this transformation series.

Re: [PATCH 1/6] sort out blockdev_direct_IO variants

[Jan Kara]

On Wed 02-06-10 01:12:02, Christoph Hellwig wrote:
> On Wed, Jun 02, 2010 at 01:00:14AM +0200, Jan Kara wrote:
> > On Sun 30-05-10 22:49:42, Christoph Hellwig wrote:
> > > Move the call to vmtruncate to get rid of accessive blocks to the callers
> > > in prepearation of the new truncate calling sequence.  This was only done
> > > for DIO_LOCKING filesystems, so the __blockdev_direct_IO_newtrunc variant
> > > was not needed anyway.  Get rid of blockdev_direct_IO_no_locking and
> > > it's _newtrunc variant while at it as just opencoding the two additional
> > > paramters is shorted than the name suffix.
> >   Hmm, I've noticed that you didn't add truncate code to btrfs. I see that
> > it already does some recovery after direct IO error but I'd like to make
> > sure that it won't be broken by your patches... Chris?
> 
> As mentioned above __blockdev_direct_IO only called vmtruncate for the
> DIO_LOCKING, which does not include btrfs.  And yes, there's quite a bit
> of things that need cleanup but didn't have it before due to the way
> the cleanup code was hidden, but that's not something I want to do
> in this transformation series.
  Ah, OK. I forgot about that DIO_LOCKING thing. Thanks for clarification.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[PATCH 2/6] get rid of nobh_write_begin_newtrunc

[Christoph Hellwig]

Move the call to vmtruncate to get rid of accessive blocks to the only
remaining caller and rename the non-truncating version to nobh_write_begin.

Get rid of the superflous file argument to it while we're at it.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/fs/buffer.c
===================================================================
--- linux-2.6.orig/fs/buffer.c	2010-05-30 22:28:16.571004834 +0200
+++ linux-2.6/fs/buffer.c	2010-05-30 22:30:49.341024320 +0200
@@ -2510,11 +2510,11 @@ static void attach_nobh_buffers(struct p
 }
 
 /*
- * Filesystems implementing the new truncate sequence should use the
- * _newtrunc postfix variant which won't incorrectly call vmtruncate.
+ * On entry, the page is fully not uptodate.
+ * On exit the page is fully uptodate in the areas outside (from,to)
  * The filesystem needs to handle block truncation upon failure.
  */
-int nobh_write_begin_newtrunc(struct file *file, struct address_space *mapping,
+int nobh_write_begin(struct address_space *mapping,
 			loff_t pos, unsigned len, unsigned flags,
 			struct page **pagep, void **fsdata,
 			get_block_t *get_block)
@@ -2547,7 +2547,7 @@ int nobh_write_begin_newtrunc(struct fil
 		unlock_page(page);
 		page_cache_release(page);
 		*pagep = NULL;
-		return block_write_begin_newtrunc(file, mapping, pos, len,
+		return block_write_begin_newtrunc(NULL, mapping, pos, len,
 					flags, pagep, fsdata, get_block);
 	}
 
@@ -2654,35 +2654,6 @@ out_release:
 
 	return ret;
 }
-EXPORT_SYMBOL(nobh_write_begin_newtrunc);
-
-/*
- * On entry, the page is fully not uptodate.
- * On exit the page is fully uptodate in the areas outside (from,to)
- */
-int nobh_write_begin(struct file *file, struct address_space *mapping,
-			loff_t pos, unsigned len, unsigned flags,
-			struct page **pagep, void **fsdata,
-			get_block_t *get_block)
-{
-	int ret;
-
-	ret = nobh_write_begin_newtrunc(file, mapping, pos, len, flags,
-					pagep, fsdata, get_block);
-
-	/*
-	 * prepare_write() may have instantiated a few blocks
-	 * outside i_size.  Trim these off again. Don't need
-	 * i_size_read because we hold i_mutex.
-	 */
-	if (unlikely(ret)) {
-		loff_t isize = mapping->host->i_size;
-		if (pos + len > isize)
-			vmtruncate(mapping->host, isize);
-	}
-
-	return ret;
-}
 EXPORT_SYMBOL(nobh_write_begin);
 
 int nobh_write_end(struct file *file, struct address_space *mapping,
Index: linux-2.6/fs/ext2/inode.c
===================================================================
--- linux-2.6.orig/fs/ext2/inode.c	2010-05-30 22:28:18.670254589 +0200
+++ linux-2.6/fs/ext2/inode.c	2010-05-30 22:30:49.342022505 +0200
@@ -806,13 +806,8 @@ ext2_nobh_write_begin(struct file *file,
 {
 	int ret;
 
-	/*
-	 * Dir-in-pagecache still uses ext2_write_begin. Would have to rework
-	 * directory handling code to pass around offsets rather than struct
-	 * pages in order to make this work easily.
-	 */
-	ret = nobh_write_begin_newtrunc(file, mapping, pos, len, flags, pagep,
-						fsdata, ext2_get_block);
+	ret = nobh_write_begin(mapping, pos, len, flags, pagep, fsdata,
+			       ext2_get_block);
 	if (ret < 0)
 		ext2_write_failed(mapping, pos + len);
 	return ret;
Index: linux-2.6/fs/jfs/inode.c
===================================================================
--- linux-2.6.orig/fs/jfs/inode.c	2010-05-30 22:28:18.696254170 +0200
+++ linux-2.6/fs/jfs/inode.c	2010-05-30 22:30:49.547300686 +0200
@@ -303,8 +303,17 @@ static int jfs_write_begin(struct file *
 				loff_t pos, unsigned len, unsigned flags,
 				struct page **pagep, void **fsdata)
 {
-	return nobh_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+	int ret;
+
+	ret = nobh_write_begin(mapping, pos, len, flags, pagep, fsdata,
 				jfs_get_block);
+	if (unlikely(ret)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+	}
+
+	return ret;
 }
 
 static sector_t jfs_bmap(struct address_space *mapping, sector_t block)
Index: linux-2.6/include/linux/buffer_head.h
===================================================================
--- linux-2.6.orig/include/linux/buffer_head.h	2010-05-30 22:28:16.601004276 +0200
+++ linux-2.6/include/linux/buffer_head.h	2010-05-30 22:30:49.611005673 +0200
@@ -231,11 +231,7 @@ void block_sync_page(struct page *);
 sector_t generic_block_bmap(struct address_space *, sector_t, get_block_t *);
 int block_truncate_page(struct address_space *, loff_t, get_block_t *);
 int file_fsync(struct file *, int);
-int nobh_write_begin_newtrunc(struct file *, struct address_space *,
-				loff_t, unsigned, unsigned,
-				struct page **, void **, get_block_t*);
-int nobh_write_begin(struct file *, struct address_space *,
-				loff_t, unsigned, unsigned,
+int nobh_write_begin(struct address_space *, loff_t, unsigned, unsigned,
 				struct page **, void **, get_block_t*);
 int nobh_write_end(struct file *, struct address_space *,
 				loff_t, unsigned, unsigned,

[PATCH 3/6] get rid of cont_write_begin_newtrunc

[Christoph Hellwig]

Move the call to vmtruncate to get rid of accessive blocks to the callers
in preparation of the new truncate sequence and rename the non-truncating
version to cont_write_begin.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/fs/adfs/inode.c
===================================================================
--- linux-2.6.orig/fs/adfs/inode.c	2010-05-30 22:28:16.408003507 +0200
+++ linux-2.6/fs/adfs/inode.c	2010-05-30 22:32:22.749253611 +0200
@@ -50,10 +50,19 @@ static int adfs_write_begin(struct file
 			loff_t pos, unsigned len, unsigned flags,
 			struct page **pagep, void **fsdata)
 {
+	int ret;
+
 	*pagep = NULL;
-	return cont_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+	ret = cont_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
 				adfs_get_block,
 				&ADFS_I(mapping->host)->mmu_private);
+	if (unlikely(ret)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+	}
+
+	return ret;
 }
 
 static sector_t _adfs_bmap(struct address_space *mapping, sector_t block)
Index: linux-2.6/fs/affs/file.c
===================================================================
--- linux-2.6.orig/fs/affs/file.c	2010-05-30 22:28:16.417004066 +0200
+++ linux-2.6/fs/affs/file.c	2010-05-30 22:32:22.749253611 +0200
@@ -406,10 +406,19 @@ static int affs_write_begin(struct file
 			loff_t pos, unsigned len, unsigned flags,
 			struct page **pagep, void **fsdata)
 {
+	int ret;
+
 	*pagep = NULL;
-	return cont_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+	ret = cont_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
 				affs_get_block,
 				&AFFS_I(mapping->host)->mmu_private);
+	if (unlikely(ret)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+	}
+
+	return ret;
 }
 
 static sector_t _affs_bmap(struct address_space *mapping, sector_t block)
Index: linux-2.6/fs/buffer.c
===================================================================
--- linux-2.6.orig/fs/buffer.c	2010-05-30 22:30:49.341024320 +0200
+++ linux-2.6/fs/buffer.c	2010-05-30 22:32:22.754253961 +0200
@@ -2351,7 +2351,7 @@ out:
  * For moronic filesystems that do not allow holes in file.
  * We may have to extend the file.
  */
-int cont_write_begin_newtrunc(struct file *file, struct address_space *mapping,
+int cont_write_begin(struct file *file, struct address_space *mapping,
 			loff_t pos, unsigned len, unsigned flags,
 			struct page **pagep, void **fsdata,
 			get_block_t *get_block, loff_t *bytes)
@@ -2377,25 +2377,6 @@ int cont_write_begin_newtrunc(struct fil
 out:
 	return err;
 }
-EXPORT_SYMBOL(cont_write_begin_newtrunc);
-
-int cont_write_begin(struct file *file, struct address_space *mapping,
-			loff_t pos, unsigned len, unsigned flags,
-			struct page **pagep, void **fsdata,
-			get_block_t *get_block, loff_t *bytes)
-{
-	int ret;
-
-	ret = cont_write_begin_newtrunc(file, mapping, pos, len, flags,
-					pagep, fsdata, get_block, bytes);
-	if (unlikely(ret)) {
-		loff_t isize = mapping->host->i_size;
-		if (pos + len > isize)
-			vmtruncate(mapping->host, isize);
-	}
-
-	return ret;
-}
 EXPORT_SYMBOL(cont_write_begin);
 
 int block_prepare_write(struct page *page, unsigned from, unsigned to,
Index: linux-2.6/fs/fat/inode.c
===================================================================
--- linux-2.6.orig/fs/fat/inode.c	2010-05-30 22:28:18.685253821 +0200
+++ linux-2.6/fs/fat/inode.c	2010-05-30 22:32:22.762255008 +0200
@@ -159,7 +159,7 @@ static int fat_write_begin(struct file *
 	int err;
 
 	*pagep = NULL;
-	err = cont_write_begin_newtrunc(file, mapping, pos, len, flags,
+	err = cont_write_begin(file, mapping, pos, len, flags,
 				pagep, fsdata, fat_get_block,
 				&MSDOS_I(mapping->host)->mmu_private);
 	if (err < 0)
Index: linux-2.6/fs/hfs/inode.c
===================================================================
--- linux-2.6.orig/fs/hfs/inode.c	2010-05-30 22:28:18.686254310 +0200
+++ linux-2.6/fs/hfs/inode.c	2010-05-30 22:32:22.763254240 +0200
@@ -39,10 +39,19 @@ static int hfs_write_begin(struct file *
 			loff_t pos, unsigned len, unsigned flags,
 			struct page **pagep, void **fsdata)
 {
+	int ret;
+
 	*pagep = NULL;
-	return cont_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+	ret = cont_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
 				hfs_get_block,
 				&HFS_I(mapping->host)->phys_size);
+	if (unlikely(ret)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+	}
+
+	return ret;
 }
 
 static sector_t hfs_bmap(struct address_space *mapping, sector_t block)
Index: linux-2.6/fs/hfsplus/inode.c
===================================================================
--- linux-2.6.orig/fs/hfsplus/inode.c	2010-05-30 22:28:18.691253611 +0200
+++ linux-2.6/fs/hfsplus/inode.c	2010-05-30 22:32:22.768253611 +0200
@@ -31,10 +31,19 @@ static int hfsplus_write_begin(struct fi
 			loff_t pos, unsigned len, unsigned flags,
 			struct page **pagep, void **fsdata)
 {
+	int ret;
+
 	*pagep = NULL;
-	return cont_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+	ret = cont_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
 				hfsplus_get_block,
 				&HFSPLUS_I(mapping->host).phys_size);
+	if (unlikely(ret)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+	}
+
+	return ret;
 }
 
 static sector_t hfsplus_bmap(struct address_space *mapping, sector_t block)
Index: linux-2.6/fs/hpfs/file.c
===================================================================
--- linux-2.6.orig/fs/hpfs/file.c	2010-05-30 22:28:16.461004695 +0200
+++ linux-2.6/fs/hpfs/file.c	2010-05-30 22:32:22.773253611 +0200
@@ -97,10 +97,19 @@ static int hpfs_write_begin(struct file
 			loff_t pos, unsigned len, unsigned flags,
 			struct page **pagep, void **fsdata)
 {
+	int ret;
+
 	*pagep = NULL;
-	return cont_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+	ret = cont_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
 				hpfs_get_block,
 				&hpfs_i(mapping->host)->mmu_private);
+	if (unlikely(ret)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+	}
+
+	return ret;
 }
 
 static sector_t _hpfs_bmap(struct address_space *mapping, sector_t block)
Index: linux-2.6/fs/qnx4/inode.c
===================================================================
--- linux-2.6.orig/fs/qnx4/inode.c	2010-05-30 22:28:16.468003507 +0200
+++ linux-2.6/fs/qnx4/inode.c	2010-05-30 22:32:22.778253891 +0200
@@ -320,10 +320,19 @@ static int qnx4_write_begin(struct file
 			struct page **pagep, void **fsdata)
 {
 	struct qnx4_inode_info *qnx4_inode = qnx4_i(mapping->host);
+	int ret;
+
 	*pagep = NULL;
-	return cont_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+	ret = cont_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
 				qnx4_get_block,
 				&qnx4_inode->mmu_private);
+	if (unlikely(ret)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+	}
+
+	return ret;
 }
 static sector_t qnx4_bmap(struct address_space *mapping, sector_t block)
 {
Index: linux-2.6/include/linux/buffer_head.h
===================================================================
--- linux-2.6.orig/include/linux/buffer_head.h	2010-05-30 22:30:49.611005673 +0200
+++ linux-2.6/include/linux/buffer_head.h	2010-05-30 22:32:22.783254030 +0200
@@ -217,9 +217,6 @@ int generic_write_end(struct file *, str
 				struct page *, void *);
 void page_zero_new_buffers(struct page *page, unsigned from, unsigned to);
 int block_prepare_write(struct page*, unsigned, unsigned, get_block_t*);
-int cont_write_begin_newtrunc(struct file *, struct address_space *, loff_t,
-			unsigned, unsigned, struct page **, void **,
-			get_block_t *, loff_t *);
 int cont_write_begin(struct file *, struct address_space *, loff_t,
 			unsigned, unsigned, struct page **, void **,
 			get_block_t *, loff_t *);

[PATCH 4/6] clean up write_begin usage for directories in pagecache

[Christoph Hellwig]

For filesystem that implement directories in pagecache we call
block_write_begin with an already allocated page for this code, while the
normal regular file write path uses the default block_write_begin behaviour.

Get rid of the __foofs_write_begin helper and opencode the normal write_begin
call in foofs_write_begin, while adding a new foofs_prepare_chunk helper for
the directory code.  The added benefit is that foofs_prepare_chunk has
a much saner calling convention.

Note that the interruptible flag passed into block_write_begin is always
ignored if we already pass in a page (see next patch for details), and
we never were doing truncations of exessive blocks for this case either so we
can switch directly to block_write_begin_newtrunc.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/fs/ext2/dir.c
===================================================================
--- linux-2.6.orig/fs/ext2/dir.c	2010-05-30 22:28:16.148254171 +0200
+++ linux-2.6/fs/ext2/dir.c	2010-05-30 22:33:06.790253891 +0200
@@ -448,6 +448,12 @@ ino_t ext2_inode_by_name(struct inode *d
 	return res;
 }
 
+static int ext2_prepare_chunk(struct page *page, loff_t pos, unsigned len)
+{
+	return block_write_begin_newtrunc(NULL, page->mapping, pos, len, 0,
+					  &page, NULL, ext2_get_block);
+}
+
 /* Releases the page */
 void ext2_set_link(struct inode *dir, struct ext2_dir_entry_2 *de,
 		   struct page *page, struct inode *inode, int update_times)
@@ -458,8 +464,7 @@ void ext2_set_link(struct inode *dir, st
 	int err;
 
 	lock_page(page);
-	err = __ext2_write_begin(NULL, page->mapping, pos, len,
-				AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
+	err = ext2_prepare_chunk(page, pos, len);
 	BUG_ON(err);
 	de->inode = cpu_to_le32(inode->i_ino);
 	ext2_set_de_type(de, inode);
@@ -542,8 +547,7 @@ int ext2_add_link (struct dentry *dentry
 got_it:
 	pos = page_offset(page) +
 		(char*)de - (char*)page_address(page);
-	err = __ext2_write_begin(NULL, page->mapping, pos, rec_len, 0,
-							&page, NULL);
+	err = ext2_prepare_chunk(page, pos, rec_len);
 	if (err)
 		goto out_unlock;
 	if (de->inode) {
@@ -576,8 +580,7 @@ out_unlock:
  */
 int ext2_delete_entry (struct ext2_dir_entry_2 * dir, struct page * page )
 {
-	struct address_space *mapping = page->mapping;
-	struct inode *inode = mapping->host;
+	struct inode *inode = page->mapping->host;
 	char *kaddr = page_address(page);
 	unsigned from = ((char*)dir - kaddr) & ~(ext2_chunk_size(inode)-1);
 	unsigned to = ((char *)dir - kaddr) +
@@ -601,8 +604,7 @@ int ext2_delete_entry (struct ext2_dir_e
 		from = (char*)pde - (char*)page_address(page);
 	pos = page_offset(page) + from;
 	lock_page(page);
-	err = __ext2_write_begin(NULL, page->mapping, pos, to - from, 0,
-							&page, NULL);
+	err = ext2_prepare_chunk(page, pos, to - from);
 	BUG_ON(err);
 	if (pde)
 		pde->rec_len = ext2_rec_len_to_disk(to - from);
@@ -621,8 +623,7 @@ out:
  */
 int ext2_make_empty(struct inode *inode, struct inode *parent)
 {
-	struct address_space *mapping = inode->i_mapping;
-	struct page *page = grab_cache_page(mapping, 0);
+	struct page *page = grab_cache_page(inode->i_mapping, 0);
 	unsigned chunk_size = ext2_chunk_size(inode);
 	struct ext2_dir_entry_2 * de;
 	int err;
@@ -631,8 +632,7 @@ int ext2_make_empty(struct inode *inode,
 	if (!page)
 		return -ENOMEM;
 
-	err = __ext2_write_begin(NULL, page->mapping, 0, chunk_size, 0,
-							&page, NULL);
+	err = ext2_prepare_chunk(page, 0, chunk_size);
 	if (err) {
 		unlock_page(page);
 		goto fail;
Index: linux-2.6/fs/ext2/ext2.h
===================================================================
--- linux-2.6.orig/fs/ext2/ext2.h	2010-05-30 22:28:16.154254799 +0200
+++ linux-2.6/fs/ext2/ext2.h	2010-05-30 22:33:06.791253542 +0200
@@ -127,9 +127,6 @@ extern void ext2_set_inode_flags(struct
 extern void ext2_get_inode_flags(struct ext2_inode_info *);
 extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 		       u64 start, u64 len);
-int __ext2_write_begin(struct file *file, struct address_space *mapping,
-		loff_t pos, unsigned len, unsigned flags,
-		struct page **pagep, void **fsdata);
 
 /* ioctl.c */
 extern long ext2_ioctl(struct file *, unsigned int, unsigned long);
Index: linux-2.6/fs/ext2/inode.c
===================================================================
--- linux-2.6.orig/fs/ext2/inode.c	2010-05-30 22:30:49.342022505 +0200
+++ linux-2.6/fs/ext2/inode.c	2010-05-30 22:33:06.798254589 +0200
@@ -765,14 +765,6 @@ ext2_readpages(struct file *file, struct
 	return mpage_readpages(mapping, pages, nr_pages, ext2_get_block);
 }
 
-int __ext2_write_begin(struct file *file, struct address_space *mapping,
-		loff_t pos, unsigned len, unsigned flags,
-		struct page **pagep, void **fsdata)
-{
-	return block_write_begin_newtrunc(file, mapping, pos, len, flags,
-					pagep, fsdata, ext2_get_block);
-}
-
 static int
 ext2_write_begin(struct file *file, struct address_space *mapping,
 		loff_t pos, unsigned len, unsigned flags,
@@ -781,7 +773,8 @@ ext2_write_begin(struct file *file, stru
 	int ret;
 
 	*pagep = NULL;
-	ret = __ext2_write_begin(file, mapping, pos, len, flags, pagep, fsdata);
+	ret = block_write_begin_newtrunc(file, mapping, pos, len, flags,
+					 pagep, fsdata, ext2_get_block);
 	if (ret < 0)
 		ext2_write_failed(mapping, pos + len);
 	return ret;
Index: linux-2.6/fs/minix/dir.c
===================================================================
--- linux-2.6.orig/fs/minix/dir.c	2010-05-30 22:28:16.174254031 +0200
+++ linux-2.6/fs/minix/dir.c	2010-05-30 22:33:06.804005532 +0200
@@ -275,8 +275,7 @@ int minix_add_link(struct dentry *dentry
 
 got_it:
 	pos = page_offset(page) + p - (char *)page_address(page);
-	err = __minix_write_begin(NULL, page->mapping, pos, sbi->s_dirsize,
-					AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
+	err = minix_prepare_chunk(page, pos, sbi->s_dirsize);
 	if (err)
 		goto out_unlock;
 	memcpy (namx, name, namelen);
@@ -301,8 +300,7 @@ out_unlock:
 
 int minix_delete_entry(struct minix_dir_entry *de, struct page *page)
 {
-	struct address_space *mapping = page->mapping;
-	struct inode *inode = (struct inode*)mapping->host;
+	struct inode *inode = page->mapping->host;
 	char *kaddr = page_address(page);
 	loff_t pos = page_offset(page) + (char*)de - kaddr;
 	struct minix_sb_info *sbi = minix_sb(inode->i_sb);
@@ -310,8 +308,7 @@ int minix_delete_entry(struct minix_dir_
 	int err;
 
 	lock_page(page);
-	err = __minix_write_begin(NULL, mapping, pos, len,
-					AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
+	err = minix_prepare_chunk(page, pos, len);
 	if (err == 0) {
 		if (sbi->s_version == MINIX_V3)
 			((minix3_dirent *) de)->inode = 0;
@@ -329,16 +326,14 @@ int minix_delete_entry(struct minix_dir_
 
 int minix_make_empty(struct inode *inode, struct inode *dir)
 {
-	struct address_space *mapping = inode->i_mapping;
-	struct page *page = grab_cache_page(mapping, 0);
+	struct page *page = grab_cache_page(inode->i_mapping, 0);
 	struct minix_sb_info *sbi = minix_sb(inode->i_sb);
 	char *kaddr;
 	int err;
 
 	if (!page)
 		return -ENOMEM;
-	err = __minix_write_begin(NULL, mapping, 0, 2 * sbi->s_dirsize,
-					AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
+	err = minix_prepare_chunk(page, 0, 2 * sbi->s_dirsize);
 	if (err) {
 		unlock_page(page);
 		goto fail;
@@ -429,8 +424,7 @@ not_empty:
 void minix_set_link(struct minix_dir_entry *de, struct page *page,
 	struct inode *inode)
 {
-	struct address_space *mapping = page->mapping;
-	struct inode *dir = mapping->host;
+	struct inode *dir = page->mapping->host;
 	struct minix_sb_info *sbi = minix_sb(dir->i_sb);
 	loff_t pos = page_offset(page) +
 			(char *)de-(char*)page_address(page);
@@ -438,8 +432,7 @@ void minix_set_link(struct minix_dir_ent
 
 	lock_page(page);
 
-	err = __minix_write_begin(NULL, mapping, pos, sbi->s_dirsize,
-					AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
+	err = minix_prepare_chunk(page, pos, sbi->s_dirsize);
 	if (err == 0) {
 		if (sbi->s_version == MINIX_V3)
 			((minix3_dirent *) de)->inode = inode->i_ino;
Index: linux-2.6/fs/minix/inode.c
===================================================================
--- linux-2.6.orig/fs/minix/inode.c	2010-05-30 22:28:16.181254031 +0200
+++ linux-2.6/fs/minix/inode.c	2010-05-30 22:33:06.810255986 +0200
@@ -357,12 +357,10 @@ static int minix_readpage(struct file *f
 	return block_read_full_page(page,minix_get_block);
 }
 
-int __minix_write_begin(struct file *file, struct address_space *mapping,
-			loff_t pos, unsigned len, unsigned flags,
-			struct page **pagep, void **fsdata)
+int minix_prepare_chunk(struct page *page, loff_t pos, unsigned len)
 {
-	return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
-				minix_get_block);
+	return block_write_begin_newtrunc(NULL, page->mapping, pos, len, 0,
+					  &page, NULL, minix_get_block);
 }
 
 static int minix_write_begin(struct file *file, struct address_space *mapping,
@@ -370,7 +368,8 @@ static int minix_write_begin(struct file
 			struct page **pagep, void **fsdata)
 {
 	*pagep = NULL;
-	return __minix_write_begin(file, mapping, pos, len, flags, pagep, fsdata);
+	return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+				minix_get_block);
 }
 
 static sector_t minix_bmap(struct address_space *mapping, sector_t block)
Index: linux-2.6/fs/minix/minix.h
===================================================================
--- linux-2.6.orig/fs/minix/minix.h	2010-05-30 22:28:16.192254101 +0200
+++ linux-2.6/fs/minix/minix.h	2010-05-30 22:33:06.818006789 +0200
@@ -53,9 +53,7 @@ extern int minix_new_block(struct inode
 extern void minix_free_block(struct inode *inode, unsigned long block);
 extern unsigned long minix_count_free_blocks(struct minix_sb_info *sbi);
 extern int minix_getattr(struct vfsmount *, struct dentry *, struct kstat *);
-extern int __minix_write_begin(struct file *file, struct address_space *mapping,
-			loff_t pos, unsigned len, unsigned flags,
-			struct page **pagep, void **fsdata);
+extern int minix_prepare_chunk(struct page *page, loff_t pos, unsigned len);
 
 extern void V1_minix_truncate(struct inode *);
 extern void V2_minix_truncate(struct inode *);
Index: linux-2.6/fs/sysv/dir.c
===================================================================
--- linux-2.6.orig/fs/sysv/dir.c	2010-05-30 22:28:16.201254171 +0200
+++ linux-2.6/fs/sysv/dir.c	2010-05-30 22:33:06.825007697 +0200
@@ -218,8 +218,7 @@ got_it:
 	pos = page_offset(page) +
 			(char*)de - (char*)page_address(page);
 	lock_page(page);
-	err = __sysv_write_begin(NULL, page->mapping, pos, SYSV_DIRSIZE,
-				AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
+	err = sysv_prepare_chunk(page, pos, SYSV_DIRSIZE);
 	if (err)
 		goto out_unlock;
 	memcpy (de->name, name, namelen);
@@ -239,15 +238,13 @@ out_unlock:
 
 int sysv_delete_entry(struct sysv_dir_entry *de, struct page *page)
 {
-	struct address_space *mapping = page->mapping;
-	struct inode *inode = (struct inode*)mapping->host;
+	struct inode *inode = page->mapping->host;
 	char *kaddr = (char*)page_address(page);
 	loff_t pos = page_offset(page) + (char *)de - kaddr;
 	int err;
 
 	lock_page(page);
-	err = __sysv_write_begin(NULL, mapping, pos, SYSV_DIRSIZE,
-				AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
+	err = sysv_prepare_chunk(page, pos, SYSV_DIRSIZE);
 	BUG_ON(err);
 	de->inode = 0;
 	err = dir_commit_chunk(page, pos, SYSV_DIRSIZE);
@@ -259,16 +256,14 @@ int sysv_delete_entry(struct sysv_dir_en
 
 int sysv_make_empty(struct inode *inode, struct inode *dir)
 {
-	struct address_space *mapping = inode->i_mapping;
-	struct page *page = grab_cache_page(mapping, 0);
+	struct page *page = grab_cache_page(inode->i_mapping, 0);
 	struct sysv_dir_entry * de;
 	char *base;
 	int err;
 
 	if (!page)
 		return -ENOMEM;
-	err = __sysv_write_begin(NULL, mapping, 0, 2 * SYSV_DIRSIZE,
-				AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
+	err = sysv_prepare_chunk(page, 0, 2 * SYSV_DIRSIZE);
 	if (err) {
 		unlock_page(page);
 		goto fail;
@@ -341,15 +336,13 @@ not_empty:
 void sysv_set_link(struct sysv_dir_entry *de, struct page *page,
 	struct inode *inode)
 {
-	struct address_space *mapping = page->mapping;
-	struct inode *dir = mapping->host;
+	struct inode *dir = page->mapping->host;
 	loff_t pos = page_offset(page) +
 			(char *)de-(char*)page_address(page);
 	int err;
 
 	lock_page(page);
-	err = __sysv_write_begin(NULL, mapping, pos, SYSV_DIRSIZE,
-				AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
+	err = sysv_prepare_chunk(page, pos, SYSV_DIRSIZE);
 	BUG_ON(err);
 	de->inode = cpu_to_fs16(SYSV_SB(inode->i_sb), inode->i_ino);
 	err = dir_commit_chunk(page, pos, SYSV_DIRSIZE);
Index: linux-2.6/fs/sysv/itree.c
===================================================================
--- linux-2.6.orig/fs/sysv/itree.c	2010-05-30 22:28:16.207255218 +0200
+++ linux-2.6/fs/sysv/itree.c	2010-05-30 22:33:06.832007627 +0200
@@ -459,12 +459,10 @@ static int sysv_readpage(struct file *fi
 	return block_read_full_page(page,get_block);
 }
 
-int __sysv_write_begin(struct file *file, struct address_space *mapping,
-			loff_t pos, unsigned len, unsigned flags,
-			struct page **pagep, void **fsdata)
+int sysv_prepare_chunk(struct page *page, loff_t pos, unsigned len)
 {
-	return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
-				get_block);
+	return block_write_begin_newtrunc(NULL, page->mapping, pos, len, 0,
+					  &page, NULL, get_block);
 }
 
 static int sysv_write_begin(struct file *file, struct address_space *mapping,
@@ -472,7 +470,8 @@ static int sysv_write_begin(struct file
 			struct page **pagep, void **fsdata)
 {
 	*pagep = NULL;
-	return __sysv_write_begin(file, mapping, pos, len, flags, pagep, fsdata);
+	return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+				get_block);
 }
 
 static sector_t sysv_bmap(struct address_space *mapping, sector_t block)
Index: linux-2.6/fs/sysv/sysv.h
===================================================================
--- linux-2.6.orig/fs/sysv/sysv.h	2010-05-30 22:28:16.215254730 +0200
+++ linux-2.6/fs/sysv/sysv.h	2010-05-30 22:33:06.839255567 +0200
@@ -136,9 +136,7 @@ extern unsigned long sysv_count_free_blo
 
 /* itree.c */
 extern void sysv_truncate(struct inode *);
-extern int __sysv_write_begin(struct file *file, struct address_space *mapping,
-			loff_t pos, unsigned len, unsigned flags,
-			struct page **pagep, void **fsdata);
+extern int sysv_prepare_chunk(struct page *page, loff_t pos, unsigned len);
 
 /* inode.c */
 extern struct inode *sysv_iget(struct super_block *, unsigned int);
Index: linux-2.6/fs/ufs/dir.c
===================================================================
--- linux-2.6.orig/fs/ufs/dir.c	2010-05-30 22:28:16.226254031 +0200
+++ linux-2.6/fs/ufs/dir.c	2010-05-30 22:33:06.850005254 +0200
@@ -95,8 +95,7 @@ void ufs_set_link(struct inode *dir, str
 	int err;
 
 	lock_page(page);
-	err = __ufs_write_begin(NULL, page->mapping, pos, len,
-				AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
+	err = ufs_prepare_chunk(page, pos, len);
 	BUG_ON(err);
 
 	de->d_ino = cpu_to_fs32(dir->i_sb, inode->i_ino);
@@ -381,8 +380,7 @@ int ufs_add_link(struct dentry *dentry,
 got_it:
 	pos = page_offset(page) +
 			(char*)de - (char*)page_address(page);
-	err = __ufs_write_begin(NULL, page->mapping, pos, rec_len,
-				AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
+	err = ufs_prepare_chunk(page, pos, rec_len);
 	if (err)
 		goto out_unlock;
 	if (de->d_ino) {
@@ -518,7 +516,6 @@ int ufs_delete_entry(struct inode *inode
 		     struct page * page)
 {
 	struct super_block *sb = inode->i_sb;
-	struct address_space *mapping = page->mapping;
 	char *kaddr = page_address(page);
 	unsigned from = ((char*)dir - kaddr) & ~(UFS_SB(sb)->s_uspi->s_dirblksize - 1);
 	unsigned to = ((char*)dir - kaddr) + fs16_to_cpu(sb, dir->d_reclen);
@@ -549,8 +546,7 @@ int ufs_delete_entry(struct inode *inode
 
 	pos = page_offset(page) + from;
 	lock_page(page);
-	err = __ufs_write_begin(NULL, mapping, pos, to - from,
-				AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
+	err = ufs_prepare_chunk(page, pos, to - from);
 	BUG_ON(err);
 	if (pde)
 		pde->d_reclen = cpu_to_fs16(sb, to - from);
@@ -577,8 +573,7 @@ int ufs_make_empty(struct inode * inode,
 	if (!page)
 		return -ENOMEM;
 
-	err = __ufs_write_begin(NULL, mapping, 0, chunk_size,
-				AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
+	err = ufs_prepare_chunk(page, 0, chunk_size);
 	if (err) {
 		unlock_page(page);
 		goto fail;
Index: linux-2.6/fs/ufs/inode.c
===================================================================
--- linux-2.6.orig/fs/ufs/inode.c	2010-05-30 22:28:16.234255149 +0200
+++ linux-2.6/fs/ufs/inode.c	2010-05-30 22:33:06.861257313 +0200
@@ -559,12 +559,10 @@ static int ufs_readpage(struct file *fil
 	return block_read_full_page(page,ufs_getfrag_block);
 }
 
-int __ufs_write_begin(struct file *file, struct address_space *mapping,
-			loff_t pos, unsigned len, unsigned flags,
-			struct page **pagep, void **fsdata)
+int ufs_prepare_chunk(struct page *page, loff_t pos, unsigned len)
 {
-	return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
-				ufs_getfrag_block);
+	return block_write_begin_newtrunc(NULL, page->mapping, pos, len, 0,
+					  &page, NULL, ufs_getfrag_block);
 }
 
 static int ufs_write_begin(struct file *file, struct address_space *mapping,
@@ -572,7 +570,8 @@ static int ufs_write_begin(struct file *
 			struct page **pagep, void **fsdata)
 {
 	*pagep = NULL;
-	return __ufs_write_begin(file, mapping, pos, len, flags, pagep, fsdata);
+	return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+				ufs_getfrag_block);
 }
 
 static sector_t ufs_bmap(struct address_space *mapping, sector_t block)
Index: linux-2.6/fs/ufs/util.h
===================================================================
--- linux-2.6.orig/fs/ufs/util.h	2010-05-30 22:28:16.241254590 +0200
+++ linux-2.6/fs/ufs/util.h	2010-05-30 22:33:06.869007627 +0200
@@ -257,9 +257,7 @@ ufs_set_inode_gid(struct super_block *sb
 
 extern dev_t ufs_get_inode_dev(struct super_block *, struct ufs_inode_info *);
 extern void ufs_set_inode_dev(struct super_block *, struct ufs_inode_info *, dev_t);
-extern int __ufs_write_begin(struct file *file, struct address_space *mapping,
-		loff_t pos, unsigned len, unsigned flags,
-		struct page **pagep, void **fsdata);
+extern int ufs_prepare_chunk(struct page *page, loff_t pos, unsigned len);
 
 /*
  * These functions manipulate ufs buffers
Index: linux-2.6/fs/nilfs2/dir.c
===================================================================
--- linux-2.6.orig/fs/nilfs2/dir.c	2010-05-30 22:28:16.255254101 +0200
+++ linux-2.6/fs/nilfs2/dir.c	2010-05-30 22:33:06.880005392 +0200
@@ -80,23 +80,11 @@ static unsigned nilfs_last_byte(struct i
 	return last_byte;
 }
 
-static int nilfs_prepare_chunk_uninterruptible(struct page *page,
-					       struct address_space *mapping,
-					       unsigned from, unsigned to)
+static int nilfs_prepare_chunk(struct page *page, unsigned from, unsigned to)
 {
 	loff_t pos = page_offset(page) + from;
-	return block_write_begin(NULL, mapping, pos, to - from,
-				 AOP_FLAG_UNINTERRUPTIBLE, &page,
-				 NULL, nilfs_get_block);
-}
-
-static int nilfs_prepare_chunk(struct page *page,
-			       struct address_space *mapping,
-			       unsigned from, unsigned to)
-{
-	loff_t pos = page_offset(page) + from;
-	return block_write_begin(NULL, mapping, pos, to - from, 0, &page,
-				 NULL, nilfs_get_block);
+	return block_write_begin_newtrunc(NULL, page->mapping, pos, to - from,
+					  0, &page, NULL, nilfs_get_block);
 }
 
 static void nilfs_commit_chunk(struct page *page,
@@ -449,7 +437,7 @@ void nilfs_set_link(struct inode *dir, s
 	int err;
 
 	lock_page(page);
-	err = nilfs_prepare_chunk_uninterruptible(page, mapping, from, to);
+	err = nilfs_prepare_chunk(page, from, to);
 	BUG_ON(err);
 	de->inode = cpu_to_le64(inode->i_ino);
 	nilfs_set_de_type(de, inode);
@@ -530,7 +518,7 @@ int nilfs_add_link(struct dentry *dentry
 got_it:
 	from = (char *)de - (char *)page_address(page);
 	to = from + rec_len;
-	err = nilfs_prepare_chunk(page, page->mapping, from, to);
+	err = nilfs_prepare_chunk(page, from, to);
 	if (err)
 		goto out_unlock;
 	if (de->inode) {
@@ -587,7 +575,7 @@ int nilfs_delete_entry(struct nilfs_dir_
 	if (pde)
 		from = (char *)pde - (char *)page_address(page);
 	lock_page(page);
-	err = nilfs_prepare_chunk(page, mapping, from, to);
+	err = nilfs_prepare_chunk(page, from, to);
 	BUG_ON(err);
 	if (pde)
 		pde->rec_len = cpu_to_le16(to - from);
@@ -615,7 +603,7 @@ int nilfs_make_empty(struct inode *inode
 	if (!page)
 		return -ENOMEM;
 
-	err = nilfs_prepare_chunk(page, mapping, 0, chunk_size);
+	err = nilfs_prepare_chunk(page, 0, chunk_size);
 	if (unlikely(err)) {
 		unlock_page(page);
 		goto fail;

[PATCH 5/6] introduce __block_write_begin

[Christoph Hellwig]

Split up the block_write_begin implementation - __block_write_begin is a new
trivial wrapper for block_prepare_write that always takes an already
allocated page and can be either called from block_write_begin or filesystem
code that already has a page allocated.  Remove the handling of already
allocated pages from block_write_begin after switching all callers that
do it to __block_write_begin.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6/fs/buffer.c
===================================================================
--- linux-2.6.orig/fs/buffer.c	2010-05-30 22:32:22.754253961 +0200
+++ linux-2.6/fs/buffer.c	2010-05-30 22:40:16.496254519 +0200
@@ -1833,9 +1833,10 @@ void page_zero_new_buffers(struct page *
 }
 EXPORT_SYMBOL(page_zero_new_buffers);
 
-static int __block_prepare_write(struct inode *inode, struct page *page,
-		unsigned from, unsigned to, get_block_t *get_block)
+int block_prepare_write(struct page *page, unsigned from, unsigned to,
+		get_block_t *get_block)
 {
+	struct inode *inode = page->mapping->host;
 	unsigned block_start, block_end;
 	sector_t block;
 	int err = 0;
@@ -1908,10 +1909,13 @@ static int __block_prepare_write(struct
 		if (!buffer_uptodate(*wait_bh))
 			err = -EIO;
 	}
-	if (unlikely(err))
+	if (unlikely(err)) {
 		page_zero_new_buffers(page, from, to);
+		ClearPageUptodate(page);
+	}
 	return err;
 }
+EXPORT_SYMBOL(block_prepare_write);
 
 static int __block_commit_write(struct inode *inode, struct page *page,
 		unsigned from, unsigned to)
@@ -1948,6 +1952,14 @@ static int __block_commit_write(struct i
 	return 0;
 }
 
+int __block_write_begin(struct page *page, loff_t pos, unsigned len,
+		get_block_t *get_block)
+{
+	unsigned start = pos & (PAGE_CACHE_SIZE - 1);
+
+	return block_prepare_write(page, start, start + len, get_block);
+}
+
 /*
  * Filesystems implementing the new truncate sequence should use the
  * _newtrunc postfix variant which won't incorrectly call vmtruncate.
@@ -1958,41 +1970,22 @@ int block_write_begin_newtrunc(struct fi
 			struct page **pagep, void **fsdata,
 			get_block_t *get_block)
 {
-	struct inode *inode = mapping->host;
-	int status = 0;
+	pgoff_t index = pos >> PAGE_CACHE_SHIFT;
 	struct page *page;
-	pgoff_t index;
-	unsigned start, end;
-	int ownpage = 0;
+	int status;
 
-	index = pos >> PAGE_CACHE_SHIFT;
-	start = pos & (PAGE_CACHE_SIZE - 1);
-	end = start + len;
-
-	page = *pagep;
-	if (page == NULL) {
-		ownpage = 1;
-		page = grab_cache_page_write_begin(mapping, index, flags);
-		if (!page) {
-			status = -ENOMEM;
-			goto out;
-		}
-		*pagep = page;
-	} else
-		BUG_ON(!PageLocked(page));
+	page = grab_cache_page_write_begin(mapping, index, flags);
+	if (!page)
+		return -ENOMEM;
 
-	status = __block_prepare_write(inode, page, start, end, get_block);
+	status = __block_write_begin(page, pos, len, get_block);
 	if (unlikely(status)) {
-		ClearPageUptodate(page);
-
-		if (ownpage) {
-			unlock_page(page);
-			page_cache_release(page);
-			*pagep = NULL;
-		}
+		unlock_page(page);
+		page_cache_release(page);
+		page = NULL;
 	}
 
-out:
+	*pagep = page;
 	return status;
 }
 EXPORT_SYMBOL(block_write_begin_newtrunc);
@@ -2379,17 +2372,6 @@ out:
 }
 EXPORT_SYMBOL(cont_write_begin);
 
-int block_prepare_write(struct page *page, unsigned from, unsigned to,
-			get_block_t *get_block)
-{
-	struct inode *inode = page->mapping->host;
-	int err = __block_prepare_write(inode, page, from, to, get_block);
-	if (err)
-		ClearPageUptodate(page);
-	return err;
-}
-EXPORT_SYMBOL(block_prepare_write);
-
 int block_commit_write(struct page *page, unsigned from, unsigned to)
 {
 	struct inode *inode = page->mapping->host;
Index: linux-2.6/fs/ext2/dir.c
===================================================================
--- linux-2.6.orig/fs/ext2/dir.c	2010-05-30 22:33:06.790253891 +0200
+++ linux-2.6/fs/ext2/dir.c	2010-05-30 22:36:53.972003647 +0200
@@ -450,8 +450,7 @@ ino_t ext2_inode_by_name(struct inode *d
 
 static int ext2_prepare_chunk(struct page *page, loff_t pos, unsigned len)
 {
-	return block_write_begin_newtrunc(NULL, page->mapping, pos, len, 0,
-					  &page, NULL, ext2_get_block);
+	return __block_write_begin(page, pos, len, ext2_get_block);
 }
 
 /* Releases the page */
Index: linux-2.6/fs/minix/inode.c
===================================================================
--- linux-2.6.orig/fs/minix/inode.c	2010-05-30 22:33:06.810255986 +0200
+++ linux-2.6/fs/minix/inode.c	2010-05-30 22:40:02.980254241 +0200
@@ -359,8 +359,7 @@ static int minix_readpage(struct file *f
 
 int minix_prepare_chunk(struct page *page, loff_t pos, unsigned len)
 {
-	return block_write_begin_newtrunc(NULL, page->mapping, pos, len, 0,
-					  &page, NULL, minix_get_block);
+	return __block_write_begin(page, pos, len, minix_get_block);
 }
 
 static int minix_write_begin(struct file *file, struct address_space *mapping,
Index: linux-2.6/fs/nilfs2/dir.c
===================================================================
--- linux-2.6.orig/fs/nilfs2/dir.c	2010-05-30 22:33:06.880005392 +0200
+++ linux-2.6/fs/nilfs2/dir.c	2010-05-30 22:36:53.982004974 +0200
@@ -83,8 +83,7 @@ static unsigned nilfs_last_byte(struct i
 static int nilfs_prepare_chunk(struct page *page, unsigned from, unsigned to)
 {
 	loff_t pos = page_offset(page) + from;
-	return block_write_begin_newtrunc(NULL, page->mapping, pos, to - from,
-					  0, &page, NULL, nilfs_get_block);
+	return __block_write_begin(page, pos, to - from, nilfs_get_block);
 }
 
 static void nilfs_commit_chunk(struct page *page,
Index: linux-2.6/fs/sysv/itree.c
===================================================================
--- linux-2.6.orig/fs/sysv/itree.c	2010-05-30 22:33:06.832007627 +0200
+++ linux-2.6/fs/sysv/itree.c	2010-05-30 22:40:02.987254172 +0200
@@ -461,8 +461,7 @@ static int sysv_readpage(struct file *fi
 
 int sysv_prepare_chunk(struct page *page, loff_t pos, unsigned len)
 {
-	return block_write_begin_newtrunc(NULL, page->mapping, pos, len, 0,
-					  &page, NULL, get_block);
+	return __block_write_begin(page, pos, len, get_block);
 }
 
 static int sysv_write_begin(struct file *file, struct address_space *mapping,
Index: linux-2.6/fs/ufs/inode.c
===================================================================
--- linux-2.6.orig/fs/ufs/inode.c	2010-05-30 22:33:06.861257313 +0200
+++ linux-2.6/fs/ufs/inode.c	2010-05-30 22:40:02.996254591 +0200
@@ -561,8 +561,7 @@ static int ufs_readpage(struct file *fil
 
 int ufs_prepare_chunk(struct page *page, loff_t pos, unsigned len)
 {
-	return block_write_begin_newtrunc(NULL, page->mapping, pos, len, 0,
-					  &page, NULL, ufs_getfrag_block);
+	return __block_write_begin(page, pos, len, ufs_getfrag_block);
 }
 
 static int ufs_write_begin(struct file *file, struct address_space *mapping,
Index: linux-2.6/include/linux/buffer_head.h
===================================================================
--- linux-2.6.orig/include/linux/buffer_head.h	2010-05-30 22:32:22.783254030 +0200
+++ linux-2.6/include/linux/buffer_head.h	2010-05-30 22:40:03.005253892 +0200
@@ -209,6 +209,8 @@ int block_write_begin_newtrunc(struct fi
 int block_write_begin(struct file *, struct address_space *,
 				loff_t, unsigned, unsigned,
 				struct page **, void **, get_block_t*);
+int __block_write_begin(struct page *page, loff_t pos, unsigned len,
+		get_block_t *get_block);
 int block_write_end(struct file *, struct address_space *,
 				loff_t, unsigned, unsigned,
 				struct page *, void *);
Index: linux-2.6/fs/ext3/inode.c
===================================================================
--- linux-2.6.orig/fs/ext3/inode.c	2010-05-30 22:28:18.676253821 +0200
+++ linux-2.6/fs/ext3/inode.c	2010-05-30 22:36:54.000003996 +0200
@@ -1196,8 +1196,7 @@ retry:
 		ret = PTR_ERR(handle);
 		goto out;
 	}
-	ret = block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
-							ext3_get_block);
+	ret = __block_write_begin(page, pos, len, ext3_get_block);
 	if (ret)
 		goto write_begin_failed;
 
Index: linux-2.6/fs/ext4/inode.c
===================================================================
--- linux-2.6.orig/fs/ext4/inode.c	2010-05-30 22:28:18.646254240 +0200
+++ linux-2.6/fs/ext4/inode.c	2010-05-30 22:36:54.006004066 +0200
@@ -1578,11 +1578,9 @@ retry:
 	*pagep = page;
 
 	if (ext4_should_dioread_nolock(inode))
-		ret = block_write_begin(file, mapping, pos, len, flags, pagep,
-				fsdata, ext4_get_block_write);
+		ret = __block_write_begin(page, pos, len, ext4_get_block_write);
 	else
-		ret = block_write_begin(file, mapping, pos, len, flags, pagep,
-				fsdata, ext4_get_block);
+		ret = __block_write_begin(page, pos, len, ext4_get_block);
 
 	if (!ret && ext4_should_journal_data(inode)) {
 		ret = walk_page_buffers(handle, page_buffers(page),
@@ -1593,7 +1591,7 @@ retry:
 		unlock_page(page);
 		page_cache_release(page);
 		/*
-		 * block_write_begin may have instantiated a few blocks
+		 * __block_write_begin may have instantiated a few blocks
 		 * outside i_size.  Trim these off again. Don't need
 		 * i_size_read because we hold i_mutex.
 		 *
@@ -3185,8 +3183,7 @@ retry:
 	}
 	*pagep = page;
 
-	ret = block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
-				ext4_da_get_block_prep);
+	ret = __block_write_begin(page, pos, len, ext4_da_get_block_prep);
 	if (ret < 0) {
 		unlock_page(page);
 		ext4_journal_stop(handle);
Index: linux-2.6/fs/reiserfs/inode.c
===================================================================
--- linux-2.6.orig/fs/reiserfs/inode.c	2010-05-30 22:28:18.703254310 +0200
+++ linux-2.6/fs/reiserfs/inode.c	2010-05-30 22:36:54.013004695 +0200
@@ -2587,8 +2587,7 @@ static int reiserfs_write_begin(struct f
 		old_ref = th->t_refcount;
 		th->t_refcount++;
 	}
-	ret = block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
-				reiserfs_get_block);
+	ret = __block_write_begin(page, pos, len, reiserfs_get_block);
 	if (ret && reiserfs_transaction_running(inode->i_sb)) {
 		struct reiserfs_transaction_handle *th = current->journal_info;
 		/* this gets a little ugly.  If reiserfs_get_block returned an

Re: [PATCH 5/6] introduce __block_write_begin

[Christoph Hellwig]

On Sun, May 30, 2010 at 10:50:18PM +0200, Christoph Hellwig wrote:
> Split up the block_write_begin implementation - __block_write_begin is a new
> trivial wrapper for block_prepare_write that always takes an already
> allocated page and can be either called from block_write_begin or filesystem
> code that already has a page allocated.  Remove the handling of already
> allocated pages from block_write_begin after switching all callers that
> do it to __block_write_begin.

__block_write_begin is used by modules so it needs to be exported.
The one liner below needs to be folded in:

Index: linux-2.6/fs/buffer.c
===================================================================
--- linux-2.6.orig/fs/buffer.c	2010-05-31 09:43:07.232004136 +0200
+++ linux-2.6/fs/buffer.c	2010-05-31 10:07:38.612253821 +0200
@@ -1959,6 +1959,7 @@ int __block_write_begin(struct page *pag
 
 	return block_prepare_write(page, start, start + len, get_block);
 }
+EXPORT_SYMBOL(__block_write_begin);
 
 /*
  * block_write_begin takes care of the basic task of block allocation and

Re: [PATCH 5/6] introduce __block_write_begin

[Jan Kara]

On Sun 30-05-10 22:50:18, Christoph Hellwig wrote:
> Split up the block_write_begin implementation - __block_write_begin is a new
> trivial wrapper for block_prepare_write that always takes an already
> allocated page and can be either called from block_write_begin or filesystem
> code that already has a page allocated.  Remove the handling of already
> allocated pages from block_write_begin after switching all callers that
> do it to __block_write_begin.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: linux-2.6/fs/buffer.c
> ===================================================================
> --- linux-2.6.orig/fs/buffer.c	2010-05-30 22:32:22.754253961 +0200
> +++ linux-2.6/fs/buffer.c	2010-05-30 22:40:16.496254519 +0200
> @@ -1833,9 +1833,10 @@ void page_zero_new_buffers(struct page *
>  }
>  EXPORT_SYMBOL(page_zero_new_buffers);
>  
> -static int __block_prepare_write(struct inode *inode, struct page *page,
> -		unsigned from, unsigned to, get_block_t *get_block)
> +int block_prepare_write(struct page *page, unsigned from, unsigned to,
> +		get_block_t *get_block)
>  {
> +	struct inode *inode = page->mapping->host;
  This new variable doesn't seem to be used...

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 5/6] introduce __block_write_begin

[Christoph Hellwig]

On Wed, Jun 02, 2010 at 01:39:50AM +0200, Jan Kara wrote:
> > +	struct inode *inode = page->mapping->host;
>   This new variable doesn't seem to be used...

It's used to look at ->i_blkbits and passed to the get_block calls.

Re: [PATCH 5/6] introduce __block_write_begin

[Jan Kara]

On Wed 02-06-10 09:25:45, Christoph Hellwig wrote:
> On Wed, Jun 02, 2010 at 01:39:50AM +0200, Jan Kara wrote:
> > > +	struct inode *inode = page->mapping->host;
> >   This new variable doesn't seem to be used...
> 
> It's used to look at ->i_blkbits and passed to the get_block calls.
  Doh, yesterday I was obviously too tired to write anything useful. I'm
sorry.

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[PATCH 6/6] get rid of block_write_begin_newtrunc

[Christoph Hellwig]

Move the call to vmtruncate to get rid of accessive blocks to the callers
in preparation of the new truncate sequence and rename the non-truncating
version to block_write_begin.

While we're at it also remove several unused arguments to block_write_begin.

Signed-off-by: Christoph Hellwig <hch@lst.de>


Index: linux-2.6/fs/buffer.c
===================================================================
--- linux-2.6.orig/fs/buffer.c	2010-05-30 22:40:16.496254519 +0200
+++ linux-2.6/fs/buffer.c	2010-05-30 22:40:19.844004484 +0200
@@ -1961,14 +1961,13 @@ int __block_write_begin(struct page *pag
 }
 
 /*
- * Filesystems implementing the new truncate sequence should use the
- * _newtrunc postfix variant which won't incorrectly call vmtruncate.
+ * block_write_begin takes care of the basic task of block allocation and
+ * bringing partial write blocks uptodate first.
+ *
  * The filesystem needs to handle block truncation upon failure.
  */
-int block_write_begin_newtrunc(struct file *file, struct address_space *mapping,
-			loff_t pos, unsigned len, unsigned flags,
-			struct page **pagep, void **fsdata,
-			get_block_t *get_block)
+int block_write_begin(struct address_space *mapping, loff_t pos, unsigned len,
+		unsigned flags, struct page **pagep, get_block_t *get_block)
 {
 	pgoff_t index = pos >> PAGE_CACHE_SHIFT;
 	struct page *page;
@@ -1988,44 +1987,6 @@ int block_write_begin_newtrunc(struct fi
 	*pagep = page;
 	return status;
 }
-EXPORT_SYMBOL(block_write_begin_newtrunc);
-
-/*
- * block_write_begin takes care of the basic task of block allocation and
- * bringing partial write blocks uptodate first.
- *
- * If *pagep is not NULL, then block_write_begin uses the locked page
- * at *pagep rather than allocating its own. In this case, the page will
- * not be unlocked or deallocated on failure.
- */
-int block_write_begin(struct file *file, struct address_space *mapping,
-			loff_t pos, unsigned len, unsigned flags,
-			struct page **pagep, void **fsdata,
-			get_block_t *get_block)
-{
-	int ret;
-
-	ret = block_write_begin_newtrunc(file, mapping, pos, len, flags,
-					pagep, fsdata, get_block);
-
-	/*
-	 * prepare_write() may have instantiated a few blocks
-	 * outside i_size.  Trim these off again. Don't need
-	 * i_size_read because we hold i_mutex.
-	 *
-	 * Filesystems which pass down their own page also cannot
-	 * call into vmtruncate here because it would lead to lock
-	 * inversion problems (*pagep is locked). This is a further
-	 * example of where the old truncate sequence is inadequate.
-	 */
-	if (unlikely(ret) && *pagep == NULL) {
-		loff_t isize = mapping->host->i_size;
-		if (pos + len > isize)
-			vmtruncate(mapping->host, isize);
-	}
-
-	return ret;
-}
 EXPORT_SYMBOL(block_write_begin);
 
 int block_write_end(struct file *file, struct address_space *mapping,
@@ -2356,7 +2317,7 @@ int cont_write_begin(struct file *file,
 
 	err = cont_expand_zero(file, mapping, pos, bytes);
 	if (err)
-		goto out;
+		return err;
 
 	zerofrom = *bytes & ~PAGE_CACHE_MASK;
 	if (pos+len > *bytes && zerofrom & (blocksize-1)) {
@@ -2364,11 +2325,7 @@ int cont_write_begin(struct file *file,
 		(*bytes)++;
 	}
 
-	*pagep = NULL;
-	err = block_write_begin_newtrunc(file, mapping, pos, len,
-				flags, pagep, fsdata, get_block);
-out:
-	return err;
+	return block_write_begin(mapping, pos, len, flags, pagep, get_block);
 }
 EXPORT_SYMBOL(cont_write_begin);
 
@@ -2510,8 +2467,8 @@ int nobh_write_begin(struct address_spac
 		unlock_page(page);
 		page_cache_release(page);
 		*pagep = NULL;
-		return block_write_begin_newtrunc(NULL, mapping, pos, len,
-					flags, pagep, fsdata, get_block);
+		return block_write_begin(mapping, pos, len, flags, pagep,
+					 get_block);
 	}
 
 	if (PageMappedToDisk(page))
Index: linux-2.6/fs/bfs/file.c
===================================================================
--- linux-2.6.orig/fs/bfs/file.c	2010-05-30 22:40:02.899254241 +0200
+++ linux-2.6/fs/bfs/file.c	2010-05-30 22:40:19.844004484 +0200
@@ -168,9 +168,17 @@ static int bfs_write_begin(struct file *
 			loff_t pos, unsigned len, unsigned flags,
 			struct page **pagep, void **fsdata)
 {
-	*pagep = NULL;
-	return block_write_begin(file, mapping, pos, len, flags,
-					pagep, fsdata, bfs_get_block);
+	int ret;
+
+	ret = block_write_begin(mapping, pos, len, flags, pagep,
+				bfs_get_block);
+	if (unlikely(ret)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+	}
+
+	return ret;
 }
 
 static sector_t bfs_bmap(struct address_space *mapping, sector_t block)
Index: linux-2.6/fs/block_dev.c
===================================================================
--- linux-2.6.orig/fs/block_dev.c	2010-05-30 22:40:02.911254172 +0200
+++ linux-2.6/fs/block_dev.c	2010-05-30 22:40:19.850004694 +0200
@@ -308,9 +308,8 @@ static int blkdev_write_begin(struct fil
 			loff_t pos, unsigned len, unsigned flags,
 			struct page **pagep, void **fsdata)
 {
-	*pagep = NULL;
-	return block_write_begin_newtrunc(file, mapping, pos, len, flags,
-				pagep, fsdata, blkdev_get_block);
+	return block_write_begin(mapping, pos, len, flags, pagep,
+				 blkdev_get_block);
 }
 
 static int blkdev_write_end(struct file *file, struct address_space *mapping,
Index: linux-2.6/fs/nilfs2/inode.c
===================================================================
--- linux-2.6.orig/fs/nilfs2/inode.c	2010-05-30 22:40:02.920254661 +0200
+++ linux-2.6/fs/nilfs2/inode.c	2010-05-30 22:40:19.856004275 +0200
@@ -197,11 +197,15 @@ static int nilfs_write_begin(struct file
 	if (unlikely(err))
 		return err;
 
-	*pagep = NULL;
-	err = block_write_begin(file, mapping, pos, len, flags, pagep,
-				fsdata, nilfs_get_block);
-	if (unlikely(err))
+	err = block_write_begin(mapping, pos, len, flags, pagep,
+				nilfs_get_block);
+	if (unlikely(err)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+
 		nilfs_transaction_abort(inode->i_sb);
+	}
 	return err;
 }
 
Index: linux-2.6/fs/nilfs2/recovery.c
===================================================================
--- linux-2.6.orig/fs/nilfs2/recovery.c	2010-05-30 22:40:02.927254172 +0200
+++ linux-2.6/fs/nilfs2/recovery.c	2010-05-30 22:40:19.861004135 +0200
@@ -505,11 +505,14 @@ static int recover_dsync_blocks(struct n
 		}
 
 		pos = rb->blkoff << inode->i_blkbits;
-		page = NULL;
-		err = block_write_begin(NULL, inode->i_mapping, pos, blocksize,
-					0, &page, NULL, nilfs_get_block);
-		if (unlikely(err))
+		err = block_write_begin(inode->i_mapping, pos, blocksize,
+					0, &page, nilfs_get_block);
+		if (unlikely(err)) {
+			loff_t isize = inode->i_size;
+			if (pos + blocksize > isize)
+				vmtruncate(inode, isize);
 			goto failed_inode;
+		}
 
 		err = nilfs_recovery_copy_block(sbi, rb, page);
 		if (unlikely(err))
Index: linux-2.6/fs/omfs/file.c
===================================================================
--- linux-2.6.orig/fs/omfs/file.c	2010-05-30 22:40:02.933254381 +0200
+++ linux-2.6/fs/omfs/file.c	2010-05-30 22:40:19.865004624 +0200
@@ -312,9 +312,17 @@ static int omfs_write_begin(struct file
 			loff_t pos, unsigned len, unsigned flags,
 			struct page **pagep, void **fsdata)
 {
-	*pagep = NULL;
-	return block_write_begin(file, mapping, pos, len, flags,
-				pagep, fsdata, omfs_get_block);
+	int ret;
+
+	ret = block_write_begin(mapping, pos, len, flags, pagep,
+				omfs_get_block);
+	if (unlikely(ret)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+	}
+
+	return ret;
 }
 
 static sector_t omfs_bmap(struct address_space *mapping, sector_t block)
Index: linux-2.6/fs/udf/inode.c
===================================================================
--- linux-2.6.orig/fs/udf/inode.c	2010-05-30 22:40:02.952254591 +0200
+++ linux-2.6/fs/udf/inode.c	2010-05-30 22:40:19.870003437 +0200
@@ -132,9 +132,16 @@ static int udf_write_begin(struct file *
 			loff_t pos, unsigned len, unsigned flags,
 			struct page **pagep, void **fsdata)
 {
-	*pagep = NULL;
-	return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
-				udf_get_block);
+	int ret;
+
+	ret = block_write_begin(mapping, pos, len, flags, pagep, udf_get_block);
+	if (unlikely(ret)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+	}
+
+	return ret;
 }
 
 static sector_t udf_bmap(struct address_space *mapping, sector_t block)
Index: linux-2.6/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- linux-2.6.orig/fs/xfs/linux-2.6/xfs_aops.c	2010-05-30 22:40:02.960254661 +0200
+++ linux-2.6/fs/xfs/linux-2.6/xfs_aops.c	2010-05-30 22:40:19.872004275 +0200
@@ -1677,9 +1677,17 @@ xfs_vm_write_begin(
 	struct page		**pagep,
 	void			**fsdata)
 {
-	*pagep = NULL;
-	return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
-								xfs_get_blocks);
+	int			ret;
+
+	ret = block_write_begin(mapping, pos, len, flags, pagep,
+				xfs_get_blocks);
+	if (unlikely(ret)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+	}
+
+	return ret;
 }
 
 STATIC sector_t
Index: linux-2.6/fs/ext2/inode.c
===================================================================
--- linux-2.6.orig/fs/ext2/inode.c	2010-05-30 22:40:02.972254311 +0200
+++ linux-2.6/fs/ext2/inode.c	2010-05-30 22:40:19.877004415 +0200
@@ -772,9 +772,8 @@ ext2_write_begin(struct file *file, stru
 {
 	int ret;
 
-	*pagep = NULL;
-	ret = block_write_begin_newtrunc(file, mapping, pos, len, flags,
-					 pagep, fsdata, ext2_get_block);
+	ret = block_write_begin(mapping, pos, len, flags, pagep,
+				ext2_get_block);
 	if (ret < 0)
 		ext2_write_failed(mapping, pos + len);
 	return ret;
Index: linux-2.6/fs/minix/inode.c
===================================================================
--- linux-2.6.orig/fs/minix/inode.c	2010-05-30 22:40:02.980254241 +0200
+++ linux-2.6/fs/minix/inode.c	2010-05-30 22:40:19.882004345 +0200
@@ -366,9 +366,17 @@ static int minix_write_begin(struct file
 			loff_t pos, unsigned len, unsigned flags,
 			struct page **pagep, void **fsdata)
 {
-	*pagep = NULL;
-	return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+	int ret;
+
+	ret = block_write_begin(mapping, pos, len, flags, pagep,
 				minix_get_block);
+	if (unlikely(ret)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+	}
+
+	return ret;
 }
 
 static sector_t minix_bmap(struct address_space *mapping, sector_t block)
Index: linux-2.6/fs/sysv/itree.c
===================================================================
--- linux-2.6.orig/fs/sysv/itree.c	2010-05-30 22:40:02.987254172 +0200
+++ linux-2.6/fs/sysv/itree.c	2010-05-30 22:40:19.885004554 +0200
@@ -468,9 +468,16 @@ static int sysv_write_begin(struct file
 			loff_t pos, unsigned len, unsigned flags,
 			struct page **pagep, void **fsdata)
 {
-	*pagep = NULL;
-	return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
-				get_block);
+	int ret;
+
+	ret = block_write_begin(mapping, pos, len, flags, pagep, get_block);
+	if (unlikely(ret)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+	}
+
+	return ret;
 }
 
 static sector_t sysv_bmap(struct address_space *mapping, sector_t block)
Index: linux-2.6/include/linux/buffer_head.h
===================================================================
--- linux-2.6.orig/include/linux/buffer_head.h	2010-05-30 22:40:03.005253892 +0200
+++ linux-2.6/include/linux/buffer_head.h	2010-05-30 22:40:19.889004624 +0200
@@ -203,12 +203,8 @@ int block_write_full_page_endio(struct p
 int block_read_full_page(struct page*, get_block_t*);
 int block_is_partially_uptodate(struct page *page, read_descriptor_t *desc,
 				unsigned long from);
-int block_write_begin_newtrunc(struct file *, struct address_space *,
-				loff_t, unsigned, unsigned,
-				struct page **, void **, get_block_t*);
-int block_write_begin(struct file *, struct address_space *,
-				loff_t, unsigned, unsigned,
-				struct page **, void **, get_block_t*);
+int block_write_begin(struct address_space *mapping, loff_t pos, unsigned len,
+		unsigned flags, struct page **pagep, get_block_t *get_block);
 int __block_write_begin(struct page *page, loff_t pos, unsigned len,
 		get_block_t *get_block);
 int block_write_end(struct file *, struct address_space *,
Index: linux-2.6/fs/ufs/inode.c
===================================================================
--- linux-2.6.orig/fs/ufs/inode.c	2010-05-30 22:40:02.996254591 +0200
+++ linux-2.6/fs/ufs/inode.c	2010-05-30 22:40:19.893004764 +0200
@@ -568,9 +568,17 @@ static int ufs_write_begin(struct file *
 			loff_t pos, unsigned len, unsigned flags,
 			struct page **pagep, void **fsdata)
 {
-	*pagep = NULL;
-	return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+	int ret;
+
+	ret = block_write_begin(mapping, pos, len, flags, pagep,
 				ufs_getfrag_block);
+	if (unlikely(ret)) {
+		loff_t isize = mapping->host->i_size;
+		if (pos + len > isize)
+			vmtruncate(mapping->host, isize);
+	}
+
+	return ret;
 }
 
 static sector_t ufs_bmap(struct address_space *mapping, sector_t block)

Re: [PATCH 0/6] first step toward the new truncate sequence

[Nick Piggin]

On Sun, May 30, 2010 at 10:49:32PM +0200, Christoph Hellwig wrote:
> This series is something that I see as the first major step towards
> a broad switch to the new truncate sequence.  The patches get rid
> of the _newtrunc variant of blockdev_direct_IO & friends and
> *_write_begin, and clean up some bits in that area that make the
> switch easier.  After this we have all vmtruncate instances except
> for inode_setattr in filesystem code.

Nice. It looks good.


>  A second series to deal
> with ->setattr will follow and after that we can easily switch
> over one filesystem after another.
> 
> I think this is still 2.6.34 material as it will make the fs
> switches a lot easier and avoid introducing the _newtrunc variants
> for one kernel release.

No objections from me, but what is the policy on changing exported APIs
without changing the name?

Re: [PATCH 0/6] first step toward the new truncate sequence

[Christoph Hellwig]

On Mon, May 31, 2010 at 05:13:53PM +1000, Nick Piggin wrote:
> >  A second series to deal
> > with ->setattr will follow and after that we can easily switch
> > over one filesystem after another.
> > 
> > I think this is still 2.6.34 material as it will make the fs
> > switches a lot easier and avoid introducing the _newtrunc variants
> > for one kernel release.
> 
> No objections from me, but what is the policy on changing exported APIs
> without changing the name?

There is no policy, but we do it all the time.  Note that we also change
the prototypes for all but cont_write_begin so it's an easy to spot
compile breakage.

Re: [PATCH 0/6] first step toward the new truncate sequence

[Nick Piggin]

On Mon, May 31, 2010 at 09:38:51AM +0200, Christoph Hellwig wrote:
> On Mon, May 31, 2010 at 05:13:53PM +1000, Nick Piggin wrote:
> > >  A second series to deal
> > > with ->setattr will follow and after that we can easily switch
> > > over one filesystem after another.
> > > 
> > > I think this is still 2.6.34 material as it will make the fs
> > > switches a lot easier and avoid introducing the _newtrunc variants
> > > for one kernel release.
> > 
> > No objections from me, but what is the policy on changing exported APIs
> > without changing the name?
> 
> There is no policy, but we do it all the time.

Yeah, just wondering. There is a pseudo don't-break-exported-symbols
thing happening sometimes but while I think it sometimes catches
removed symbols in reviews, more subtle breakage probably usually gets
past.


>  Note that we also change
> the prototypes for all but cont_write_begin so it's an easy to spot
> compile breakage.

True.

Re: [PATCH 0/6] first step toward the new truncate sequence

[Boaz Harrosh]

On 05/30/2010 11:49 PM, Christoph Hellwig wrote:
> This series is something that I see as the first major step towards
> a broad switch to the new truncate sequence.  The patches get rid
> of the _newtrunc variant of blockdev_direct_IO & friends and
> *_write_begin, and clean up some bits in that area that make the
> switch easier.  After this we have all vmtruncate instances except
> for inode_setattr in filesystem code.  A second series to deal
> with ->setattr will follow and after that we can easily switch
> over one filesystem after another.
> 
> I think this is still 2.6.34 material as it will make the fs
> switches a lot easier and avoid introducing the _newtrunc variants
> for one kernel release.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Christoph, Nick hi.

I'm attempting a truncate conversion for exofs, will need a review.

I'm basing the first attempt on Al's next tree that has Nicks last
patches. Will rebase on these when they get into that tree. Please
advise on the best tree to use?
(BTW did you mean 2.6.35 above, I guess)

So one minor thing ext2_setsize can be static: 
(It used to be used by the struct inode_operations)

---
git diff --stat -p -M fs/ext2/inode.c
 fs/ext2/inode.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 1921443..c8ae0da 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -1169,7 +1169,7 @@ static void ext2_truncate_blocks(struct inode *inode, loff_t offset)
 	__ext2_truncate_blocks(inode, offset);
 }
 
-int ext2_setsize(struct inode *inode, loff_t newsize)
+static int ext2_setsize(struct inode *inode, loff_t newsize)
 {
 	loff_t oldsize;
 	int error;

Re: [PATCH 0/6] first step toward the new truncate sequence

[Boaz Harrosh]

On 05/31/2010 12:50 PM, Boaz Harrosh wrote:
> On 05/30/2010 11:49 PM, Christoph Hellwig wrote:
>> This series is something that I see as the first major step towards
>> a broad switch to the new truncate sequence.  The patches get rid
>> of the _newtrunc variant of blockdev_direct_IO & friends and
>> *_write_begin, and clean up some bits in that area that make the
>> switch easier.  After this we have all vmtruncate instances except
>> for inode_setattr in filesystem code.  A second series to deal
>> with ->setattr will follow and after that we can easily switch
>> over one filesystem after another.
>>
>> I think this is still 2.6.34 material as it will make the fs
>> switches a lot easier and avoid introducing the _newtrunc variants
>> for one kernel release.
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> Christoph, Nick hi.
> 
> I'm attempting a truncate conversion for exofs, will need a review.
> 
> I'm basing the first attempt on Al's next tree that has Nicks last
> patches. Will rebase on these when they get into that tree. Please
> advise on the best tree to use?
> (BTW did you mean 2.6.35 above, I guess)
> 
> So one minor thing ext2_setsize can be static: 
> (It used to be used by the struct inode_operations)
> 

One more thing. ext2_setsize is only used by ext2_setattr do we still
need this code? (form ext2_setsize) half of it is done in ext2_setattr
already)

	inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
	if (inode_needs_sync(inode)) {
		sync_mapping_buffers(inode->i_mapping);
		ext2_sync_inode (inode);
	} else {
		mark_inode_dirty(inode);
	}

And generic_setattr does some more of that, No?

Boaz

Re: [PATCH 0/6] first step toward the new truncate sequence

[Nick Piggin]

On Mon, May 31, 2010 at 01:17:35PM +0300, Boaz Harrosh wrote:
> On 05/31/2010 12:50 PM, Boaz Harrosh wrote:
> > On 05/30/2010 11:49 PM, Christoph Hellwig wrote:
> >> This series is something that I see as the first major step towards
> >> a broad switch to the new truncate sequence.  The patches get rid
> >> of the _newtrunc variant of blockdev_direct_IO & friends and
> >> *_write_begin, and clean up some bits in that area that make the
> >> switch easier.  After this we have all vmtruncate instances except
> >> for inode_setattr in filesystem code.  A second series to deal
> >> with ->setattr will follow and after that we can easily switch
> >> over one filesystem after another.
> >>
> >> I think this is still 2.6.34 material as it will make the fs
> >> switches a lot easier and avoid introducing the _newtrunc variants
> >> for one kernel release.
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> > Christoph, Nick hi.
> > 
> > I'm attempting a truncate conversion for exofs, will need a review.
> > 
> > I'm basing the first attempt on Al's next tree that has Nicks last
> > patches. Will rebase on these when they get into that tree. Please
> > advise on the best tree to use?
> > (BTW did you mean 2.6.35 above, I guess)
> > 
> > So one minor thing ext2_setsize can be static: 

Thanks.


> > (It used to be used by the struct inode_operations)
> > 
> 
> One more thing. ext2_setsize is only used by ext2_setattr do we still
> need this code? (form ext2_setsize) half of it is done in ext2_setattr
> already)
> 
> 	inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
> 	if (inode_needs_sync(inode)) {
> 		sync_mapping_buffers(inode->i_mapping);
> 		ext2_sync_inode (inode);
> 	} else {
> 		mark_inode_dirty(inode);
> 	}
> 
> And generic_setattr does some more of that, No?

Not sure. Some callers appear not to set ATTR_CTIME/ATTR_MTIME when
making ATTR_SIZE changes. And I don't know the history of the required
sync semantics here either.

I think these would be good questions to raise with the ext?
maintainers, though. ext3/4 have similar code.

Re: [PATCH 0/6] first step toward the new truncate sequence

[Christoph Hellwig]

On Mon, May 31, 2010 at 11:15:51PM +1000, Nick Piggin wrote:
> Not sure. Some callers appear not to set ATTR_CTIME/ATTR_MTIME when
> making ATTR_SIZE changes. And I don't know the history of the required
> sync semantics here either.

It signals the difference between truncate and ftruncate.  See the
comment in xfs_setattr:

	/*
	 * Only change the c/mtime if we are changing the size
	 * or we are explicitly asked to change it. This handles
	 * the semantic difference between truncate() and ftruncate()
	 * as implemented in the VFS.
	 *
	 * The regular truncate() case without ATTR_CTIME and ATTR_MTIME
	 * is a special case where we need to update the times despite
	 * not having these flags set.  For all other operations the
	 * VFS set these flags explicitly if it wants a timestamp
	 * update.
	 */

Re: [PATCH 0/6] first step toward the new truncate sequence

[Boaz Harrosh]

On 05/31/2010 04:15 PM, Nick Piggin wrote:
> On Mon, May 31, 2010 at 01:17:35PM +0300, Boaz Harrosh wrote:
>> On 05/31/2010 12:50 PM, Boaz Harrosh wrote:
>>> On 05/30/2010 11:49 PM, Christoph Hellwig wrote:
>>>> This series is something that I see as the first major step towards
>>>> a broad switch to the new truncate sequence.  The patches get rid
>>>> of the _newtrunc variant of blockdev_direct_IO & friends and
>>>> *_write_begin, and clean up some bits in that area that make the
>>>> switch easier.  After this we have all vmtruncate instances except
>>>> for inode_setattr in filesystem code.  A second series to deal
>>>> with ->setattr will follow and after that we can easily switch
>>>> over one filesystem after another.
>>>>
>>>> I think this is still 2.6.34 material as it will make the fs
>>>> switches a lot easier and avoid introducing the _newtrunc variants
>>>> for one kernel release.
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>>
>>> Christoph, Nick hi.
>>>
>>> I'm attempting a truncate conversion for exofs, will need a review.
>>>
>>> I'm basing the first attempt on Al's next tree that has Nicks last
>>> patches. Will rebase on these when they get into that tree. Please
>>> advise on the best tree to use?
>>> (BTW did you mean 2.6.35 above, I guess)
>>>
>>> So one minor thing ext2_setsize can be static: 
> 
> Thanks.
> 
> 
>>> (It used to be used by the struct inode_operations)
>>>
>>
>> One more thing. ext2_setsize is only used by ext2_setattr do we still
>> need this code? (form ext2_setsize) half of it is done in ext2_setattr
>> already)
>>
>> 	inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
>> 	if (inode_needs_sync(inode)) {
>> 		sync_mapping_buffers(inode->i_mapping);
>> 		ext2_sync_inode (inode);
>> 	} else {
>> 		mark_inode_dirty(inode);
>> 	}
>>
>> And generic_setattr does some more of that, No?
> 
> Not sure. Some callers appear not to set ATTR_CTIME/ATTR_MTIME when
> making ATTR_SIZE changes. And I don't know the history of the required
> sync semantics here either.
> 
> I think these would be good questions to raise with the ext?
> maintainers, though. ext3/4 have similar code.
> 

OK, I've rechecked and and no one is doing:
	inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
So this one is needed as is.

But the:
 	if (inode_needs_sync(inode)) {
 		sync_mapping_buffers(inode->i_mapping);
 		ext2_sync_inode (inode);
 	} else {
 		mark_inode_dirty(inode);
 	}

Is just stupid given that mark_inode_dirty(inode); is
again done in ext2_setattr() right after the call to
_setsize. Perhaps if needed, then move it into _setattr

(Please also have a look at my exofs: truncate patch I would love your input)
Boaz

Re: [PATCH 0/6] first step toward the new truncate sequence

[Christoph Hellwig]

On Mon, May 31, 2010 at 12:50:00PM +0300, Boaz Harrosh wrote:
> I'm basing the first attempt on Al's next tree that has Nicks last
> patches. Will rebase on these when they get into that tree. Please
> advise on the best tree to use?
> (BTW did you mean 2.6.35 above, I guess)

That really depends on Al's merge plans.

> So one minor thing ext2_setsize can be static: 
> (It used to be used by the struct inode_operations)

Indeed, I'll fold it into a related patch in my third batch.