Date: Mon, 7 Dec 2015 15:43:19 +0100
From: Miroslav Lichvar <mlichvar@redhat.com>
To: John Stultz <john.stultz@linaro.org>
Cc: linux-kernel@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
        Richard Cochran <richardcochran@gmail.com>,
        Prarit Bhargava <prarit@redhat.com>, Andy Lutomirski <luto@kernel.org>
Subject: Re: [RFC][PATCH] timekeeping: Cap adjustments so they don't exceede
 the maxadj value
Message-ID: <20151207144319.GB1402@localhost>
References: <1449266274-26517-1-git-send-email-john.stultz@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1449266274-26517-1-git-send-email-john.stultz@linaro.org>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1928
Lines: 54

On Fri, Dec 04, 2015 at 01:57:54PM -0800, John Stultz wrote:
> Thus its been occasionally noted that users have seen
> confusing warnings like:
> 
>     Adjusting tsc more than 11% (5941981 vs 7759439)

Thanks for looking into this, John.

> Thus this patch adds some extra logic to enforce the max adjustment
> cap in the internal steering.
> 
> Note: This has the potential to slow corrections when the ADJ_TICK
> value is furthest away from the default value. So it would be good to
> get some testing from folks using Chrony, to make sure we don't
> cause any troubles there.

I ran few tests with chronyd, forcing it to make the largest possible
adjustment of the tick and it seems to work fine.

However, I'm still able to trigger the warning by feeding a large
offset to the NTP PLL using a short time constant. When the multiplier
is already at the maximum and it wants to move it even further, there
should be no timekeeping_apply_adjustment() call as it always adjusts
the multiplier at least by one.

> +	adj_scale = 0;
>  	tick_error = abs(tick_error);
> -	for (adj = 0; tick_error > interval; adj++)
> +	while (tick_error > interval) {
> +		u32 base = tk->tkr_mono.clock->mult;
> +		u32 max = tk->tkr_mono.clock->maxadj;
> +		u32 cur_adj = tk->tkr_mono.mult;
> +		u32 adj = 1 << (adj_scale + 1);
> +
> +		if (negative && (cur_adj - adj) < (base - max))
> +			break;
> +		if (!negative && (cur_adj + adj) > (base + max))
> +			break;
> +
> +		adj_scale++;
>  		tick_error >>= 1;
> +	}
>  
>  	/* scale the corrections */
> -	timekeeping_apply_adjustment(tk, offset, negative, adj);
> +	timekeeping_apply_adjustment(tk, offset, negative, adj_scale);

-- 
Miroslav Lichvar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/