From: "Kani, Toshimitsu" <toshi.kani@hpe.com>
To: "bp@alien8.de" <bp@alien8.de>
CC: "rjw@rjwysocki.net" <rjw@rjwysocki.net>,
        "lenb@kernel.org" <lenb@kernel.org>,
        "mchehab@kernel.org" <mchehab@kernel.org>,
        "tony.luck@intel.com" <tony.luck@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
        "linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>
Subject: Re: [PATCH v2 4/7] ghes_edac: avoid multiple calls to dmi_walk()
Thread-Topic: [PATCH v2 4/7] ghes_edac: avoid multiple calls to dmi_walk()
Thread-Index: AQHTDKTuWyxK/OMe00e0Ovda8CeGBqJzlIKAgAEZcQCAAIyugIAD9yeAgAW2iQCABSfDgIAACkUAgAAEFwCAAAdxAIAACkmAgAAGWwCAAAC7gIAAB8WAgAAEpwCAAAu+AIAACWYAgAAIzwCAATq3AA==
Date: Tue, 15 Aug 2017 15:35:51 +0000
Message-ID: <1502810766.2042.149.camel@hpe.com>
References: <20170814162436.lta5xm742hh6g7yk@pd.tnic>
         <1502728754.2042.139.camel@hpe.com>
         <20170814170552.j3lhzvnwlpz75y4g@pd.tnic>
         <1502732561.2042.141.camel@hpe.com>
         <20170814180526.wtfjzgc6uye2qtx6@pd.tnic>
         <1502734083.2042.143.camel@hpe.com>
         <20170814183551.sgk2i7lxpmpyodhv@pd.tnic>
         <1502736750.2042.145.camel@hpe.com>
         <20170814193432.mjldfhfal5ba5dt7@pd.tnic>
         <1502741290.2042.147.camel@hpe.com>
         <20170814203942.6t3mrq3hc324blab@pd.tnic>
In-Reply-To: <20170814203942.6t3mrq3hc324blab@pd.tnic>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
Content-ID: <5CECFBCC3EA51C47B23091B714FA3A4F@NAMPRD84.PROD.OUTLOOK.COM>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Aug 2017 15:35:51.3048
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DF4PR84MB0185
X-OriginatorOrg: hpe.com
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from base64 to 8bit by nfs id v7FFcUTu024962
Content-Length: 1802
Lines: 44

On Mon, 2017-08-14 at 22:39 +0200, Borislav Petkov wrote:
> On Mon, Aug 14, 2017 at 08:17:54PM +0000, Kani, Toshimitsu wrote:
> > I think the current code design of allocating mci & ghes_edac_pvt
> > for each GHES source entry makes sense.
> 
> And I don't.
> 
> > edac_raw_mc_handle_error() also has the same expectation that the
> > call is serialized per mci.
> 
> There's no such thing as "per mci" if the driver scans *all DIMMs*
> per register call. If it does it this way, then it is only one mci.

ghes_edac instantiates an mci as a pseudo device representing a GHES
error source.  Each error source associates with all DIMMs, and may
report errors independently.  As ghes_edac is an GHES error-reporting
wrapper to edac, this abstraction makes sense.

> It is actually wrong right now because if you register more than one
> mci and you do edac_inc_ce_error()/edac_inc_ue_error(), potentially
> different counters get incremented for the same errors. Exactly
> because each instance registered is *wrongly* responsible for all
> DIMMs on the system.

I do not see a problem in having counters for each GHES error source. 
This is just statistics info, and ghes_edac does not expect any OS
action from the counters.

> So you either need to partition the DIMMs per mci (which I can't
> imagine how it would work) or introduce locking when incrementing the
> mci->counters.

I do not think changing the calling convention to edac library
interfaces is a good idea for a special case like ghes_edac.  Such
changes can be a burden for us going forward.  I think ghes_edac just
needs to work with the current prerequisite.

User apps like ras-mc-ctl works as expected for a given (not-so-great)
DIMM info from SMBIOS as well.  I do not see a probelm from user
perspective, either.

Thanks,
-Toshi