Re: S3 resume issue in cpufreq -> get_cpu_idle_time->vcpu_runstate_get

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Wed, Aug 18, 2021 at 08:24:39AM +0200, Juergen Gross wrote:
> Marek,
> 
> On 17.08.21 17:15, Juergen Gross wrote:
> > On 17.08.21 16:50, Marek Marczykowski-Górecki wrote:
> > I'll be looking into that.
> 
> Could you please try the attached patch?

It works, thanks!

So, the only other suspend-related issue I'm aware of, is:
https://lore.kernel.org/xen-devel/20210131021526.GB6354@mail-itl/

But I don't have reliable reproducer for that one...


> From e38d0816a33fbaa3c0c45bcd6e9d433b1e021222 Mon Sep 17 00:00:00 2001
> From: Juergen Gross <jgross@xxxxxxxx>
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: George Dunlap <george.dunlap@xxxxxxxxxx>
> Cc: Dario Faggioli <dfaggioli@xxxxxxxx>
> Date: Wed, 18 Aug 2021 08:18:20 +0200
> Subject: [PATCH] xen/sched: fix get_cpu_idle_time() for smt=0 suspend/resume
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> With smt=0 during a suspend/resume cycle of the machine the threads
> which have been parked before will briefly come up again. This can
> result in problems e.g. with cpufreq driver being active as this will
> call into get_cpu_idle_time() for a cpu without initialized scheduler
> data.
> 
> Fix that by letting get_cpu_idle_time() deal with this case.
> 
> Fixes: 132cbe8f35632fb2 ("sched: fix get_cpu_idle_time() with core 
> scheduling")
> Reported-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

Tested-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>

> ---
> An alternative way to fix the issue would be to keep the sched_resource
> of offline cpus allocated like we already do with idle vcpus and units.
> This fix would be more intrusive, but it would avoid similar other bugs
> like this one.
> ---
>  xen/common/sched/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
> index 6d34764d38..9ac1b01ca8 100644
> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -337,7 +337,7 @@ uint64_t get_cpu_idle_time(unsigned int cpu)
>      struct vcpu_runstate_info state = { 0 };
>      const struct vcpu *v = idle_vcpu[cpu];
>  
> -    if ( cpu_online(cpu) && v )
> +    if ( cpu_online(cpu) && v && get_sched_res(cpu) )
>          vcpu_runstate_get(v, &state);
>  
>      return state.time[RUNSTATE_running];
> -- 
> 2.26.2
> 






-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature