Re: [PATCH v2 4/7] ghes_edac: avoid multiple calls to dmi_walk()

From: Kani, Toshimitsu
Date: Wed Aug 16 2017 - 11:26:16 EST


On Wed, 2017-08-16 at 10:29 +0200, Borislav Petkov wrote:
> On Tue, Aug 15, 2017 at 08:48:16AM -0700, Luck, Tony wrote:
> > Won't the user see all their DIMMs reported for each memory
> > controller
> > under /sys/devices/system/edac/mc/mc*/dimm* ?
> >
> > That sounds confusing.
>
> Right, and adding the locking was really easy. If only people would
> debate less and actually try to do what they're being advised to.
> But not really: if you wanna have something done, you have to do it
> yourself.

Sorry, but I did not agree on allowing concurrent accesses to mci...

Â/* Memory Device - Type 17 of SMBIOS spec */
> Âstruct memdev_dmi_entry {
> @@ -169,14 +173,11 @@ void ghes_edac_report_mem_error(struct ghes
> *ghes, int sev,
> Â enum hw_event_mc_err_type type;
> Â struct edac_raw_error_desc *e;
> Â struct mem_ctl_info *mci;
> - struct ghes_edac_pvt *pvt = NULL;
> + struct ghes_edac_pvt *pvt = ghes_pvt;
> + unsigned long flags;
> Â char *p;
> Â u8 grain_bits;

I believe you now need to protect from a race condition that a single
mci and pvt can be initialized / consumed from multiple threads. This
protection is missing in your patch.

Thanks,
-Toshi