Re: [PATCH 05/10] xfs: kill xfs_iomap

From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 05/10] xfs: kill xfs_iomap
Date: Wed, 1 Dec 2010 15:32:27 +1100	[thread overview]
Message-ID: <20101201043227.GF16922@dastard> (raw)
In-Reply-To: <20101122130903.938808862@bombadil.infradead.org>

On Mon, Nov 22, 2010 at 08:05:11AM -0500, Christoph Hellwig wrote:
> Opencode the xfs_iomap code in it's two callers.  The overlap of passed
> flags already was minimal and will be further reduced in the next patch.
> 
> As a side effect the BMAPI_* flags for xfs_bmapi and the IO_* flags
> for I/O end processing are merged into a single set of flags, which
> should be a bit more descriptive of the operation we perform.
> 
> Also improve the tracing by giving each caller it's own type set of
> tracepoints.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks ok. Minor comment:

> @@ -1472,7 +1563,7 @@ xfs_vm_direct_IO(
>  	ssize_t			ret;
>  
>  	if (rw & WRITE) {
> -		iocb->private = xfs_alloc_ioend(inode, IO_NEW);
> +		iocb->private = xfs_alloc_ioend(inode, 0);
>  
>  		ret = __blockdev_direct_IO(rw, iocb, inode, bdev, iov,
>  					    offset, nr_segs,

Using an ioend type to "0" is not very obvious given all the other
uses have a defined type. I know that this converted to IO_UNWRITTEN
in IO completion if neceessary, but perhaps a IO_DIRECT type might
be better just to document it? Or perhaps a comment stating why 0 is
OK to use here?

Everything else looks fine, so:

Reviewed-by: Dave Chinner <dchinner@redhat.com>

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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