xfs
[Top] [All Lists]

Re: [PATCH 04/25] xfs: uncached buffer reads need to return an error

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 04/25] xfs: uncached buffer reads need to return an error
From: Phil White <pwhite@xxxxxxx>
Date: Mon, 29 Oct 2012 17:36:41 -0700
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1351146854-19343-5-git-send-email-david@xxxxxxxxxxxxx>
References: <1351146854-19343-1-git-send-email-david@xxxxxxxxxxxxx> <1351146854-19343-5-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Oct 25, 2012 at 05:33:53PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> With verification being done as an IO completion callback, different
> errors can be returned from a read. Uncached reads only return a
> buffer or NULL on failure, which means the verification error cannot
> be returned to the caller.
> 
> Split the error handling for these reads into two - a failure to get
> a buffer will still return NULL, but a read error will return a
> referenced buffer with b_error set rather than NULL. The caller is
> responsible for checking the error state of the buffer returned.
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
>  fs/xfs/xfs_buf.c     |    9 ++-------
>  fs/xfs/xfs_fsops.c   |    5 +++++
>  fs/xfs/xfs_mount.c   |    6 ++++++
>  fs/xfs/xfs_rtalloc.c |    9 ++++++++-
>  4 files changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 7cab1b3..62b7e89 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -715,8 +715,7 @@ xfs_buf_read_uncached(
>       int                     flags,
>       xfs_buf_iodone_t        verify)
>  {
> -     xfs_buf_t               *bp;
> -     int                     error;
> +     struct xfs_buf          *bp;
>  
>       bp = xfs_buf_get_uncached(target, numblks, flags);
>       if (!bp)
> @@ -730,11 +729,7 @@ xfs_buf_read_uncached(
>       bp->b_iodone = verify;
>  
>       xfsbdstrat(target->bt_mount, bp);
> -     error = xfs_buf_iowait(bp);
> -     if (error) {
> -             xfs_buf_relse(bp);
> -             return NULL;
> -     }
> +     xfs_buf_iowait(bp);
>       return bp;
>  }
>  
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 917e121..dee14eb 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -149,6 +149,11 @@ xfs_growfs_data_private(
>                               XFS_FSS_TO_BB(mp, 1), 0, NULL);
>       if (!bp)
>               return EIO;
> +     if (bp->b_error) {
> +             int     error = bp->b_error;
> +             xfs_buf_relse(bp);
> +             return error;
> +     }
>       xfs_buf_relse(bp);
>  
>       new = nb;       /* use new as a temporary here */
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index d39ad72..dc51e32 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -658,6 +658,12 @@ reread:
>                       xfs_warn(mp, "SB buffer read failed");
>               return EIO;
>       }
> +     if (bp->b_error) {
> +             error = bp->b_error;
> +             if (loud)
> +                     xfs_warn(mp, "SB validate failed");
> +             goto release_buf;
> +     }
>  
>       /*
>        * Initialize the mount structure from the superblock.
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index b271ed9..98dc670 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -1876,6 +1876,11 @@ xfs_growfs_rt(
>                               XFS_FSB_TO_BB(mp, 1), 0, NULL);
>       if (!bp)
>               return EIO;
> +     if (bp->b_error) {
> +             error = bp->b_error;
> +             xfs_buf_relse(bp);
> +             return error;
> +     }
>       xfs_buf_relse(bp);
>  
>       /*
> @@ -2221,8 +2226,10 @@ xfs_rtmount_init(
>       bp = xfs_buf_read_uncached(mp->m_rtdev_targp,
>                                       d - XFS_FSB_TO_BB(mp, 1),
>                                       XFS_FSB_TO_BB(mp, 1), 0, NULL);
> -     if (!bp) {
> +     if (!bp || bp->b_error) {
>               xfs_warn(mp, "realtime device size check failed");
> +             if (bp)
> +                     xfs_buf_relse(bp);
>               return EIO;
>       }
>       xfs_buf_relse(bp);
> -- 
> 1.7.10
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

Looks OK. 

Reviewed-by: Phil White <pwhite@xxxxxxx>

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