Re: [PATCH 5/5] xfs: don't chain ioends during writepage submission

[Christoph Hellwig <hch@infradead.org>]

> +STATIC struct xfs_ioend *
>  xfs_add_to_ioend(
>  	struct inode		*inode,
>  	struct buffer_head	*bh,
>  	xfs_off_t		offset,
>  	struct xfs_writepage_ctx *wpc)
>  {
> +	struct xfs_ioend	*ioend_to_submit = NULL;

Maybe just

	struct xfs_ioend	*prev = NULL;

to be a little less verbose?

> @@ -738,29 +726,22 @@ xfs_writepage_submit(
>  	struct writeback_control *wbc,
>  	int			status)
>  {
> -	struct blk_plug		plug;
> -
> -	/* Reserve log space if we might write beyond the on-disk inode size. */
> -	if (!status && wpc->ioend && wpc->ioend->io_type != XFS_IO_UNWRITTEN &&
> -	    xfs_ioend_is_append(wpc->ioend))
> -		status = xfs_setfilesize_trans_alloc(wpc->ioend);
> -
> -	if (wpc->iohead) {
> -		blk_start_plug(&plug);
> -		xfs_submit_ioend(wbc, wpc->iohead, status);
> -		blk_finish_plug(&plug);
> -	}
> +	if (wpc->ioend)
> +		xfs_submit_ioend(wbc, wpc->ioend, status);
>  	return status;
>  }

With this change xfs_writepage_submit is rather pointless, I'd
rather open code it in the callers.

> +			ioend = xfs_add_to_ioend(inode, bh, offset, wpc);
> +			if (ioend) {
> +				ioend->io_list = NULL;
> +				if (!ioend_to_submit)
> +					ioend_to_submit = ioend;
> +				else
> +					ioend_tail->io_list = ioend;
> +				ioend_tail = ioend;
> +			}

Just using a list_head for this is a lot easier to read and less
error prone at the cost of a single additional pointer in the ioend.

> +	while (ioend_to_submit) {
> +		struct xfs_ioend *next = ioend_to_submit->io_list;
> +
> +		ioend_to_submit->io_list = NULL;
> +		xfs_submit_ioend(wbc, ioend_to_submit, 0);
> +		ioend_to_submit = next;
> +	}
>  	return 0;
>  
>  out_error:
> @@ -853,9 +848,16 @@ out_error:
>  	 * ioend, then we can't touch it here and need to rely on IO submission
>  	 * to unlock it.
>  	 */
> -	if (count)
> +	if (count) {
>  		xfs_start_page_writeback(page, 0, count);
> -	else {
> +		while (ioend_to_submit) {
> +			struct xfs_ioend *next = ioend_to_submit->io_list;
> +
> +			ioend_to_submit->io_list = NULL;
> +			xfs_submit_ioend(wbc, ioend_to_submit, 0);
> +			ioend_to_submit = next;
> +		}

I think this code cold be consolidated:

	ASSERT(wpc->ioend || !count);
out:
	if (count) {
		xfs_start_page_writeback(page, !error, count);
		while (ioend_to_submit) {
			struct xfs_ioend *next = ioend_to_submit->io_list;

			ioend_to_submit->io_list = NULL;
			xfs_submit_ioend(wbc, ioend_to_submit, 0);
			ioend_to_submit = next;
		}
	} else {
  		xfs_aops_discard_page(page);
  		ClearPageUptodate(page);
  		unlock_page(page);
  	}

	return error;

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