From: "Kani, Toshimitsu" <toshi.kani@hpe.com>
To: "bp@alien8.de" <bp@alien8.de>,
        "linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>
CC: "mchehab@infradead.org" <mchehab@infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "tony.luck@intel.com" <tony.luck@intel.com>,
        "rjw@rjwysocki.net" <rjw@rjwysocki.net>
Subject: Re: [PATCH 3/3] EDAC, ghes: Make it a proper module
Thread-Topic: [PATCH 3/3] EDAC, ghes: Make it a proper module
Thread-Index: AQHTBewsoOaHQfQgAk6o+lOz407qJKJplmgA
Date: Fri, 28 Jul 2017 18:50:56 +0000
Message-ID: <1501267290.2042.89.camel@hpe.com>
References: <20170726084827.11447-1-bp@alien8.de>
         <20170726084827.11447-4-bp@alien8.de>
In-Reply-To: <20170726084827.11447-4-bp@alien8.de>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
Content-ID: <D965EC1366D4714380C2389A1D17E38F@NAMPRD84.PROD.OUTLOOK.COM>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Jul 2017 18:50:56.0373
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DF4PR84MB0187
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 v6SIq0q8032158
Content-Length: 1736
Lines: 62

On Wed, 2017-07-26 at 10:48 +0200, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
> 
> Register with the GHES notifier chain so that there's no need to call
> into the module with ghes_edac_report_mem_error().
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
 :
> +static int report_mem_error(struct notifier_block *nb, unsigned long
> sev, void *data)
>  {
> +	struct cper_sec_mem_err *mem_err = data;
>  	enum hw_event_mc_err_type type;
>  	struct edac_raw_error_desc *e;
>  	struct mem_ctl_info *mci;
> -	struct ghes_edac_pvt *pvt = NULL;
> -	char *p;
> +	struct ghes_edac_pvt *pvt = ghes_pvt;
>  	u8 grain_bits;
> +	char *p;
>  
> -	list_for_each_entry(pvt, &ghes_reglist, list) {
> -		if (ghes == pvt->ghes)
> -			break;
> -	}
>  	if (!pvt) {

I think it always hits this error condition.  See below.

> -		pr_err("Internal error: Can't find EDAC
> structure\n");
> -		return;
> +		edac_pr_err("Internal error: Can't find EDAC
> structure\n");
> +		return NOTIFY_DONE;
>  	}
> +
 :
> +static int __init ghes_edac_register(void)
>  {
> +	struct ghes_edac_pvt *pvt = ghes_pvt;

This simply sets NULL to pvt, and does not initialize ghes_pvt.

>  	bool fake = false;
>  	int rc, num_dimm = 0;
>  	struct mem_ctl_info *mci;
>  	struct edac_mc_layer layers[1];
> -	struct ghes_edac_pvt *pvt;
>  	struct ghes_edac_dimm_fill dimm_fill;
>  
 :
> -EXPORT_SYMBOL_GPL(ghes_edac_register);
> +module_init(ghes_edac_register);

Since this patch has removed the GHES-presence check and ordering hack
for ghes_edac, it now registers itself per the module_init() ordering
regardless of the presence of GHES.  As Mauro pointed out, some type of
GHES check needs to be in place before making this change.

Thanks,
-Toshi