Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1581412 > unrolled thread
| Started by | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| First post | 2017-02-15 17:00 +0100 |
| Last post | 2017-02-15 18:10 +0100 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH V8 08/10] ras: acpi / apei: generate trace event for unrecognized CPER section Steven Rostedt <rostedt@goodmis.org> - 2017-02-15 17:00 +0100
Re: [PATCH V8 08/10] ras: acpi / apei: generate trace event for unrecognized CPER section "Baicar, Tyler" <tbaicar@codeaurora.org> - 2017-02-15 18:00 +0100
Re: [PATCH V8 08/10] ras: acpi / apei: generate trace event for unrecognized CPER section "Baicar, Tyler" <tbaicar@codeaurora.org> - 2017-02-15 18:10 +0100
Re: [PATCH V8 08/10] ras: acpi / apei: generate trace event for unrecognized CPER section Steven Rostedt <rostedt@goodmis.org> - 2017-02-15 18:10 +0100
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-02-15 17:00 +0100 |
| Subject | Re: [PATCH V8 08/10] ras: acpi / apei: generate trace event for unrecognized CPER section |
| Message-ID | <tba9Q-1T0-7@gated-at.bofh.it> |
On Wed, 1 Feb 2017 10:16:51 -0700
Tyler Baicar <tbaicar@codeaurora.org> wrote:
> @@ -452,11 +454,21 @@ static void ghes_do_proc(struct ghes *ghes,
> {
> int sev, sec_sev;
> struct acpi_hest_generic_data *gdata;
> + uuid_le sec_type;
> + uuid_le *fru_id = &NULL_UUID_LE;
> + char *fru_text = "";
>
> sev = ghes_severity(estatus->error_severity);
> apei_estatus_for_each_section(estatus, gdata) {
> sec_sev = ghes_severity(gdata->error_severity);
> - if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
> + sec_type = *(uuid_le *)gdata->section_type;
> +
> + if (gdata->validation_bits & CPER_SEC_VALID_FRU_ID)
> + fru_id = (uuid_le *)gdata->fru_id;
> + if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)
> + fru_text = gdata->fru_text;
> +
> + if (!uuid_le_cmp(sec_type,
> CPER_SEC_PLATFORM_MEM)) {
> struct cper_sec_mem_err *mem_err;
>
> @@ -467,7 +479,7 @@ static void ghes_do_proc(struct ghes *ghes,
> ghes_handle_memory_failure(gdata, sev);
> }
> #ifdef CONFIG_ACPI_APEI_PCIEAER
> - else if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
> + else if (!uuid_le_cmp(sec_type,
> CPER_SEC_PCIE)) {
> struct cper_sec_pcie *pcie_err;
>
> @@ -500,6 +512,12 @@ static void ghes_do_proc(struct ghes *ghes,
>
> }
> #endif
> + else {
As an optimization, you can add:
else if (trace_unknown_sec_event_enabled()) {
instead, as then this wont be called unless the tracepoint is
activated. Will keep the logic from doing anything with
acpi_hest_generic_data_payload().
Note, that trace_*_enabled() is activated via a jump_label, thus
there's no branches involved.
-- Steve
> + void *unknown_err = acpi_hest_generic_data_payload(gdata);
> + trace_unknown_sec_event(&sec_type,
> + fru_id, fru_text, sec_sev,
> + unknown_err, gdata->error_data_length);
> + }
> }
> }
>
>
[toc] | [next] | [standalone]
| From | "Baicar, Tyler" <tbaicar@codeaurora.org> |
|---|---|
| Date | 2017-02-15 18:00 +0100 |
| Message-ID | <tbb5U-2sv-23@gated-at.bofh.it> |
| In reply to | #1581412 |
On 2/15/2017 8:52 AM, Steven Rostedt wrote:
> On Wed, 1 Feb 2017 10:16:51 -0700
> Tyler Baicar <tbaicar@codeaurora.org> wrote:
>
>> @@ -452,11 +454,21 @@ static void ghes_do_proc(struct ghes *ghes,
>> {
>> int sev, sec_sev;
>> struct acpi_hest_generic_data *gdata;
>> + uuid_le sec_type;
>> + uuid_le *fru_id = &NULL_UUID_LE;
>> + char *fru_text = "";
>>
>> sev = ghes_severity(estatus->error_severity);
>> apei_estatus_for_each_section(estatus, gdata) {
>> sec_sev = ghes_severity(gdata->error_severity);
>> - if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
>> + sec_type = *(uuid_le *)gdata->section_type;
>> +
>> + if (gdata->validation_bits & CPER_SEC_VALID_FRU_ID)
>> + fru_id = (uuid_le *)gdata->fru_id;
>> + if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)
>> + fru_text = gdata->fru_text;
>> +
>> + if (!uuid_le_cmp(sec_type,
>> CPER_SEC_PLATFORM_MEM)) {
>> struct cper_sec_mem_err *mem_err;
>>
>> @@ -467,7 +479,7 @@ static void ghes_do_proc(struct ghes *ghes,
>> ghes_handle_memory_failure(gdata, sev);
>> }
>> #ifdef CONFIG_ACPI_APEI_PCIEAER
>> - else if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
>> + else if (!uuid_le_cmp(sec_type,
>> CPER_SEC_PCIE)) {
>> struct cper_sec_pcie *pcie_err;
>>
>> @@ -500,6 +512,12 @@ static void ghes_do_proc(struct ghes *ghes,
>>
>> }
>> #endif
>> + else {
> As an optimization, you can add:
>
> else if (trace_unknown_sec_event_enabled()) {
>
> instead, as then this wont be called unless the tracepoint is
> activated. Will keep the logic from doing anything with
> acpi_hest_generic_data_payload().
>
> Note, that trace_*_enabled() is activated via a jump_label, thus
> there's no branches involved.
>
> -- Steve
Hello Steve,
In v9 I currently have this and the ARM trace event from this series
both wrapped in an
ifdef verifying that CONFIG_RAS is enabled. This resolves the kbuild
failures and
will have this code compiled out when that config isn't enabled. Do you
think I
should use the ifdef or use these *_enabled() functions?
Thanks,
Tyler
>> + void *unknown_err = acpi_hest_generic_data_payload(gdata);
>> + trace_unknown_sec_event(&sec_type,
>> + fru_id, fru_text, sec_sev,
>> + unknown_err, gdata->error_data_length);
>> + }
>> }
>> }
>>
>>
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
[toc] | [prev] | [next] | [standalone]
| From | "Baicar, Tyler" <tbaicar@codeaurora.org> |
|---|---|
| Date | 2017-02-15 18:10 +0100 |
| Message-ID | <tbbfA-2La-11@gated-at.bofh.it> |
| In reply to | #1581458 |
On 2/15/2017 10:03 AM, Steven Rostedt wrote: > On Wed, 15 Feb 2017 09:54:09 -0700 > "Baicar, Tyler" <tbaicar@codeaurora.org> wrote: > \ >> In v9 I currently have this and the ARM trace event from this series >> both wrapped in an >> ifdef verifying that CONFIG_RAS is enabled. This resolves the kbuild >> failures and >> will have this code compiled out when that config isn't enabled. Do you >> think I >> should use the ifdef or use these *_enabled() functions? >> > I believe you need both. The *_enabled() functions wont prevent build > errors. But since tracing is seldom enabled (usually only when you want > to see what's happening), the *_enabled() functions optimize the code > to not perform tasks that are only needed when tracing is enabled. When > I say "enabled" I mean actively tracing, as supposed to being just > enabled in the kernel. > > -- Steve Got it, I will add both! Thanks, Tyler -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-02-15 18:10 +0100 |
| Message-ID | <tbbfA-2La-13@gated-at.bofh.it> |
| In reply to | #1581458 |
On Wed, 15 Feb 2017 09:54:09 -0700 "Baicar, Tyler" <tbaicar@codeaurora.org> wrote: \ > > In v9 I currently have this and the ARM trace event from this series > both wrapped in an > ifdef verifying that CONFIG_RAS is enabled. This resolves the kbuild > failures and > will have this code compiled out when that config isn't enabled. Do you > think I > should use the ifdef or use these *_enabled() functions? > I believe you need both. The *_enabled() functions wont prevent build errors. But since tracing is seldom enabled (usually only when you want to see what's happening), the *_enabled() functions optimize the code to not perform tasks that are only needed when tracing is enabled. When I say "enabled" I mean actively tracing, as supposed to being just enabled in the kernel. -- Steve
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web