On Wed, Apr 16, 2014 at 02:23:23AM -0400, Chen, Gong wrote:
> We have different CPER error type. For example, for processor error type,
> it has following definition:
>
> struct cper_sec_proc_generic {
> __u64 validation_bits;
> __u8 proc_type;
> __u8 proc_isa;
> __u8 proc_error_type;
> __u8 operation;
> __u8 flags;
> __u8 level;
> __u16 reserved;
> __u64 cpu_version;
> char cpu_brand[128];
> __u64 proc_id;
> __u64 target_addr;
> __u64 requestor_id;
> __u64 responder_id;
> __u64 ip;
> };
>
> If we want to show it in string format, 256 bytes should be not enough. But
> by now we don't use this macro for processor error type so I will shrink it
> to 256 bytes and add a comment for it.
Yeah, we don't have to always adhere to the spec if we feel it doesn't
make any sense. A lot of those fields above are purely useless and we
shouldn't carry them blindly to the outside.
For example, we don't need to carry cpu_brand[128] to the outside for
*every* error. Who even came up with this crap, is beyond me??? It's
like the cpu changes brand on every other error or what? You harvest
this info only *once* from /proc/cpuinfo. cpu_version too. And so on and
so on...
Please sanity-check stuff like that before hardcoding it into the
tracepoint. If it is in the spec it doesn't always mean it makes sense.
We need to carry out only the minimum amount of information of each
error which is actually getting used in userspace, for additional RAS
actions. Carrying fat blobs just because the spec says so is simply
wrong.
Thanks.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
