Re: [PATCH v2 04/10] EDAC/ghes: Make SMBIOS handle private data to ghes

From: Robert Richter <rrichter@marvell.com>
To: Borislav Petkov <bp@alien8.de>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Tony Luck <tony.luck@intel.com>,
	James Morse <james.morse@arm.com>,
	Aristeu Rozanski <aris@redhat.com>,
	Matthias Brugger <mbrugger@suse.com>,
	<linux-edac@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 04/10] EDAC/ghes: Make SMBIOS handle private data to ghes
Date: Tue, 5 May 2020 14:48:23 +0200	[thread overview]
Message-ID: <20200505124823.j3kmcv4z53epupvx@rric.localdomain> (raw)
In-Reply-To: <20200424162157.GB6916@zn.tnic>

On 24.04.20 18:21:57, Borislav Petkov wrote:
> On Wed, Apr 22, 2020 at 01:58:08PM +0200, Robert Richter wrote:

> > @@ -562,6 +647,7 @@ void ghes_edac_unregister(struct ghes *ghes)
> >  {
> >  	struct mem_ctl_info *mci;
> >  	unsigned long flags;
> > +	LIST_HEAD(dimm_list);
> >  
> >  	mutex_lock(&ghes_reg_mutex);
> >  
> > @@ -574,14 +660,19 @@ void ghes_edac_unregister(struct ghes *ghes)
> >  	spin_lock_irqsave(&ghes_lock, flags);
> >  	mci = ghes_pvt ? ghes_pvt->mci : NULL;
> >  	ghes_pvt = NULL;
> > +	list_splice_init(&ghes_dimm_list, &dimm_list);
> 
> Why do you need to do this?
> 
> Can't you simply do:
> 
> 	 ghes_dimm_release(&ghes_dimm_list);
> 
> here?

This decouples the locking. Otherwise ghes_dimm_release() would be
called with ghes_lock held which I want to avoid to keep the locking
simple.

-Robert