From: Ira Weiny <ira.weiny@intel.com>
To: Ira Weiny <ira.weiny@intel.com>,
Dan Williams <dan.j.williams@intel.com>,
Dave Jiang <dave.jiang@intel.com>,
Jonathan Cameron <jonathan.cameron@huawei.com>,
Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>,
Shiju Jose <shiju.jose@huawei.com>
Cc: Dan Carpenter <dan.carpenter@linaro.org>,
Yazen Ghannam <yazen.ghannam@amd.com>,
Davidlohr Bueso <dave@stgolabs.net>,
"Alison Schofield" <alison.schofield@intel.com>,
Vishal Verma <vishal.l.verma@intel.com>,
Ard Biesheuvel <ardb@kernel.org>, <linux-efi@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <linux-cxl@vger.kernel.org>,
Ira Weiny <ira.weiny@intel.com>,
"Rafael J. Wysocki" <rafael@kernel.org>,
Tony Luck <tony.luck@intel.com>, "Borislav Petkov" <bp@alien8.de>
Subject: RE: [PATCH v2 1/3] acpi/ghes: Process CXL Component Events
Date: Mon, 22 Apr 2024 21:16:00 -0700 [thread overview]
Message-ID: <6627360078428_bd77929462@iweiny-mobl.notmuch> (raw)
In-Reply-To: <66272f40420e6_bbee62945d@iweiny-mobl.notmuch>
Ira Weiny wrote:
> Dan Williams wrote:
> > Ira Weiny wrote:
>
[snip]
> >
> > > +static cxl_cper_callback cper_callback;
> > > +static void cxl_cper_cb_fn(struct work_struct *work)
> > > +{
> > > + struct cxl_cper_work_data wd;
> > > +
> > > + while (kfifo_get(&cxl_cper_fifo, &wd))
> > > + cper_callback(wd.event_type, &wd.rec);
> > > +}
> > > +static DECLARE_WORK(cxl_cb_work, cxl_cper_cb_fn);
> > > +struct work_struct *cxl_cper_work = NULL;
> >
> > Initializing global data to NULL is redundant, however this feels like
> > one too many dynamic things registered.
> >
> > cxl_cper_work and cper_callback are dynamic, but from the GHES
> > perspective all it cares about is checking if work is registered and if
> > so put the data in the kfifo and trigger that work func.
> >
> > It need not care about what happens after the work is queued. So, lets
> > just have the CXL driver register its own cxl_cper_work instance and
> > skip defining one locally here. Export cxl_cper_fifo for the driver to
> > optionally reference.
>
> Ok I thought we had decided not to do that. If I recall exporting the
> fifo had some difficulties but it may have been my unfamiliarity with it.
Looking more closely I see that AER does something similar but it uses
constructs which I thought we should avoid. For example all of
ghes_handle_aer is predicated on CONFIG_ACPI_APEI_PCIEAER.[0]
What I have is a bit complicated, with the extra pointer. Keeping the
kfifo within the ghes module is much easier to follow IMO.
Let me know if you really want me to pursue this separation but my late
night gut feeling is this is going to be more trouble than it is worth to
keep the separation amongst the modules.
Ira
[0]
static void ghes_handle_aer(struct acpi_hest_generic_data *gdata)
{
#ifdef CONFIG_ACPI_APEI_PCIEAER
...
#endif
}
next prev parent reply other threads:[~2024-04-23 4:16 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-22 22:25 [PATCH v2 0/3] efi/cxl-cper: Report CXL CPER events through tracing Ira Weiny
2024-04-22 22:25 ` [PATCH v2 1/3] acpi/ghes: Process CXL Component Events Ira Weiny
2024-04-22 23:54 ` Dan Williams
2024-04-23 3:47 ` Ira Weiny
2024-04-23 4:16 ` Ira Weiny [this message]
2024-04-22 22:25 ` [PATCH v2 2/3] cxl/pci: Process CPER events Ira Weiny
2024-04-22 22:25 ` [PATCH v2 3/3] ras/events: Trace CXL CPER events without CXL stack Ira Weiny
2024-04-23 0:16 ` Dan Williams
2024-04-23 4:10 ` Ira Weiny
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=6627360078428_bd77929462@iweiny-mobl.notmuch \
--to=ira.weiny@intel.com \
--cc=Smita.KoralahalliChannabasappa@amd.com \
--cc=alison.schofield@intel.com \
--cc=ardb@kernel.org \
--cc=bp@alien8.de \
--cc=dan.carpenter@linaro.org \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=dave@stgolabs.net \
--cc=jonathan.cameron@huawei.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-efi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=shiju.jose@huawei.com \
--cc=tony.luck@intel.com \
--cc=vishal.l.verma@intel.com \
--cc=yazen.ghannam@amd.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).