Re: [PATCH v3 5/6] xfs: spilt xfs_dialloc() into 2 functions

Gao Xiang
2020-12-07T14:34:43Z

Hi Christoph,

On Mon, Dec 07, 2020 at 02:56:42PM +0100, Christoph Hellwig wrote:
> > if (pag->pagi_freecount) {
> > xfs_perag_put(pag);
> > + *IO_agbp = agbp;
> > + return 0;
>
> I think assigning *IO_agbp would benefit from a little consolidation.
> Set it to NULL in the normal unsuccessful return, and add a found_ag
> label that assigns agbp and returns 0.

Just to confirm the main idea, I think it might be:

*IO_agbp = NULL; at first,

and combine all such assignment
> > + *IO_agbp = agbp;
> > + return 0;
>

into a new found_ag lebel, and use goto found_ag; for such cases.
Do I understand correctly? If that is correct, will update
in the next version.

Thanks,
Gao Xiang

>
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>