All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kani, Toshimitsu" <toshi.kani@hpe.com>
To: "bp@alien8.de" <bp@alien8.de>
Cc: "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	"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-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
	"rjw@rjwysocki.net" <rjw@rjwysocki.net>
Subject: Re: [PATCH v2 4/7] ghes_edac: avoid multiple calls to dmi_walk()
Date: Mon, 7 Aug 2017 17:59:15 +0000	[thread overview]
Message-ID: <1502128177.2042.131.camel@hpe.com> (raw)
In-Reply-To: <20170805051615.GC23214@nazgul.tnic>

On Sat, 2017-08-05 at 07:16 +0200, Borislav Petkov wrote:
> On Fri, Aug 04, 2017 at 09:02:17PM +0000, Kani, Toshimitsu wrote:
> > GHES platform devices correspond to GHES entries, which define
> > firmware interfaces to report generic memory errors to the OS, such
> > as NMI and SCI.  These devices are associated with all DIMMs, not a
> > particular DIMM.
> 
> And? Stating the obvious brings you what exactly?
> 
> IOW, you still haven't answered my question.

Sorry about that.  Let me copy your original comments to make sure I
answer your questions this time.

> So the problem is that ghes_edac_register() gets called multiple
> times depending on how many GHES platform devices are on the system.
> But yet they all scan *all* DIMMs. 

Right, and this patch changes ghes_edac_register() to scan all DIMMs at
the first time and do not scan them next times.

> So instead you should return if
> the DIMMs have been counted already and not register a second time.
> Which makes that whole mc counting kinda useless. So you could rip
>
that out too.

Oh, I see.  So, ghes_edac_register() should return no-op a second time,
and does not call edac_mc_add_mc() to register with a separate mci.

I think we should keep the current scheme, which registers an mci for
each GHES entry.  ghes_edac_report_mem_error() expects that error-
reporting is serialized per a GHES entry.  Sharing a single mci among
all GHES entries / error interfaces might lead to a race condition.

Thanks,
-Toshi


WARNING: multiple messages have this Message-ID (diff)
From: Toshi Kani <toshi.kani@hpe.com>
To: "bp@alien8.de" <bp@alien8.de>
Cc: "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
	"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-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
	"rjw@rjwysocki.net" <rjw@rjwysocki.net>
Subject: [v2,4/7] ghes_edac: avoid multiple calls to dmi_walk()
Date: Mon, 7 Aug 2017 17:59:15 +0000	[thread overview]
Message-ID: <1502128177.2042.131.camel@hpe.com> (raw)

On Sat, 2017-08-05 at 07:16 +0200, Borislav Petkov wrote:
> On Fri, Aug 04, 2017 at 09:02:17PM +0000, Kani, Toshimitsu wrote:
> > GHES platform devices correspond to GHES entries, which define
> > firmware interfaces to report generic memory errors to the OS, such
> > as NMI and SCI.  These devices are associated with all DIMMs, not a
> > particular DIMM.
> 
> And? Stating the obvious brings you what exactly?
> 
> IOW, you still haven't answered my question.

Sorry about that.  Let me copy your original comments to make sure I
answer your questions this time.

> So the problem is that ghes_edac_register() gets called multiple
> times depending on how many GHES platform devices are on the system.
> But yet they all scan *all* DIMMs. 

Right, and this patch changes ghes_edac_register() to scan all DIMMs at
the first time and do not scan them next times.

> So instead you should return if
> the DIMMs have been counted already and not register a second time.
> Which makes that whole mc counting kinda useless. So you could rip
>
that out too.

Oh, I see.  So, ghes_edac_register() should return no-op a second time,
and does not call edac_mc_add_mc() to register with a separate mci.

I think we should keep the current scheme, which registers an mci for
each GHES entry.  ghes_edac_report_mem_error() expects that error-
reporting is serialized per a GHES entry.  Sharing a single mci among
all GHES entries / error interfaces might lead to a race condition.

