Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 1 Aug 2023 10:40:38 +0200
From:   Greg KH <gregkh@linuxfoundation.org>
To:     "Gangurde, Abhijit" <abhijit.gangurde@amd.com>
Cc:     "masahiroy@kernel.org" <masahiroy@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Simek, Michal" <michal.simek@amd.com>,
        "git (AMD-Xilinx)" <git@amd.com>,
        "Agarwal, Nikhil" <nikhil.agarwal@amd.com>,
        "Gupta, Nipun" <Nipun.Gupta@amd.com>
Subject: Re: [PATCH v2 1/4] cdx: Introduce lock to protect controller ops and
 controller list
Message-ID: <2023080105-eggnog-probably-9ff3@gregkh>
References: <20230731120813.123247-1-abhijit.gangurde@amd.com>
 <20230731120813.123247-2-abhijit.gangurde@amd.com>
 <2023073148-carried-unshaved-77e4@gregkh>
 <DM4PR12MB7765398763E00B565AB1C6DB8F0AA@DM4PR12MB7765.namprd12.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <DM4PR12MB7765398763E00B565AB1C6DB8F0AA@DM4PR12MB7765.namprd12.prod.outlook.com>
Precedence: bulk

On Tue, Aug 01, 2023 at 08:34:00AM +0000, Gangurde, Abhijit wrote:
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: Monday, July 31, 2023 5:55 PM
> > To: Gangurde, Abhijit <abhijit.gangurde@amd.com>
> > Cc: masahiroy@kernel.org; linux-kernel@vger.kernel.org; Simek, Michal
> > <michal.simek@amd.com>; git (AMD-Xilinx) <git@amd.com>; Agarwal, Nikhil
> > <nikhil.agarwal@amd.com>; Gupta, Nipun <Nipun.Gupta@amd.com>
> > Subject: Re: [PATCH v2 1/4] cdx: Introduce lock to protect controller ops and
> > controller list
> > 
> > On Mon, Jul 31, 2023 at 05:38:10PM +0530, Abhijit Gangurde wrote:
> > > Add a mutex lock to prevent race between controller ops initiated by
> > > the bus subsystem and the controller registration/unregistration.
> > >
> > > Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
> > > ---
> > >  drivers/cdx/cdx.c | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
> > > index d2cad4c670a0..66797c8fe400 100644
> > > --- a/drivers/cdx/cdx.c
> > > +++ b/drivers/cdx/cdx.c
> > > @@ -72,6 +72,8 @@
> > >
> > >  /* CDX controllers registered with the CDX bus */
> > >  static DEFINE_XARRAY_ALLOC(cdx_controllers);
> > > +/* Lock to protect controller ops and controller list */
> > > +static DEFINE_MUTEX(cdx_controller_lock);
> > 
> > Wait, why do you have a local list and not just rely on the list the
> > driver core has for you already?  Isn't this a duplicate list where you
> > have objects on two different lists with a lifespan controlled only by
> > one of them?
> 
> cdx_controllers list is holding just the controllers registered on the cdx bus system.

Which are devices on the bus, so why do you need a separate list?

> CDX devices are still maintained by driver core list. Controller list is used by rescan
> which triggers rescan on all the controllers.

Again, why a separate list?  The driver core already tracks these,
right?

thanks,

greg k-h