Re: [PATCH 2/4] xfs: use unhashed buffers for size checks

[Christoph Hellwig <hch@infradead.org>]

> +struct xfs_buf *
> +xfs_buf_read_uncached(
> +	struct xfs_mount	*mp,
> +	struct xfs_buftarg	*target,
> +	xfs_daddr_t		daddr,
> +	size_t			length)
> +{
> +	xfs_buf_t	*bp;
> +	int		error;

struct xfs_buf and the same indentation as the parameters, please.

> +
> +	bp = xfs_buf_get_noaddr(length, target);

I think both the buf_get and buf_read interfaces for the non-hash
buffers should have the same name.  Either your uncached or maybe better
unhashed?  (And certainly no noaddr, which is not very useful)

> +	if (!bp || XFS_BUF_ISERROR(bp))
> +		goto fail;

xfs_buf_get_noaddr never returns an error in the buffer.

> +	/* set up the buffer for a read IO */
> +	xfs_buf_lock(bp);
> +	XFS_BUF_SET_ADDR(bp, daddr);
> +	XFS_BUF_READ(bp);
> +	XFS_BUF_BUSY(bp);
> +
> +	xfsbdstrat(mp, bp);
> +	error = xfs_iowait(bp);
> +	if (error || XFS_BUF_ISERROR(bp))
> +		goto fail;
> +
> +	return bp;
> +fail:
> +	if (bp)
> +		xfs_buf_relse(bp);

again, different labels please.

Also this one returns the buffer locked, while buf_get_noaddr doesn't.
I suspect we should also change buf_get_noaddr to return a locked buffer
to make it consistant with all other buf_read/get interfaces.

> +struct xfs_buf * xfs_buf_read_uncached(struct xfs_mount *mp,
> +				struct xfs_buftarg *target,
> +				xfs_daddr_t daddr, size_t length);

wrong placement of the *


This patch, or at least the introduction of the new read helper should
be moved before patch 1 so that we don't add code that gets removed a
little later.

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