Re: [PATCH 1/3] kill wait_on_page_writeback_range

[Jan Kara <jack@suse.cz>]

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