Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 16 Mar 2020 10:40:56 +0100
From:   Borislav Petkov <bp@alien8.de>
To:     Robert Richter <rrichter@marvell.com>
Cc:     Mauro Carvalho Chehab <mchehab@kernel.org>,
        Tony Luck <tony.luck@intel.com>,
        James Morse <james.morse@arm.com>,
        Aristeu Rozanski <aris@redhat.com>, linux-edac@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 09/11] EDAC/ghes: Implement DIMM mapping table for SMBIOS
 handles
Message-ID: <20200316094056.GC26126@zn.tnic>
References: <20200306151318.17422-1-rrichter@marvell.com>
 <20200306151318.17422-10-rrichter@marvell.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20200306151318.17422-10-rrichter@marvell.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Mar 06, 2020 at 04:13:16PM +0100, Robert Richter wrote:
> GHES uses SMBIOS handles to identify the DIMM that was causing a hw
> error. Currently this is stored in smbios_handle of struct dimm_info.
> This implementation has several drawbacks. The information is private
> to the ghes driver, but struct dimm_info is for general use. The DIMMs
> are tied to a *mci struct, which makes a lockup inefficient. It is

"lookup"

> hard to dynamically allocate DIMMs and also to meet locking
> constraints when adding or removing them. This becomes even more
> challenging when having multiple MC instances that group a set of
> DIMMs, so the change is needed also to later support multiple MC
> instances.

Err, I don't understand: normally a bunch of DIMMs belong to a memory
controller and that gives you the hierarchy automatically. Why is that a
problem?

And normally you allocate the DIMM representations on MC init and
deallocate them when removing the MC.

So what is the problem here?

...

...
> @@ -72,6 +79,52 @@ struct memdev_dmi_entry {
>  	u16 conf_mem_clk_speed;
>  } __attribute__((__packed__));
>  
> +/* ghes_reg_mutex must be held. */

lockdep_assert_held()

...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette