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

Kani, Toshimitsu
2017-08-16T15:26:04Z

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