Andy Lutomirski <luto@xxxxxxxxxx> wrote:
> On Fri, Nov 17, 2017 at 10:19 AM, Nadav Amit <namit@xxxxxxxxxx> wrote:
>
>> +static inline void update_cr4(unsigned long cr4)
>> +{
>> +#ifdef CONFIG_LOCKDEP
>> + WARN_ON(!irqs_disabled());
>> +#endif
>> + this_cpu_write(cpu_tlbstate.cr4, cr4);
>> + __write_cr4(cr4);
>> +}
>
> Let's call this __cr4_set() instead of update_cr4(). The __ is to
> remind people that it's not really for use and starting with cr4_ is
> consistent with the rest of the functions.
>
> Also, can you split this whole thing into two patches? The
> introduction of the separate update function is a great cleanup, but I
> think it's orthogonal.
Sure. Will do. Thanks for reviewing it.
Just one small clarification: I do not see an issue of correctness right now
if __native_flush_tlb_global_irq_disabled() is the only one that modifies
CR4 in an interrupt handler. Yet, the current scheme seems error prone.
Regards,
Nadav