Thanks,
-Toshi

  reply	other threads:[~2017-08-07 17:59 UTC|newest]

Thread overview: 130+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-03 21:57 [PATCH v2 0/7] enable ghes_edac on selected platforms Toshi Kani
2017-08-03 21:57 ` [PATCH v2 1/7] ACPI / blacklist: add acpi_match_oemlist() interface Toshi Kani
2017-08-03 21:57   ` [v2,1/7] " Toshi Kani
2017-08-04  3:42   ` [PATCH v2 1/7] " Borislav Petkov
2017-08-04  3:42     ` [v2,1/7] " Borislav Petkov
2017-08-04 20:39     ` [PATCH v2 1/7] " Kani, Toshimitsu
2017-08-04 20:39       ` [v2,1/7] " Toshi Kani
2017-08-05  5:12       ` [PATCH v2 1/7] " Borislav Petkov
2017-08-05  5:12         ` [v2,1/7] " Borislav Petkov
2017-08-07 14:49         ` [PATCH v2 1/7] " Kani, Toshimitsu
2017-08-07 14:49           ` [v2,1/7] " Toshi Kani
2017-08-03 21:57 ` [PATCH v2 2/7] intel_pstate: convert to use acpi_match_oemlist() Toshi Kani
2017-08-03 21:57   ` [v2,2/7] " Toshi Kani
2017-08-03 21:57 ` [PATCH v2 3/7] ACPI / APEI: add OSC APEI bit check for ghes_edac Toshi Kani
2017-08-03 21:57   ` [v2,3/7] " Toshi Kani
2017-08-04  3:44   ` [PATCH v2 3/7] " Borislav Petkov
2017-08-04  3:44     ` [v2,3/7] " Borislav Petkov
2017-08-04 20:49     ` [PATCH v2 3/7] " Kani, Toshimitsu
2017-08-04 20:49       ` [v2,3/7] " Toshi Kani
2017-08-05  5:14       ` [PATCH v2 3/7] " Borislav Petkov
2017-08-05  5:14         ` [v2,3/7] " Borislav Petkov
2017-08-07 14:50         ` [PATCH v2 3/7] " Kani, Toshimitsu
2017-08-07 14:50           ` [v2,3/7] " Toshi Kani
2017-08-03 21:57 ` [PATCH v2 4/7] ghes_edac: avoid multiple calls to dmi_walk() Toshi Kani
2017-08-03 21:57   ` [v2,4/7] " Toshi Kani
2017-08-04  4:05   ` [PATCH v2 4/7] " Borislav Petkov
2017-08-04  4:05     ` [v2,4/7] " Borislav Petkov
2017-08-04 21:02     ` [PATCH v2 4/7] " Kani, Toshimitsu
2017-08-04 21:02       ` [v2,4/7] " Toshi Kani
2017-08-05  5:16       ` [PATCH v2 4/7] " Borislav Petkov
2017-08-05  5:16         ` [v2,4/7] " Borislav Petkov
2017-08-07 17:59         ` Kani, Toshimitsu [this message]
2017-08-07 17:59           ` Toshi Kani
2017-08-11  9:04           ` [PATCH v2 4/7] " Borislav Petkov
2017-08-11  9:04             ` [v2,4/7] " Borislav Petkov
2017-08-14 15:57             ` [PATCH v2 4/7] " Kani, Toshimitsu
2017-08-14 15:57               ` [v2,4/7] " Toshi Kani
2017-08-14 16:24               ` [PATCH v2 4/7] " Borislav Petkov
2017-08-14 16:24                 ` [v2,4/7] " Borislav Petkov
2017-08-14 16:48                 ` [PATCH v2 4/7] " Kani, Toshimitsu
2017-08-14 16:48                   ` [v2,4/7] " Toshi Kani
2017-08-14 17:05                   ` [PATCH v2 4/7] " Borislav Petkov
2017-08-14 17:05                     ` [v2,4/7] " Borislav Petkov
2017-08-14 17:52                     ` [PATCH v2 4/7] " Kani, Toshimitsu
2017-08-14 17:52                       ` [v2,4/7] " Toshi Kani
2017-08-14 18:05                       ` [PATCH v2 4/7] " Borislav Petkov
2017-08-14 18:05                         ` [v2,4/7] " Borislav Petkov
2017-08-14 18:17                         ` [PATCH v2 4/7] " Kani, Toshimitsu
2017-08-14 18:17                           ` [v2,4/7] " Toshi Kani
2017-08-14 18:35                           ` [PATCH v2 4/7] " Borislav Petkov
2017-08-14 18:35                             ` [v2,4/7] " Borislav Petkov
2017-08-14 19:02                             ` [PATCH v2 4/7] " Kani, Toshimitsu
2017-08-14 19:02                               ` [v2,4/7] " Toshi Kani
2017-08-14 19:34                               ` [PATCH v2 4/7] " Borislav Petkov
2017-08-14 19:34                                 ` [v2,4/7] " Borislav Petkov
2017-08-14 20:17                                 ` [PATCH v2 4/7] " Kani, Toshimitsu
2017-08-14 20:17                                   ` [v2,4/7] " Toshi Kani
2017-08-14 20:39                                   ` [PATCH v2 4/7] " Borislav Petkov
2017-08-14 20:39                                     ` [v2,4/7] " Borislav Petkov
2017-08-15 15:35                                     ` [PATCH v2 4/7] " Kani, Toshimitsu
2017-08-15 15:35                                       ` [v2,4/7] " Toshi Kani
2017-08-15 15:48                                       ` [PATCH v2 4/7] " Luck, Tony
2017-08-15 15:48                                         ` [v2,4/7] " Luck, Tony
2017-08-15 15:53                                         ` [PATCH v2 4/7] " Kani, Toshimitsu
2017-08-15 15:53                                           ` [v2,4/7] " Toshi Kani
2017-08-16  8:29                                         ` [PATCH v2 4/7] " Borislav Petkov
2017-08-16  8:29                                           ` [v2,4/7] " Borislav Petkov
2017-08-16 11:29                                           ` [PATCH v2 4/7] " Borislav Petkov
2017-08-16 11:29                                             ` [v2,4/7] " Borislav Petkov
2017-08-16 13:59                                           ` [PATCH v2 4/7] " Steven Rostedt
2017-08-16 13:59                                             ` [v2,4/7] " Steven Rostedt
2017-08-16 14:03                                             ` [PATCH v2 4/7] " Borislav Petkov
2017-08-16 14:03                                               ` [v2,4/7] " Borislav Petkov
2017-08-16 14:22                                               ` [PATCH v2 4/7] " Steven Rostedt
2017-08-16 14:22                                                 ` [v2,4/7] " Steven Rostedt
2017-08-16 17:31                                                 ` [PATCH v2 4/7] " Borislav Petkov
2017-08-16 17:31                                                   ` [v2,4/7] " Borislav Petkov
2017-08-16 15:26                                           ` [PATCH v2 4/7] " Kani, Toshimitsu
2017-08-16 15:26                                             ` [v2,4/7] " Toshi Kani
2017-08-16 16:42                                             ` [PATCH v2 4/7] " Borislav Petkov
2017-08-16 16:42                                               ` [v2,4/7] " Borislav Petkov
2017-08-16 17:28                                               ` [PATCH v2 4/7] " Kani, Toshimitsu
2017-08-16 17:28                                                 ` [v2,4/7] " Toshi Kani
2017-08-16 17:40                                                 ` [PATCH v2 4/7] " Borislav Petkov
2017-08-16 17:40                                                   ` [v2,4/7] " Borislav Petkov
2017-08-16 18:01                                                   ` [PATCH v2 4/7] " Kani, Toshimitsu
2017-08-16 18:01                                                     ` [v2,4/7] " Toshi Kani
2017-08-17 21:08                                                     ` [PATCH v2 4/7] " Kani, Toshimitsu
2017-08-17 21:08                                                       ` [v2,4/7] " Toshi Kani
2017-08-21  9:29                                                       ` [PATCH v2 4/7] " Borislav Petkov
2017-08-21  9:29                                                         ` [v2,4/7] " Borislav Petkov
2017-08-15 15:50                                       ` [PATCH v2 4/7] " Borislav Petkov
2017-08-15 15:50                                         ` [v2,4/7] " Borislav Petkov
2017-08-15 16:19                                         ` [PATCH v2 4/7] " Kani, Toshimitsu
2017-08-15 16:19                                           ` [v2,4/7] " Toshi Kani
2017-08-03 21:57 ` [PATCH v2 5/7] ghes_edac: add platform check to enable ghes_edac Toshi Kani
2017-08-03 21:57   ` [v2,5/7] " Toshi Kani
2017-08-04  8:31   ` [PATCH v2 5/7] " Borislav Petkov
2017-08-04  8:31     ` [v2,5/7] " Borislav Petkov
2017-08-04 21:06     ` [PATCH v2 5/7] " Kani, Toshimitsu
2017-08-04 21:06       ` [v2,5/7] " Toshi Kani
2017-08-05  5:37       ` [PATCH v2 5/7] " Borislav Petkov
2017-08-05  5:37         ` [v2,5/7] " Borislav Petkov
2017-08-07 14:54         ` [PATCH v2 5/7] " Kani, Toshimitsu
2017-08-07 14:54           ` [v2,5/7] " Toshi Kani
2017-08-03 21:57 ` [PATCH v2 6/7] EDAC: add edac_check_mc_owner() to check MC owner Toshi Kani
2017-08-03 21:57   ` [v2,6/7] " Toshi Kani
2017-08-04  8:30   ` [PATCH v2 6/7] " Borislav Petkov
2017-08-04  8:30     ` [v2,6/7] " Borislav Petkov
2017-08-04 21:35     ` [PATCH v2 6/7] " Kani, Toshimitsu
2017-08-04 21:35       ` [v2,6/7] " Toshi Kani
2017-08-05  5:44       ` [PATCH v2 6/7] " Borislav Petkov
2017-08-05  5:44         ` [v2,6/7] " Borislav Petkov
2017-08-07 14:55         ` [PATCH v2 6/7] " Kani, Toshimitsu
2017-08-07 14:55           ` [v2,6/7] " Toshi Kani
2017-08-04 13:06   ` [PATCH v2 6/7] " kbuild test robot
2017-08-04 13:06     ` kbuild test robot
2017-08-04 13:06     ` [v2,6/7] " kbuild test robot
2017-08-04 15:21     ` [PATCH v2 6/7] " Kani, Toshimitsu
2017-08-04 15:21       ` [v2,6/7] " Toshi Kani
2017-08-03 21:57 ` [PATCH v2 7/7] edac drivers: add MC owner check in init Toshi Kani
2017-08-03 21:57   ` [v2,7/7] " Toshi Kani
2017-08-04  8:39   ` [PATCH v2 7/7] " Borislav Petkov
2017-08-04  8:39     ` [v2,7/7] " Borislav Petkov
2017-08-04 21:48     ` [PATCH v2 7/7] " Kani, Toshimitsu
2017-08-04 21:48       ` [v2,7/7] " Toshi Kani
2017-08-05  5:49       ` [PATCH v2 7/7] " Borislav Petkov
2017-08-05  5:49         ` [v2,7/7] " Borislav Petkov
2017-08-07 14:57         ` [PATCH v2 7/7] " Kani, Toshimitsu
2017-08-07 14:57           ` [v2,7/7] " Toshi Kani

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1502128177.2042.131.camel@hpe.com \
    --to=toshi.kani@hpe.com \
    --cc=bp@alien8.de \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=tony.luck@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.