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

[Juergen Gross <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2001 bytes --]

On 07.06.26 08:45, Ingo Molnar wrote:
> 
> * Jürgen Groß <jgross@suse.com> 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 #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]