Re: [PATCH v1 06/26] x86/virt/tdx: Add tdx_page_array helpers for new TDX Module objects

dan.j.williams
2025-11-19T16:20:25Z

Dave Hansen wrote:
> On 11/16/25 18:22, Xu Yilun wrote:
> ...> + struct tdx_page_array *array __free(kfree) = kzalloc(sizeof(*array),
> > + GFP_KERNEL);
> > + if (!array)
> > + return NULL;
>
> I just reworked this to use normal goto's. It looks a billion times
> better. Please remove all these __free()'s unless you have specific
> evidence that they make the code better.
>
> Maybe I'm old fashioned, but I don't see anything wrong with:
>
> static struct tdx_pglist *tdx_pglist_alloc(unsigned int nr_pages)
> {
> struct tdx_page_array *array = NULL;
> struct page **pages = NULL;
> struct page *root = NULL;
> int ret;
>
> if (!nr_pages)
> return NULL;
>
> array = kzalloc(sizeof(*array), GFP_KERNEL);
> if (!array)
> goto out_free;
>
> root = kzalloc(PAGE_SIZE, GFP_KERNEL);
> if (!root)
> goto out_free;

I think this s/alloc_page/kcalloc/ is the bulk of the improvement, that
is a good change I missed. Otherwise, this version of the function is
longer, and I feel the use of gotos makes this function less
maintainable longer term. Future refactors or feature additions need to
be careful.

Please document that tip prefers that goto be used in cases where a
single goto target can be used.