Re: [Xen-devel] [PATCH 3/3] x86: add accessors for scratch cpu mask

[Jan Beulich <jbeulich@suse.com>]

On 13.02.2020 16:15, Roger Pau Monné wrote:
> On Thu, Feb 13, 2020 at 11:12:12AM +0100, Jan Beulich wrote:
>> On 12.02.2020 17:49, Roger Pau Monne wrote:
>>> --- a/xen/arch/x86/smpboot.c
>>> +++ b/xen/arch/x86/smpboot.c
>>> @@ -57,6 +57,30 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_mask);
>>>  DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, scratch_cpumask);
>>>  static cpumask_t scratch_cpu0mask;
>>>  
>>> +#ifndef NDEBUG
>>> +cpumask_t *scratch_cpumask(const char *fn)
>>
>> Please don't pass an argument that you can deduce, and then
>> provide even more meaningful data:
>>
>>> +{
>>> +    static DEFINE_PER_CPU(const char *, scratch_cpumask_use);
>>> +
>>> +    /*
>>> +     * Scratch cpumask cannot be used in IRQ context, or else we would have to
>>> +     * make sure all users have interrupts disabled while using the scratch
>>> +     * mask.
>>> +     */
>>> +    BUG_ON(in_irq());
>>> +
>>> +    if ( fn && unlikely(this_cpu(scratch_cpumask_use)) )
>>> +    {
>>> +        printk("%s: scratch CPU mask already in use by %s\n",
>>> +              fn, this_cpu(scratch_cpumask_use));
>>
>> Use __builtin_return_address(0) here, which will allow
>> identifying which of perhaps multiple uses in a function is
>> the offending one.
> 
> Will change.
> 
>>
>> Also, why in smpboot.c instead of in smp.c? This isn't a
>> boot or CPU-hot-online related function afaict.
> 
> I've added it to smpboot.c because that's where scratch_cpumask is
> defined. I could move it to smp.c, but I would prefer to keep the
> accessor as close as possible to the declaration.

May I suggest then to move the definition of the symbol?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel