Re: [PATCH 2/5] xfs: Introduce writeback context for writepages

[Christoph Hellwig <hch@infradead.org>]

This looks good in general and now passes testing for me.  A couple
comments below:

>  
>  /*
> - * Cancel submission of all buffer_heads so far in this endio.
> - * Toss the endio too.  Only ever called for the initial page
> - * in a writepage request, so only ever one page.
> - */
> -STATIC void
> -xfs_cancel_ioend(
> -	xfs_ioend_t		*ioend)
> -{
> -	xfs_ioend_t		*next;
> -	struct buffer_head	*bh, *next_bh;
> -
> -	do {
> -		next = ioend->io_list;
> -		bh = ioend->io_buffer_head;
> -		do {
> -			next_bh = bh->b_private;
> -			clear_buffer_async_write(bh);
> -			/*
> -			 * The unwritten flag is cleared when added to the
> -			 * ioend. We're not submitting for I/O so mark the
> -			 * buffer unwritten again for next time around.
> -			 */
> -			if (ioend->io_type == XFS_IO_UNWRITTEN)
> -				set_buffer_unwritten(bh);
> -			unlock_buffer(bh);
> -		} while ((bh = next_bh) != NULL);
> -
> -		mempool_free(ioend, xfs_ioend_pool);
> -	} while ((ioend = next) != NULL);
> -}

Removing xfs_cancel_ioend and replacing it with the start and cancel
writeback scheme that we currently only use for
xfs_setfilesize_trans_alloc failures actually seems to be the biggest
change in this patch and is entirely undocumented.  Any chance you
could split this into a prep patch and properly document it?

> -
> -	if (!ioend || need_ioend || type != ioend->io_type) {
> -		xfs_ioend_t	*previous = *result;
> -
> -		ioend = xfs_alloc_ioend(inode, type);
> -		ioend->io_offset = offset;
> -		ioend->io_buffer_head = bh;
> -		ioend->io_buffer_tail = bh;
> -		if (previous)
> -			previous->io_list = ioend;
> -		*result = ioend;
> +	if (!wpc->ioend || wpc->io_type != wpc->ioend->io_type ||
> +	    bh->b_blocknr != wpc->last_block + 1) {

We now start a new ioend if the blocks aren't contiguous, which seems
reasonable.  But this also means the similar check in xfs_submit_ioend
should be removed at the same time.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs