Re: [PATCH v4 3/6] xfs: move on-disk inode allocation out of xfs_ialloc()

From: Christoph Hellwig <hch@lst.de>
To: Gao Xiang <hsiangkao@redhat.com>
Cc: linux-xfs@vger.kernel.org,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	Dave Chinner <david@fromorbit.com>,
	Christoph Hellwig <hch@lst.de>,
	Eric Sandeen <sandeen@sandeen.net>,
	Dave Chinner <dchinner@redhat.com>
Subject: Re: [PATCH v4 3/6] xfs: move on-disk inode allocation out of xfs_ialloc()
Date: Wed, 9 Dec 2020 08:52:46 +0100	[thread overview]
Message-ID: <20201209075246.GA10645@lst.de> (raw)
In-Reply-To: <20201208122003.3158922-4-hsiangkao@redhat.com>

> +	/* Initialise the newly allocated inode. */
> +	return xfs_init_new_inode(*tpp, dp, ino, mode, nlink, rdev, prid);

IMHO this comment is not overly helpful..

> +	if (IS_ERR(ip)) {
> +		error = PTR_ERR(ip);
> +		ip = NULL;
>  		goto out_trans_cancel;
> +	}

And the calling convention with the ERR_PTR return does not seem to
fit the call chain to well.  But those are minor details, so:

>  STATIC int
>  xfs_qm_qino_alloc(
> -	xfs_mount_t	*mp,
> -	xfs_inode_t	**ip,
> -	uint		flags)
> +	struct xfs_mount	*mp,
> +	struct xfs_inode	**ipp,
> +	unsigned int		flags)
>  {
>  	xfs_trans_t	*tp;
>  	int		error;
>  	bool		need_alloc = true;

Why do you reindent and de-typdefify the arguments, but not the local
variables?

All the stuff below also seems to deal with the fact that the old return
ip by reference calling convention seems to actually work better with
the code base..