Re: Review - writing to multiple non-contiguous unwritten extents within a page is broken.

[Christoph Hellwig <hch@infradead.org>]

On Wed, May 23, 2007 at 07:21:03PM +1000, David Chinner wrote:
> Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_aops.c
> ===================================================================
> --- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_aops.c	2007-05-23 16:33:04.000000000 +1000
> +++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_aops.c	2007-05-23 17:52:15.540456674 +1000
> @@ -1008,6 +1008,8 @@ xfs_page_state_convert(
>  		if (buffer_unwritten(bh) || buffer_delay(bh) ||
>  		    ((buffer_uptodate(bh) || PageUptodate(page)) &&
>  		     !buffer_mapped(bh) && (unmapped || startio))) {
> +			int new_ioend = 0;
> +
>  			/*
>  			 * Make sure we don't use a read-only iomap
>  			 */
> @@ -1026,6 +1028,15 @@ xfs_page_state_convert(
>  			}
>  
>  			if (!iomap_valid) {
> +				/*
> +				 * if we didn't have a valid mapping then we
> +				 * need to ensure that we put the new mapping
> +				 * in a new ioend structure. This needs to be
> +				 * done to ensure that the ioends correctly
> +				 * reflect the block mappings at io completion
> +				 * for unwritten extent conversion.
> +				 */
> +				new_ioend = 1;
>  				if (type == IOMAP_NEW) {
>  					size = xfs_probe_cluster(inode,
>  							page, bh, head, 0);
> @@ -1045,7 +1056,7 @@ xfs_page_state_convert(
>  				if (startio) {
>  					xfs_add_to_ioend(inode, bh, offset,
>  							type, &ioend,
> -							!iomap_valid);
> +							new_ioend);

Looks good.  I'm pretty sure that we had something like a new_ioend variable
in the initial versions of this code, but I don't know when and why we
got rid of it.