[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250730112549.00003e7d@huawei.com>
Date: Wed, 30 Jul 2025 11:25:49 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Aneesh Kumar K.V <aneesh.kumar@...nel.org>
CC: <linux-coco@...ts.linux.dev>, <kvmarm@...ts.linux.dev>,
<linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>, <aik@....com>,
<lukas@...ner.de>, Samuel Ortiz <sameo@...osinc.com>, Xu Yilun
<yilun.xu@...ux.intel.com>, Jason Gunthorpe <jgg@...pe.ca>, "Suzuki K
Poulose" <Suzuki.Poulose@....com>, Steven Price <steven.price@....com>,
Catalin Marinas <catalin.marinas@....com>, Marc Zyngier <maz@...nel.org>,
Will Deacon <will@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>
Subject: Re: [RFC PATCH v1 12/38] coco: host: arm64: CCA host platform
device driver
On Wed, 30 Jul 2025 14:28:55 +0530
Aneesh Kumar K.V <aneesh.kumar@...nel.org> wrote:
> Jonathan Cameron <Jonathan.Cameron@...wei.com> writes:
>
> > On Mon, 28 Jul 2025 19:21:49 +0530
> > "Aneesh Kumar K.V (Arm)" <aneesh.kumar@...nel.org> wrote:
> >
>
> ...
>
> >> +
> >> +#include "rmm-da.h"
> >> +
> >> +/* Number of streams that we can support at the hostbridge level */
> >> +#define CCA_HB_PLATFORM_STREAMS 4
> >> +
> >> +/* Total number of stream id supported at root port level */
> >> +#define MAX_STREAM_ID 256
> >> +
> >> +DEFINE_FREE(vfree, void *, if (!IS_ERR_OR_NULL(_T)) vfree(_T))
> >> +static struct pci_tsm *cca_tsm_pci_probe(struct pci_dev *pdev)
> >> +{
> >> + int rc;
> >> + struct pci_host_bridge *hb;
> >> + struct cca_host_dsc_pf0 *dsc_pf0 __free(vfree) = NULL;
> >
> > Read the stuff in cleanup.h and work out why this needs
> > changing to be inline below and not use this NULL pattern here
> > (unless you like grumpy Linus ;)
> >
> > Note that with the err_out, even if you do that you'll still be
> > breaking with the guidance doc (and actually causing undefined
> > behavior :) Get rid of those gotos if you want to use __free()
> >
> >
>
> I’ve already fixed up similar cases by removing the goto based on cleanup.h
> docs in other functions.I must have missed this one.
>
> By the way, isn't using the `NULL` pattern acceptable when there are
> no additional lock variables involved (ie, unwind order doesn't matter)?
> Or should we always follow the pattern below regardless?
>
> struct cca_host_dsc_pf0 *dsc_pf0 __free(vfree) =
> vcalloc(sizeof(*dsc_pf0), GFP_KERNEL);
Always do this. It's not really about what happens today but
more what we might break by failing to notice a future patch causes
problems. Keeping the unwind ordering tightly couple with setup
means we basically can't get it wrong (famous last words ;)
Jonathan
>
> -aneesh
Powered by blists - more mailing lists