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

[<dan.j.williams@intel.com>]

Dave Hansen wrote:
> On 11/19/25 08:20, dan.j.williams@intel.com wrote:
> > Please document that tip prefers that goto be used in cases where a
> > single goto target can be used.
> 
> I don't think it's that hard and fast of a rule. It's really just a more
> general: use the best tool for the job. For instance, if your line of
> code was already on the long side, the additional __free(foo) might be
> the straw that breaks the camel's back.
> 
> So it all depends.
> 
> In this case, we have very linear code flow, very long lines, and quite
> uniform allocator use (they can all be kfree()). That tips the scales
> squarely in the direction of goto.

In the absence of near term future use cases that will violate the
uniform allocator observation, yes, today's needs can rely on a single
error exit path.

For clarity for Yilun, be circumspect on scope-based-cleanup usage for
arch/x86/virt/, but for TSM driver bits in drivers/virt/, do feel free
to continue aggressive avoidance of goto.

> But there's already code that takes that allocation order into account.
> It's also going to be in this patch whether or not __free() is utilized:
> 
> > +void tdx_page_array_free(struct tdx_page_array *array)
> > +{
> > +	if (!array)
> > +		return;
> > +
> > +	__free_page(array->root);
> > +	tdx_free_pages_bulk(array->nr_pages, array->pages);
> > +	kfree(array->pages);
> > +	kfree(array);
> > +}
> Ideally, we'd be able to define the free ordering requirements in a
> single bit of code and use it in both paths.

__free() does not effect object destructors like this, only the
constructor early error exits. It would be nice to have uniformity, but
that is more something that devres offers rather than
scope-based-cleanup. Devres is not appropriate for this path.

So yes, some mental load needs to be spent on validating the order of
the destructor path.