sort out lose ends after the O_SYNC series

sort out lose ends after the O_SYNC series

[Christoph Hellwig]

Sort out some left overs of the O_SYNC series.    The first patch
removes wait_on_writeback_range after converting the remaining user
over to the much better filemap_fdatawait_range interface, and the
other two remove some unessecary manual O_SYNC handling in afs and
ubifs.

All this requires my O_DSYNC patch applied first.

[PATCH 1/3] kill wait_on_page_writeback_range

[Christoph Hellwig]

All callers really want the more logical filemap_fdatawait_range interface,
so convert them to use it and merge wait_on_page_writeback_range into
filemap_fdatawait_range.


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

Index: linux-2.6/Documentation/filesystems/vfs.txt
===================================================================
--- linux-2.6.orig/Documentation/filesystems/vfs.txt	2009-09-30 13:43:04.670028719 -0300
+++ linux-2.6/Documentation/filesystems/vfs.txt	2009-09-30 13:43:11.088004004 -0300
@@ -472,7 +472,7 @@ __sync_single_inode) to check if ->write
 writing out the whole address_space.
 
 The Writeback tag is used by filemap*wait* and sync_page* functions,
-via wait_on_page_writeback_range, to wait for all writeback to
+via filemap_fdatawait_range, to wait for all writeback to
 complete.  While waiting ->sync_page (if defined) will be called on
 each page that is found to require writeback.
 
