Re: [PATCH v2 07/10] x86/msr: Switch rdmsr_safe_on_cpu() users to rdmsrq_safe_on_cpu()

[Juergen Gross]

On 07.06.26 08:45, Ingo Molnar wrote:
> * Jürgen Groß <jgross@xxxxxxxx> wrote:
>
> > Okay, fine with me.
> >
> > I'll send V3 after rc1 of 7.2 as Dave suggested.
>
> Since these are really trivial with little value in postponing
> them over a full cycle and generating conflicts, a -rc7 base
> might work as well, as long as you send them in the next 1-2 days
> or so. (Unless Dave feels strongly about the timing.)
>
> > One final question (not directly for this series, but for a future one):
> >
> > I'd like to switch rdmsrq() from a macro to an inline function, as a macro
> > using one of its parameters as the destination for an assignment isn't
> > nice.
>
> So basically:
>
>    - rdmsrq(MSR_CORE_PERF_GLOBAL_CTRL, ctrl);
>    + ctrl = rdmsrq(MSR_CORE_PERF_GLOBAL_CTRL);
>
> ? Agreed with that.
>
> > Additionally I'd like to replace rdmsr*() use cases with rdmsrq*()
> > and wrmsr*() with wrmsrq*() (similar to the replacements done is this
> > series).
>
> Yeah.
>
> > The rdmsrq() transformation into a function can easily be done via a
> > coccinelle script. What would be your choice regarding the sequence doing
> > this?
> >
> > a) Start with rdmsrq() transformation into a function.
> > b) First do the rdmsr/wrmsr -> rdmsrq/wrmsrq replacements, then do the
> >     rdmsrq() transformation into a function (this will touch more code, as
> >     the initial rdmsr() use cases will be affected, too).
> > c) Leave rdmsrq() as a macro.
>
> I'd prefer (b), because while it's technically a bit more work,
> the flow is easier to review: the rdmsr/wrmsr replacements
> basically do a:
>
>    macro(u32, u32) =>> macro(u64)
>
> replacement, while with (a) we do a:
>
>    macro(u32, u32) => u64 = function()
>
> replacement that is a bit more complex transformation and bit
> harder to review/validate at a glance. So I think it's better
> to do the two transformations separately.

Okay, thanks. Will use this approach.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature