xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/4] xfs: use unhashed buffers for size checks
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 8 Sep 2010 21:38:07 -0400
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1283958778-28610-3-git-send-email-david@xxxxxxxxxxxxx>
References: <1283958778-28610-1-git-send-email-david@xxxxxxxxxxxxx> <1283958778-28610-3-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-08-17)
> +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.

<Prev in Thread] Current Thread [Next in Thread>