Index: linux-2.6/fs/jbd2/commit.c
===================================================================
--- linux-2.6.orig/fs/jbd2/commit.c	2009-09-30 13:43:04.644024427 -0300
+++ linux-2.6/fs/jbd2/commit.c	2009-09-30 13:43:11.089004202 -0300
@@ -286,7 +286,7 @@ static int journal_finish_inode_data_buf
 		if (err) {
 			/*
 			 * Because AS_EIO is cleared by
-			 * wait_on_page_writeback_range(), set it again so
+			 * filemap_fdatawait_range(), set it again so
 			 * that user process can get -EIO from fsync().
 			 */
 			set_bit(AS_EIO,
Index: linux-2.6/fs/sync.c
===================================================================
--- linux-2.6.orig/fs/sync.c	2009-09-30 13:43:04.637021859 -0300
+++ linux-2.6/fs/sync.c	2009-09-30 13:43:11.092004514 -0300
@@ -452,9 +452,7 @@ int do_sync_mapping_range(struct address
 
 	ret = 0;
 	if (flags & SYNC_FILE_RANGE_WAIT_BEFORE) {
-		ret = wait_on_page_writeback_range(mapping,
-					offset >> PAGE_CACHE_SHIFT,
-					endbyte >> PAGE_CACHE_SHIFT);
+		ret = filemap_fdatawait_range(mapping, offset, endbyte);
 		if (ret < 0)
 			goto out;
 	}
@@ -467,9 +465,7 @@ int do_sync_mapping_range(struct address
 	}
 
 	if (flags & SYNC_FILE_RANGE_WAIT_AFTER) {
-		ret = wait_on_page_writeback_range(mapping,
-					offset >> PAGE_CACHE_SHIFT,
-					endbyte >> PAGE_CACHE_SHIFT);
+		ret = filemap_fdatawait_range(mapping, offset, endbyte);
 	}
 out:
 	return ret;
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2009-09-30 13:43:04.663004220 -0300
+++ linux-2.6/include/linux/fs.h	2009-09-30 13:43:11.096003767 -0300
@@ -2086,8 +2086,6 @@ extern int filemap_fdatawait_range(struc
 extern int filemap_write_and_wait(struct address_space *mapping);
 extern int filemap_write_and_wait_range(struct address_space *mapping,
 				        loff_t lstart, loff_t lend);
-extern int wait_on_page_writeback_range(struct address_space *mapping,
-				pgoff_t start, pgoff_t end);
 extern int __filemap_fdatawrite_range(struct address_space *mapping,
 				loff_t start, loff_t end, int sync_mode);
 extern int filemap_fdatawrite_range(struct address_space *mapping,
Index: linux-2.6/mm/filemap.c
===================================================================
--- linux-2.6.orig/mm/filemap.c	2009-09-30 13:43:04.655004248 -0300
+++ linux-2.6/mm/filemap.c	2009-09-30 13:45:00.480006033 -0300
@@ -260,27 +260,27 @@ int filemap_flush(struct address_space *
 EXPORT_SYMBOL(filemap_flush);
 
 /**
- * wait_on_page_writeback_range - wait for writeback to complete
- * @mapping:	target address_space
- * @start:	beginning page index
- * @end:	ending page index
+ * filemap_fdatawait_range - wait for writeback to complete
+ * @mapping:		address space structure to wait for
+ * @start_byte:		offset in bytes where the range starts
+ * @end_byte:		offset in bytes where the range ends (inclusive)
  *
- * Wait for writeback to complete against pages indexed by start->end
- * inclusive
+ * Walk the list of under-writeback pages of the given address space
+ * in the given range and wait for all of them.
  */
-int wait_on_page_writeback_range(struct address_space *mapping,
-				pgoff_t start, pgoff_t end)
+int filemap_fdatawait_range(struct address_space *mapping, loff_t start_byte,
+			    loff_t end_byte)
 {
+	pgoff_t index = start_byte >> PAGE_CACHE_SHIFT;
+	pgoff_t end = end_byte >> PAGE_CACHE_SHIFT;
 	struct pagevec pvec;
 	int nr_pages;
 	int ret = 0;
-	pgoff_t index;
 
-	if (end < start)
+	if (end_byte < start_byte)
 		return 0;
 
 	pagevec_init(&pvec, 0);
-	index = start;
 	while ((index <= end) &&
 			(nr_pages = pagevec_lookup_tag(&pvec, mapping, &index,
 			PAGECACHE_TAG_WRITEBACK,
@@ -310,25 +310,6 @@ int wait_on_page_writeback_range(struct 
 
 	return ret;
 }
-
-/**
- * filemap_fdatawait_range - wait for all under-writeback pages to complete in a given range
- * @mapping: address space structure to wait for
- * @start:	offset in bytes where the range starts
- * @end:	offset in bytes where the range ends (inclusive)
- *
- * Walk the list of under-writeback pages of the given address space
- * in the given range and wait for all of them.
- *
- * This is just a simple wrapper so that callers don't have to convert offsets
- * to page indexes themselves
- */
-int filemap_fdatawait_range(struct address_space *mapping, loff_t start,
-			    loff_t end)
-{
-	return wait_on_page_writeback_range(mapping, start >> PAGE_CACHE_SHIFT,
-					    end >> PAGE_CACHE_SHIFT);
-}
 EXPORT_SYMBOL(filemap_fdatawait_range);
 
 /**
@@ -345,8 +326,7 @@ int filemap_fdatawait(struct address_spa
 	if (i_size == 0)
 		return 0;
 
-	return wait_on_page_writeback_range(mapping, 0,
-				(i_size - 1) >> PAGE_CACHE_SHIFT);
+	return filemap_fdatawait_range(mapping, 0, i_size - 1);
 }
 EXPORT_SYMBOL(filemap_fdatawait);
 
@@ -393,9 +373,8 @@ int filemap_write_and_wait_range(struct 
 						 WB_SYNC_ALL);
 		/* See comment of filemap_write_and_wait() */
 		if (err != -EIO) {
-			int err2 = wait_on_page_writeback_range(mapping,
-						lstart >> PAGE_CACHE_SHIFT,
-						lend >> PAGE_CACHE_SHIFT);
+			int err2 = filemap_fdatawait_range(mapping,
+						lstart, lend);
 			if (!err)
 				err = err2;
 		}

Re: [PATCH 1/3] kill wait_on_page_writeback_range

[Jan Kara]

On Wed 30-09-09 22:16:33, Christoph Hellwig wrote:
> 
> All callers really want the more logical filemap_fdatawait_range interface,
> so convert them to use it and merge wait_on_page_writeback_range into
> filemap_fdatawait_range.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
  Looks good. Acked-by: Jan Kara <jack@suse.cz>

									Honza
> 
> Index: linux-2.6/Documentation/filesystems/vfs.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/filesystems/vfs.txt	2009-09-30 13:43:04.670028719 -0300
> +++ linux-2.6/Documentation/filesystems/vfs.txt	2009-09-30 13:43:11.088004004 -0300
> @@ -472,7 +472,7 @@ __sync_single_inode) to check if ->write
>  writing out the whole address_space.
>  
>  The Writeback tag is used by filemap*wait* and sync_page* functions,
> -via wait_on_page_writeback_range, to wait for all writeback to
> +via filemap_fdatawait_range, to wait for all writeback to
>  complete.  While waiting ->sync_page (if defined) will be called on
>  each page that is found to require writeback.
>  
> Index: linux-2.6/fs/jbd2/commit.c
> ===================================================================
> --- linux-2.6.orig/fs/jbd2/commit.c	2009-09-30 13:43:04.644024427 -0300
> +++ linux-2.6/fs/jbd2/commit.c	2009-09-30 13:43:11.089004202 -0300
> @@ -286,7 +286,7 @@ static int journal_finish_inode_data_buf
>  		if (err) {
>  			/*
>  			 * Because AS_EIO is cleared by
> -			 * wait_on_page_writeback_range(), set it again so
> +			 * filemap_fdatawait_range(), set it again so
>  			 * that user process can get -EIO from fsync().
>  			 */
>  			set_bit(AS_EIO,
> Index: linux-2.6/fs/sync.c
> ===================================================================
> --- linux-2.6.orig/fs/sync.c	2009-09-30 13:43:04.637021859 -0300
> +++ linux-2.6/fs/sync.c	2009-09-30 13:43:11.092004514 -0300
> @@ -452,9 +452,7 @@ int do_sync_mapping_range(struct address
>  
>  	ret = 0;
>  	if (flags & SYNC_FILE_RANGE_WAIT_BEFORE) {
> -		ret = wait_on_page_writeback_range(mapping,
> -					offset >> PAGE_CACHE_SHIFT,
> -					endbyte >> PAGE_CACHE_SHIFT);
> +		ret = filemap_fdatawait_range(mapping, offset, endbyte);
>  		if (ret < 0)
>  			goto out;
>  	}
> @@ -467,9 +465,7 @@ int do_sync_mapping_range(struct address
>  	}
>  
>  	if (flags & SYNC_FILE_RANGE_WAIT_AFTER) {
> -		ret = wait_on_page_writeback_range(mapping,
> -					offset >> PAGE_CACHE_SHIFT,
> -					endbyte >> PAGE_CACHE_SHIFT);
> +		ret = filemap_fdatawait_range(mapping, offset, endbyte);
>  	}
>  out:
>  	return ret;
> Index: linux-2.6/include/linux/fs.h
> ===================================================================
> --- linux-2.6.orig/include/linux/fs.h	2009-09-30 13:43:04.663004220 -0300
> +++ linux-2.6/include/linux/fs.h	2009-09-30 13:43:11.096003767 -0300
> @@ -2086,8 +2086,6 @@ extern int filemap_fdatawait_range(struc
>  extern int filemap_write_and_wait(struct address_space *mapping);
>  extern int filemap_write_and_wait_range(struct address_space *mapping,
>  				        loff_t lstart, loff_t lend);
> -extern int wait_on_page_writeback_range(struct address_space *mapping,
> -				pgoff_t start, pgoff_t end);
>  extern int __filemap_fdatawrite_range(struct address_space *mapping,
>  				loff_t start, loff_t end, int sync_mode);
>  extern int filemap_fdatawrite_range(struct address_space *mapping,
> Index: linux-2.6/mm/filemap.c
> ===================================================================
> --- linux-2.6.orig/mm/filemap.c	2009-09-30 13:43:04.655004248 -0300
> +++ linux-2.6/mm/filemap.c	2009-09-30 13:45:00.480006033 -0300
> @@ -260,27 +260,27 @@ int filemap_flush(struct address_space *
>  EXPORT_SYMBOL(filemap_flush);
>  
>  /**
> - * wait_on_page_writeback_range - wait for writeback to complete
> - * @mapping:	target address_space
> - * @start:	beginning page index
> - * @end:	ending page index
> + * filemap_fdatawait_range - wait for writeback to complete
> + * @mapping:		address space structure to wait for
> + * @start_byte:		offset in bytes where the range starts
> + * @end_byte:		offset in bytes where the range ends (inclusive)
>   *
> - * Wait for writeback to complete against pages indexed by start->end
> - * inclusive
> + * Walk the list of under-writeback pages of the given address space
> + * in the given range and wait for all of them.
>   */
> -int wait_on_page_writeback_range(struct address_space *mapping,
> -				pgoff_t start, pgoff_t end)
> +int filemap_fdatawait_range(struct address_space *mapping, loff_t start_byte,
> +			    loff_t end_byte)
>  {
> +	pgoff_t index = start_byte >> PAGE_CACHE_SHIFT;
> +	pgoff_t end = end_byte >> PAGE_CACHE_SHIFT;
>  	struct pagevec pvec;
>  	int nr_pages;
>  	int ret = 0;
> -	pgoff_t index;
>  
> -	if (end < start)
> +	if (end_byte < start_byte)
>  		return 0;
>  
>  	pagevec_init(&pvec, 0);
> -	index = start;
>  	while ((index <= end) &&
>  			(nr_pages = pagevec_lookup_tag(&pvec, mapping, &index,
>  			PAGECACHE_TAG_WRITEBACK,
> @@ -310,25 +310,6 @@ int wait_on_page_writeback_range(struct 
>  
>  	return ret;
>  }
> -
> -/**
> - * filemap_fdatawait_range - wait for all under-writeback pages to complete in a given range
> - * @mapping: address space structure to wait for
> - * @start:	offset in bytes where the range starts
> - * @end:	offset in bytes where the range ends (inclusive)
> - *
> - * Walk the list of under-writeback pages of the given address space
> - * in the given range and wait for all of them.
> - *
> - * This is just a simple wrapper so that callers don't have to convert offsets
> - * to page indexes themselves
> - */
> -int filemap_fdatawait_range(struct address_space *mapping, loff_t start,
> -			    loff_t end)
> -{
> -	return wait_on_page_writeback_range(mapping, start >> PAGE_CACHE_SHIFT,
> -					    end >> PAGE_CACHE_SHIFT);
> -}
>  EXPORT_SYMBOL(filemap_fdatawait_range);
>  
>  /**
> @@ -345,8 +326,7 @@ int filemap_fdatawait(struct address_spa
>  	if (i_size == 0)
>  		return 0;
>  
> -	return wait_on_page_writeback_range(mapping, 0,
> -				(i_size - 1) >> PAGE_CACHE_SHIFT);
> +	return filemap_fdatawait_range(mapping, 0, i_size - 1);
>  }
>  EXPORT_SYMBOL(filemap_fdatawait);
>  
> @@ -393,9 +373,8 @@ int filemap_write_and_wait_range(struct 
>  						 WB_SYNC_ALL);
>  		/* See comment of filemap_write_and_wait() */
>  		if (err != -EIO) {
> -			int err2 = wait_on_page_writeback_range(mapping,
> -						lstart >> PAGE_CACHE_SHIFT,
> -						lend >> PAGE_CACHE_SHIFT);
> +			int err2 = filemap_fdatawait_range(mapping,
> +						lstart, lend);
>  			if (!err)
>  				err = err2;
>  		}
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[PATCH 2/3]: afs: remove manual O_SYNC handling

[Christoph Hellwig]

generic_file_aio_write already calls into ->fsync to handle O_SYNC/O_DSYNC.
Remove the duplicate manual invocation.


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

Index: linux-2.6/fs/afs/write.c
===================================================================
--- linux-2.6.orig/fs/afs/write.c	2009-09-30 13:59:20.550278942 -0300
+++ linux-2.6/fs/afs/write.c	2009-09-30 14:01:32.739005666 -0300
@@ -671,7 +671,6 @@ ssize_t afs_file_write(struct kiocb *ioc
 	struct afs_vnode *vnode = AFS_FS_I(dentry->d_inode);
 	ssize_t result;
 	size_t count = iov_length(iov, nr_segs);
-	int ret;
 
 	_enter("{%x.%u},{%zu},%lu,",
 	       vnode->fid.vid, vnode->fid.vnode, count, nr_segs);
@@ -691,14 +690,6 @@ ssize_t afs_file_write(struct kiocb *ioc
 		return result;
 	}
 
-	/* return error values for O_SYNC and IS_SYNC() */
-	if (IS_SYNC(&vnode->vfs_inode) || iocb->ki_filp->f_flags & O_DSYNC) {
-		ret = afs_fsync(iocb->ki_filp, dentry,
-				(iocb->ki_filp->f_flags & __O_SYNC) ? 0 : 1);
-		if (ret < 0)
-			result = ret;
-	}
-
 	_leave(" = %zd", result);
 	return result;
 }

[PATCH 3/3] ubifs: remove manual O_SYNC handling

[Christoph Hellwig]

generic_file_aio_write already calls into ->fsync to handle O_SYNC/O_DSYNC.
Remove the duplicate call to ubifs_sync_wbufs_by_inode which is already
covered by ubifs_fsync.

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

Index: linux-2.6/fs/ubifs/file.c
===================================================================
--- linux-2.6.orig/fs/ubifs/file.c	2009-09-30 14:01:26.046278266 -0300
+++ linux-2.6/fs/ubifs/file.c	2009-09-30 14:02:16.850272504 -0300
@@ -1389,7 +1389,6 @@ static ssize_t ubifs_aio_write(struct ki
 			       unsigned long nr_segs, loff_t pos)
 {
 	int err;
-	ssize_t ret;
 	struct inode *inode = iocb->ki_filp->f_mapping->host;
 	struct ubifs_info *c = inode->i_sb->s_fs_info;
 
@@ -1397,17 +1396,7 @@ static ssize_t ubifs_aio_write(struct ki
 	if (err)
 		return err;
 
-	ret = generic_file_aio_write(iocb, iov, nr_segs, pos);
-	if (ret < 0)
-		return ret;
-
-	if (ret > 0 && (IS_SYNC(inode) || iocb->ki_filp->f_flags & O_DSYNC)) {
-		err = ubifs_sync_wbufs_by_inode(c, inode);
-		if (err)
-			return err;
-	}
-
-	return ret;
+	return generic_file_aio_write(iocb, iov, nr_segs, pos);
 }
 
 static int ubifs_set_page_dirty(struct page *page)

Re: [PATCH 3/3] ubifs: remove manual O_SYNC handling

[Artem Bityutskiy]

On Wed, 2009-09-30 at 22:17 +0200, Christoph Hellwig wrote:
> generic_file_aio_write already calls into ->fsync to handle O_SYNC/O_DSYNC.
> Remove the duplicate call to ubifs_sync_wbufs_by_inode which is already
> covered by ubifs_fsync.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Pushed to ubifs-2.6.git tree, thanks!

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

--
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

Re: [PATCH 3/3] ubifs: remove manual O_SYNC handling

[Artem Bityutskiy]

On Thu, 2009-10-01 at 10:39 +0300, Artem Bityutskiy wrote:
> On Wed, 2009-09-30 at 22:17 +0200, Christoph Hellwig wrote:
> > generic_file_aio_write already calls into ->fsync to handle O_SYNC/O_DSYNC.
> > Remove the duplicate call to ubifs_sync_wbufs_by_inode which is already
> > covered by ubifs_fsync.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Pushed to ubifs-2.6.git tree, thanks!

Just to make sure, the patch looks independent on anything else to me,
right? So I pushed it to my 2.6.32-rc1 - based tree.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

--
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

Re: [PATCH 3/3] ubifs: remove manual O_SYNC handling

[Jan Kara]

On Thu 01-10-09 10:42:46, Artem Bityutskiy wrote:
> On Thu, 2009-10-01 at 10:39 +0300, Artem Bityutskiy wrote:
> > On Wed, 2009-09-30 at 22:17 +0200, Christoph Hellwig wrote:
> > > generic_file_aio_write already calls into ->fsync to handle O_SYNC/O_DSYNC.
> > > Remove the duplicate call to ubifs_sync_wbufs_by_inode which is already
> > > covered by ubifs_fsync.
> > > 
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > 
> > Pushed to ubifs-2.6.git tree, thanks!
> 
> Just to make sure, the patch looks independent on anything else to me,
> right? So I pushed it to my 2.6.32-rc1 - based tree.
  There is a patch dependency on a Christoph's O_DSYNC patch because the
check for O_DSYNC used to be a check for O_SYNC. Functionally, there's
no dependency...

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

Re: [PATCH 2/3]: afs: remove manual O_SYNC handling

[David Howells]

Christoph Hellwig <hch@lst.de> wrote:

> generic_file_aio_write already calls into ->fsync to handle O_SYNC/O_DSYNC.
> Remove the duplicate manual invocation.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: David Howells <dhowells@redhat.com>