On Tue, Nov 24, 2020 at 11:51:29PM +0800, Gao Xiang wrote:
> It's enough to just use return code, and get rid of an argument.
>
> Signed-off-by: Gao Xiang <hsiangkao@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_ialloc.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 45cf7e55f5ee..5c8b0210aad3 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -607,13 +607,14 @@ xfs_inobt_insert_sprec(
>
> /*
> * Allocate new inodes in the allocation group specified by agbp.
> - * Return 0 for success, else error code.
> + * Return 0 for successfully allocating some inodes in this AG;
> + * 1 for skipping to allocating in the next AG;
> + * < 0 for error code.
> */
> STATIC int
> xfs_ialloc_ag_alloc(
> struct xfs_trans *tp,
> - struct xfs_buf *agbp,
> - int *alloc)
> + struct xfs_buf *agbp)
> {
> struct xfs_agi *agi;
> struct xfs_alloc_arg args;
> @@ -795,10 +796,9 @@ xfs_ialloc_ag_alloc(
> allocmask = (1 << (newlen / XFS_INODES_PER_HOLEMASK_BIT)) - 1;
> }
>
> - if (args.fsbno == NULLFSBLOCK) {
> - *alloc = 0;
> - return 0;
> - }
> + if (args.fsbno == NULLFSBLOCK)
> + return 1;
> +
> ASSERT(args.len == args.minlen);
>
> /*
> @@ -903,7 +903,6 @@ xfs_ialloc_ag_alloc(
> */
> xfs_trans_mod_sb(tp, XFS_TRANS_SB_ICOUNT, (long)newlen);
> xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, (long)newlen);
> - *alloc = 1;
> return 0;
> }
>
> @@ -1715,7 +1714,6 @@ xfs_dialloc(
> struct xfs_buf *agbp;
> xfs_agnumber_t agno;
> int error;
> - int ialloced;
> bool noroom = false;
> xfs_agnumber_t start_agno;
> struct xfs_perag *pag;
> @@ -1799,8 +1797,8 @@ xfs_dialloc(
> goto nextag_relse_buffer;
>
>
> - error = xfs_ialloc_ag_alloc(tp, agbp, &ialloced);
> - if (error) {
> + error = xfs_ialloc_ag_alloc(tp, agbp);
> + if (error < 0) {
> xfs_trans_brelse(tp, agbp);
>
> if (error != -ENOSPC)
> @@ -1811,7 +1809,7 @@ xfs_dialloc(
> return 0;
> }
>
> - if (ialloced) {
> + if (!error) {
I wonder if this should be "if (error == 0)" because the comment
for _ialloc_ag_alloc says that 0 and 1 have specific meanings?
Otherwise looks fine to me.
--D
> /*
> * We successfully allocated some inodes, return
> * the current context to the caller so that it
> --
> 2.18.4
>
