diff mbox series

[01/11] xfs: make xfs_buf_alloc return an error code

Message ID 157924221800.3029431.576491686056157423.stgit@magnolia
State Superseded
Headers show
Series xfs: make buffer functions return error codes | expand

Commit Message

Darrick J. Wong Jan. 17, 2020, 6:23 a.m. UTC
From: Darrick J. Wong <darrick.wong@oracle.com>

Convert _xfs_buf_alloc() to return numeric error codes like most
everywhere else in xfs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf.c |   21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

Comments

Dave Chinner Jan. 19, 2020, 9:49 p.m. UTC | #1
On Thu, Jan 16, 2020 at 10:23:38PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Convert _xfs_buf_alloc() to return numeric error codes like most
> everywhere else in xfs.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
....
> @@ -715,8 +718,8 @@ xfs_buf_get_map(
>  		return NULL;
>  	}
>  
> -	new_bp = _xfs_buf_alloc(target, map, nmaps, flags);
> -	if (unlikely(!new_bp))
> +	error = _xfs_buf_alloc(target, map, nmaps, flags, &new_bp);
> +	if (unlikely(error))
>  		return NULL;

You can kill the unlikely() while you are at it. They are largely
unnecessary as the compiler already considers branches with returns
in them as unlikely and we don't need "unlikely" as code
documentation for error conditions like this...

>  	error = xfs_buf_allocate_memory(new_bp, flags);
> @@ -917,8 +920,8 @@ xfs_buf_get_uncached(
>  	DEFINE_SINGLE_BUF_MAP(map, XFS_BUF_DADDR_NULL, numblks);
>  
>  	/* flags might contain irrelevant bits, pass only what we care about */
> -	bp = _xfs_buf_alloc(target, &map, 1, flags & XBF_NO_IOACCT);
> -	if (unlikely(bp == NULL))
> +	error = _xfs_buf_alloc(target, &map, 1, flags & XBF_NO_IOACCT, &bp);
> +	if (unlikely(error))
>  		goto fail;

here too.

Cheers,

Dave.
Darrick J. Wong Jan. 20, 2020, 10:19 p.m. UTC | #2
On Mon, Jan 20, 2020 at 08:49:03AM +1100, Dave Chinner wrote:
> On Thu, Jan 16, 2020 at 10:23:38PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Convert _xfs_buf_alloc() to return numeric error codes like most
> > everywhere else in xfs.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > ---
> ....
> > @@ -715,8 +718,8 @@ xfs_buf_get_map(
> >  		return NULL;
> >  	}
> >  
> > -	new_bp = _xfs_buf_alloc(target, map, nmaps, flags);
> > -	if (unlikely(!new_bp))
> > +	error = _xfs_buf_alloc(target, map, nmaps, flags, &new_bp);
> > +	if (unlikely(error))
> >  		return NULL;
> 
> You can kill the unlikely() while you are at it. They are largely
> unnecessary as the compiler already considers branches with returns
> in them as unlikely and we don't need "unlikely" as code
> documentation for error conditions like this...
> 
> >  	error = xfs_buf_allocate_memory(new_bp, flags);
> > @@ -917,8 +920,8 @@ xfs_buf_get_uncached(
> >  	DEFINE_SINGLE_BUF_MAP(map, XFS_BUF_DADDR_NULL, numblks);
> >  
> >  	/* flags might contain irrelevant bits, pass only what we care about */
> > -	bp = _xfs_buf_alloc(target, &map, 1, flags & XBF_NO_IOACCT);
> > -	if (unlikely(bp == NULL))
> > +	error = _xfs_buf_alloc(target, &map, 1, flags & XBF_NO_IOACCT, &bp);
> > +	if (unlikely(error))
> >  		goto fail;
> 
> here too.

Will fix.

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
diff mbox series

Patch

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index a0229c368e78..a00e63d08a3b 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -198,20 +198,22 @@  xfs_buf_free_maps(
 	}
 }
 
-static struct xfs_buf *
+static int
 _xfs_buf_alloc(
 	struct xfs_buftarg	*target,
 	struct xfs_buf_map	*map,
 	int			nmaps,
-	xfs_buf_flags_t		flags)
+	xfs_buf_flags_t		flags,
+	struct xfs_buf		**bpp)
 {
 	struct xfs_buf		*bp;
 	int			error;
 	int			i;
 
+	*bpp = NULL;
 	bp = kmem_zone_zalloc(xfs_buf_zone, KM_NOFS);
 	if (unlikely(!bp))
-		return NULL;
+		return -ENOMEM;
 
 	/*
 	 * We don't want certain flags to appear in b_flags unless they are
@@ -239,7 +241,7 @@  _xfs_buf_alloc(
 	error = xfs_buf_get_maps(bp, nmaps);
 	if (error)  {
 		kmem_cache_free(xfs_buf_zone, bp);
-		return NULL;
+		return error;
 	}
 
 	bp->b_bn = map[0].bm_bn;
@@ -256,7 +258,8 @@  _xfs_buf_alloc(
 	XFS_STATS_INC(bp->b_mount, xb_create);
 	trace_xfs_buf_init(bp, _RET_IP_);
 
-	return bp;
+	*bpp = bp;
+	return 0;
 }
 
 /*
@@ -715,8 +718,8 @@  xfs_buf_get_map(
 		return NULL;
 	}
 
-	new_bp = _xfs_buf_alloc(target, map, nmaps, flags);
-	if (unlikely(!new_bp))
+	error = _xfs_buf_alloc(target, map, nmaps, flags, &new_bp);
+	if (unlikely(error))
 		return NULL;
 
 	error = xfs_buf_allocate_memory(new_bp, flags);
@@ -917,8 +920,8 @@  xfs_buf_get_uncached(
 	DEFINE_SINGLE_BUF_MAP(map, XFS_BUF_DADDR_NULL, numblks);
 
 	/* flags might contain irrelevant bits, pass only what we care about */
-	bp = _xfs_buf_alloc(target, &map, 1, flags & XBF_NO_IOACCT);
-	if (unlikely(bp == NULL))
+	error = _xfs_buf_alloc(target, &map, 1, flags & XBF_NO_IOACCT, &bp);
+	if (unlikely(error))
 		goto fail;
 
 	page_count = PAGE_ALIGN(numblks << BBSHIFT) >> PAGE_SHIFT;