| Message ID | 157924221800.3029431.576491686056157423.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: make buffer functions return error codes | expand |
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.
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 --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;