Re: [PATCH] lockless MCE i386 port

[Andi Kleen]

> +#define MCE_LOG_LEN 32
> +#define MCE_OVERFLOW 0		/* bit 0 in flags means overflow */
> +#define MCE_LOG_SIGNATURE	"MACHINECHECK"
> +#define MCE_GET_RECORD_LEN	_IOR('M', 1, int)
> +#define MCE_GET_LOG_LEN		_IOR('M', 2, int)
> +#define MCE_GETCLEAR_FLAGS	_Io


Just noticed this: 

First I think it would be better if you used the same format
(with u64) as x86-64 because this is a user visible interface,
and we get problems with 32bit emulation if it's too different.

Also it would allow to share the mcelog.c codebase.

> +
> +struct mce {
> +	u64 tsc;	/* cpu timestamp counter */
> +	u32 stsl;
> +	u32 stsh;
> +	u32 miscl;
> +	u32 misch;
> +	u32 addrl;
> +	u32 addrh;
> +	u32 mcgstl;
> +	u32 mcgsth;
> +	u32 eip;
> +	u8  cs;		/* code segment */
> +	u8  bank;	/* machine check bank */
> +	u8  cpu;	/* cpu that raised the error */
> +	u8  finished;	/* entry is valid */
> +	void *ext_arg;	/* extended feature arg */

A pointer? That doesn't make sense. This record must 
be self contained because it is passed by read() 

-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/