Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 8 Jan 2020 11:50:11 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Anchal Agarwal <anchalag@amazon.com>
Cc:     tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, hpa@zytor.com,
        x86@kernel.org, boris.ostrovsky@oracle.com, jgross@suse.com,
        linux-pm@vger.kernel.org, linux-mm@kvack.org, kamatam@amazon.com,
        sstabellini@kernel.org, konrad.wilk@oracle.co,
        roger.pau@citrix.com, axboe@kernel.dk, davem@davemloft.net,
        rjw@rjwysocki.net, len.brown@intel.com, pavel@ucw.cz,
        eduval@amazon.com, sblbir@amazon.com,
        xen-devel@lists.xenproject.org, vkuznets@redhat.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        Woodhouse@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com,
        dwmw@amazon.co.uk, fllinden@amaozn.com
Subject: Re: [RFC PATCH V2 11/11] x86: tsc: avoid system instability in
 hibernation
Message-ID: <20200108105011.GY2827@hirez.programming.kicks-ass.net>
References: <20200107234526.GA19034@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200107234526.GA19034@dev-dsk-anchalag-2a-9c2d1d96.us-west-2.amazon.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Jan 07, 2020 at 11:45:26PM +0000, Anchal Agarwal wrote:
> From: Eduardo Valentin <eduval@amazon.com>
> 
> System instability are seen during resume from hibernation when system
> is under heavy CPU load. This is due to the lack of update of sched
> clock data, and the scheduler would then think that heavy CPU hog
> tasks need more time in CPU, causing the system to freeze
> during the unfreezing of tasks. For example, threaded irqs,
> and kernel processes servicing network interface may be delayed
> for several tens of seconds, causing the system to be unreachable.

> The fix for this situation is to mark the sched clock as unstable
> as early as possible in the resume path, leaving it unstable
> for the duration of the resume process. This will force the
> scheduler to attempt to align the sched clock across CPUs using
> the delta with time of day, updating sched clock data. In a post
> hibernation event, we can then mark the sched clock as stable
> again, avoiding unnecessary syncs with time of day on systems
> in which TSC is reliable.

This makes no frigging sense what so bloody ever. If the clock is
stable, we don't care about sched_clock_data. When it is stable you get
a linear function of the TSC without complicated bits on.

When it is unstable, only then do we care about the sched_clock_data.

> Reviewed-by: Erik Quanstrom <quanstro@amazon.com>
> Reviewed-by: Frank van der Linden <fllinden@amazon.com>
> Reviewed-by: Balbir Singh <sblbir@amazon.com>
> Reviewed-by: Munehisa Kamata <kamatam@amazon.com>
> Tested-by: Anchal Agarwal <anchalag@amazon.com>
> Signed-off-by: Eduardo Valentin <eduval@amazon.com>
> ---

NAK, the code very much relies on never getting marked stable again
after it gets set to unstable.

> diff --git a/kernel/sched/clock.c b/kernel/sched/clock.c
> index 1152259a4ca0..374d40e5b1a2 100644
> --- a/kernel/sched/clock.c
> +++ b/kernel/sched/clock.c
> @@ -116,7 +116,7 @@ static void __scd_stamp(struct sched_clock_data *scd)
>  	scd->tick_raw = sched_clock();
>  }
>  
> -static void __set_sched_clock_stable(void)
> +void set_sched_clock_stable(void)
>  {
>  	struct sched_clock_data *scd;
>  
> @@ -236,7 +236,7 @@ static int __init sched_clock_init_late(void)
>  	smp_mb(); /* matches {set,clear}_sched_clock_stable() */
>  
>  	if (__sched_clock_stable_early)
> -		__set_sched_clock_stable();
> +		set_sched_clock_stable();
>  
>  	return 0;
>  }
> -- 
> 2.15.3.AMZN
>