[PATCH] xen: Add comment for missing FROZEN notifier transitions

[PATCH] xen: Add comment for missing FROZEN notifier transitions

[Anna-Maria Gleixner]

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.

Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: xen-devel@lists.xenproject.org 
Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
---
 arch/arm/xen/enlighten.c         |    6 ++++++
 arch/x86/xen/enlighten.c         |    7 +++++++
 drivers/xen/events/events_fifo.c |    6 ++++++
 3 files changed, 19 insertions(+)

--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -213,6 +213,12 @@ static int xen_cpu_notification(struct n
 				unsigned long action,
 				void *hcpu)
 {
+	/*
+	 * Xen guests do not offline/online CPUs during
+	 * suspend/resume, thus CPU_TASKS_FROZEN masked transitions
+	 * are not considered.
+	 */
+
 	switch (action) {
 	case CPU_STARTING:
 		xen_percpu_init();
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1788,6 +1788,13 @@ static int xen_hvm_cpu_notify(struct not
 			      void *hcpu)
 {
 	int cpu = (long)hcpu;
+
+	/*
+	 * Xen guests do not offline/online CPUs during
+	 * suspend/resume, thus CPU_TASKS_FROZEN masked transitions
+	 * are not considered.
+	 */
+
 	switch (action) {
 	case CPU_UP_PREPARE:
 		xen_vcpu_setup(cpu);
--- 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.
+	*/
+
 	switch (action) {
 	case CPU_UP_PREPARE:
 		if (!per_cpu(cpu_control_block, cpu))

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Stefano Stabellini]

On Wed, 6 Apr 2016, David Vrabel wrote:
> On 04/04/16 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.
> > 
> > Cc: David Vrabel <david.vrabel@citrix.com>
> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Cc: xen-devel@lists.xenproject.org 
> > Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
> > ---
> >  arch/arm/xen/enlighten.c         |    6 ++++++
> >  arch/x86/xen/enlighten.c         |    7 +++++++
> >  drivers/xen/events/events_fifo.c |    6 ++++++
> >  3 files changed, 19 insertions(+)
> > 
> > --- a/arch/arm/xen/enlighten.c
> > +++ b/arch/arm/xen/enlighten.c
> > @@ -213,6 +213,12 @@ static int xen_cpu_notification(struct n
> >  				unsigned long action,
> >  				void *hcpu)
> >  {
> > +	/*
> > +	 * Xen guests do not offline/online CPUs during
> > +	 * suspend/resume, thus CPU_TASKS_FROZEN masked transitions
> > +	 * are not considered.
> > +	 */
> 
> This may not be true for arm guests.
 
ARM guests behave like x86 PV guests in this regard. I expect the
comment to be appropriate for both archs or none.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Stefano Stabellini]

On Tue, 5 Apr 2016, Juergen Gross wrote:
> 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.

I agree

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[David Vrabel]

On 04/04/16 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.
> 
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: xen-devel@lists.xenproject.org 
> Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
> ---
>  arch/arm/xen/enlighten.c         |    6 ++++++
>  arch/x86/xen/enlighten.c         |    7 +++++++
>  drivers/xen/events/events_fifo.c |    6 ++++++
>  3 files changed, 19 insertions(+)
> 
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -213,6 +213,12 @@ static int xen_cpu_notification(struct n
>  				unsigned long action,
>  				void *hcpu)
>  {
> +	/*
> +	 * Xen guests do not offline/online CPUs during
> +	 * suspend/resume, thus CPU_TASKS_FROZEN masked transitions
> +	 * are not considered.
> +	 */

This may not be true for arm guests.

David

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Juergen Gross]

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
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Boris Ostrovsky]

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?

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


-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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[David Vrabel]

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?

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Julien Grall]

(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.
>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: xen-devel@lists.xenproject.org
> Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
> ---
>   arch/arm/xen/enlighten.c         |    6 ++++++
>   arch/x86/xen/enlighten.c         |    7 +++++++
>   drivers/xen/events/events_fifo.c |    6 ++++++
>   3 files changed, 19 insertions(+)
>
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -213,6 +213,12 @@ static int xen_cpu_notification(struct n
>   				unsigned long action,
>   				void *hcpu)
>   {
> +	/*
> +	 * Xen guests do not offline/online CPUs during
> +	 * suspend/resume, thus CPU_TASKS_FROZEN masked transitions
> +	 * are not considered.
> +	 */
> +
>   	switch (action) {
>   	case CPU_STARTING:
>   		xen_percpu_init();
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1788,6 +1788,13 @@ static int xen_hvm_cpu_notify(struct not
>   			      void *hcpu)
>   {
>   	int cpu = (long)hcpu;
> +
> +	/*
> +	 * Xen guests do not offline/online CPUs during
> +	 * suspend/resume, thus CPU_TASKS_FROZEN masked transitions
> +	 * are not considered.
> +	 */
> +
>   	switch (action) {
>   	case CPU_UP_PREPARE:
>   		xen_vcpu_setup(cpu);
> --- 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.

> +
>   	switch (action) {
>   	case CPU_UP_PREPARE:
>   		if (!per_cpu(cpu_control_block, cpu))
>

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel