On Tue, Mar 17, 2015 at 12:36:58PM +0100, Quentin Casasnovas wrote:
> Right, FWIW I think your approach is valid, but not very generic. Re-using
> the check_insn() and making it more generic so we can widen its use felt
> like a better approach to me.
>
> AIUI, you didn't like my earlier draft because it wasn't very readable, but
> I think this was just due to the (bad) example I took and by reworking it a
> bit more, we could end up with the code you previously envisionned:
>
> if (static_cpu_has_safe(X86_FEATURE_XSAVEOPT))
> return check_insn(XSAVEOPT, xsave_buf, ...);
> else if (static_cpu_has_safe(X86_FEATURE_XSAVES)
> return check_insn(XSAVES, xsave_buf, ...);
> else
> return check_insn(XSAVE, xsave_buf, ...)
>
> Or maybe you were saying the actual macros weren't readable?
Well, TBH, I don't like check_insn() either:
* naming is generic but it is not really used in a generic way - only in
FPU code.
* having variable arguments makes it really really unreadable to me when
you start looking at how it is called:
...
if (config_enabled(CONFIG_X86_32))
return check_insn(fxrstor %[fx], "=m" (*fx), [fx] "m" (*fx));
...
The only thing that lets me differentiate what is input and what is
output is the "=" in there and you have to know inline asm to know that.
* The arguments have the same syntax as inline asm() arguments but you
don't see "asm volatile" there so it looks like something half-arsed in
between.
* the first argument is the instruction string with the operands which
gets stringified, yuck!
Do I need to say more? :-)
So what I would like is if we killed those half-arsed macros and
use either generic, clean macros like the alternatives or define
FPU-specific ones which do what FPU code needs done. If the second,
they should be self-contained, all in one place so that you don't have
to grep like crazy to rhyme together what the macro does - nothing like
xsave_fault. Yuck.
Or even extend the generic macros to fit the FPU use case, if possible
and if it makes sense.
Oh, and we shouldn't leave readability somewhere on the road.
I hope you catch my drift here.
Thanks.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
