Re: [Xen-devel] [PATCH] xen: Add comment for missing FROZEN notifier transitions

[Juergen Gross <jgross@suse.com>]

On 04/04/16 18:48, Boris Ostrovsky wrote:
> On 04/04/2016 12:30 PM, David Vrabel wrote:
>> On 04/04/16 17:21, Julien Grall wrote:
>>> (CC Stefano new e-mail address)
>>>
>>> Hello Anna-Maria,
>>>
>>> On 04/04/2016 13:32, Anna-Maria Gleixner wrote:
>>>> Xen guests do not offline/online CPUs during suspend/resume and
>>>> therefore FROZEN notifier transitions are not required. Add this
>>>> explanation as a comment in the code to get not confused why
>>>> CPU_TASKS_FROZEN masked transitions are not considered.
>> Alternatively, these could be added even if they are not encountered.
>> This might be more future-proof but the documentation might be clearer.
>>
>> Boris, Juergen, any opinion?

I'd rather do more than a comment:

Either mask CPU_TASKS_FROZEN from action if it really doesn't matter
whether the flag is set or not (which IMHO is the case here), or
BUG_ON(action & CPU_TASKS_FROZEN) if this really should never happen.

> Wouldn't the same comment need to be added to xen_hvm_cpu_notify()?

The patch of Anna-Maria does that.


Juergen

> 
> 
> -boris
> 
> 
>>
>> David>> --- a/drivers/xen/events/events_fifo.c
>>>> +++ b/drivers/xen/events/events_fifo.c
>>>> @@ -425,6 +425,12 @@ static int evtchn_fifo_cpu_notification(
>>>>        int cpu = (long)hcpu;
>>>>        int ret = 0;
>>>>
>>>> +    /*
>>>> +     * Xen guests do not offline/online CPUs during
>>>> +     * suspend/resume, thus CPU_TASKS_FROZEN masked transitions
>>>> +     * are not considered.
>>>> +    */
>>> NIT: The '*' is not aligned with the others.
>> If this doesn't need any other changes, I'll fix this on commit.
>>
>> David
> 
>