Received-SPF: pass (google.com: domain of linux-kernel+bounces-73264-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 20 Feb 2024 23:27:21 +0800
From: Feng Tang <feng.tang@intel.com>
To: "Paul E. McKenney" <paulmck@kernel.org>
CC: Waiman Long <longman@redhat.com>, Thomas Gleixner <tglx@linutronix.de>,
	John Stultz <jstultz@google.com>, Stephen Boyd <sboyd@kernel.org>, "Peter
 Zijlstra" <peterz@infradead.org>, <linux-kernel@vger.kernel.org>, Jin Wang
	<jin1.wang@intel.com>
Subject: Re: [PATCH v3] clocksource: Scale the max retry number of watchdog
 read according to CPU numbers
Message-ID: <ZdTE2SzO1pZ0QjUn@feng-clx.sh.intel.com>
References: <20240129134505.961208-1-feng.tang@intel.com>
 <87msrwadvu.ffs@tglx>
 <ZdNnjdNTjtvpbGH0@feng-clx.sh.intel.com>
 <388686b2-5305-43d1-8edf-19ba66d52727@redhat.com>
 <0b7833a4-75f3-43ba-9d87-6f83cf4faa5a@paulmck-laptop>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <0b7833a4-75f3-43ba-9d87-6f83cf4faa5a@paulmck-laptop>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?E5nGaBenukb1wrrA6R0KEN9uVqXMaPRu6IHrpMWzioMAcMXl7g+8NNJe5yrN?=
 =?us-ascii?Q?RSGRSkW1qRseXkWIAY4fFaCRJsaDqf310TtOIkrrEO5RyjVF779Ea3xdSDFu?=
 =?us-ascii?Q?kk+QsGkZviIEAfvgUR6FHu5mCOMqfH6ndtE5wdStniuniEMFS7Z1DyYYwjpQ?=
 =?us-ascii?Q?zYARtcQUUpvv5pJyKO1OOX+YrYiEwb6r0WGBvDHtp2/wxOIV38a2jVPKfMoP?=
 =?us-ascii?Q?s4vcb+5/XmTE0qaEjg9mnj13EK0OOxAm6auT6uZ/8rKO9EszusJ22UbBfOxe?=
 =?us-ascii?Q?/HIr0vwJD6EsI5KsY8pRiKpCX6ptYVaB3ieLLH0+F35MoAZKxLCRkrB7Y8DV?=
 =?us-ascii?Q?bGVuCsbPBvmrPzacRXHOQNNic8FTHbZUntqXlCMOIO35PUkVC8YHILL8dtdz?=
 =?us-ascii?Q?H1S75UUJvnZ47IeR4SLxxu/Xtz06Rl+NVF1aGebWWwZuq9IreYIlgjkBfh0p?=
 =?us-ascii?Q?dos9mDRzc7dc3l0TJ3vYZ76wCimna2hI1d3PhXj3DkJFjXPvdy7/Sn1OtbRD?=
 =?us-ascii?Q?CPagCf0s4U13+4EtgdednCB6LCrSYcpu0Vxy+1CRM3AEkFFgJPGHsgg2zgo4?=
 =?us-ascii?Q?3IXTpXlDGrEx/ZOH8AzsTylbmNkpyvnhGouWj+4fKZMAcPdSLHZzWRDNGXlx?=
 =?us-ascii?Q?o4owWX3EETSjzAndDytIREJuI4a5juaXasuJBqMNHEbIL0kPqko1suBzTENG?=
 =?us-ascii?Q?SVfgtueCJmtsBnMLobeMjGMkasNTbB3YaLxagiGwGfj5VfMij3f62G3RZ2nf?=
 =?us-ascii?Q?eS2ZcxwApghfUFknaIxA3ul0nQAX059qsNLtRqdpFF+4msbbAr6A4gFFRqXo?=
 =?us-ascii?Q?ZishBD8PDrG+caq1fwQ4rjCa2A4ifTQSJqP0k/SjtpBY0aJvYQiwwFj9akHi?=
 =?us-ascii?Q?M2KtT1c6oWYuARX7MpU9qd98i9KxcSNKy6oUvmG1iI+SVRidWA7o1VOUJCL2?=
 =?us-ascii?Q?o0OTH7zJFDeN3YFz+yij2wUEktLQYl5c4pSMQI9L/p1EPGhpAZ4hC/1xcCaN?=
 =?us-ascii?Q?VmBLT9ViV1FFNN7uvGMJZFlGnP6uKwXXN4OuBefAefJIJmaYtp6KcekBH/O7?=
 =?us-ascii?Q?7nj8e4Japd968Ul/8CTG6LPiaOkCeK2L/c9Hc3jChodGMnrDE3JPOSoi/hdr?=
 =?us-ascii?Q?cUpsIbUXQT9TCHnU5Su2T6IODMaBJwJxyH0LSp6HHjMqVK577a/HFeOCb+br?=
 =?us-ascii?Q?nmceCC6iaKbbcWJ5O/9mSDnbMl10IQu95m7ElMmkqJCTyQXBFjfm8fSciuRJ?=
 =?us-ascii?Q?9935of+6hrN//WypH6Cj1bYfnqxxNq8gDcH3AJoLpfc/c3GkNwZ+WmwthDHt?=
 =?us-ascii?Q?iY7ZDcYjpTyrkNT1qH0ji0onhZ2qXNbd+0zSfzo6MGV5WSntpxjhTBa/9zI1?=
 =?us-ascii?Q?9tddmHmGRrF8uNsjbFhZqMWI+G4ClIrqqQ8Og6CjRlYK61sFAXNrv2QlQmAT?=
 =?us-ascii?Q?rmU4IyfEL2xUxaqvv1Q5Lwud53iFzcLXVSQhS0XsIJZNFWv2VQ0jvMtKua//?=
 =?us-ascii?Q?nHsanf6YBsEBZiZi6dOOogSpOwhaQqPEvwu86ic1JFwhKXs1cLSG2eQIOdT0?=
 =?us-ascii?Q?rpcQamdzwNGpHrQOQ4EHYC5ie5pPcnLRsqi57cXQ?=
X-MS-Exchange-CrossTenant-Network-Message-Id: db82b271-f221-4481-67f6-08dc322a6899
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6304.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Feb 2024 15:41:30.6019
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 6dasKC7BrWORSAj5W/nnfrMurHceP5bsMT3xkWxmtssQ0mt4yjC6sgdZIwd5qgvmFR8Ax7EiWAlp91XhVvIjAA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB8094
X-OriginatorOrg: intel.com

On Tue, Feb 20, 2024 at 07:24:27AM -0800, Paul E. McKenney wrote:
> On Mon, Feb 19, 2024 at 09:20:31PM -0500, Waiman Long wrote:
> > 
> > On 2/19/24 09:37, Feng Tang wrote:
> > > Hi Thomas,
> > > 
> > > On Mon, Feb 19, 2024 at 12:32:05PM +0100, Thomas Gleixner wrote:
> > > > On Mon, Jan 29 2024 at 21:45, Feng Tang wrote:
> > > > > +static inline long clocksource_max_watchdog_read_retries(void)
> > > > > +{
> > > > > +	long max_retries = max_cswd_read_retries;
> > > > > +
> > > > > +	if (max_cswd_read_retries <= 0) {
> > > > > +		/* santity check for user input value */
> > > > > +		if (max_cswd_read_retries != -1)
> > > > > +			pr_warn_once("max_cswd_read_retries was set with an invalid number: %ld\n",
> > > > > +				max_cswd_read_retries);
> > > > > +
> > > > > +		max_retries = ilog2(num_online_cpus()) + 1;
> > > > I'm getting tired of these knobs and the horrors behind them. Why not
> > > > simply doing the obvious:
> > > > 
> > > >         retries = ilog2(num_online_cpus()) + 1;
> > > > 
> > > > and remove the knob alltogether?
> > > Thanks for the suggestion! Yes, this makes sense to me. IIUC, the
> > > 'max_cswd_read_retries' was introduced mainly to cover different
> > > platforms' requirement, which could now be covered by the new
> > > self-adaptive number.
> > > 
> > > If there is no concern from other developers, I will send a new
> > > version in this direction.
> > 
> > I see no problem simplifying it.
> 
> My guess is that we will eventually end up with something like this:
> 
> 	retries = ilog2(num_online_cpus()) / 2 + 1;

Good point! Initially when writing the patch, I did try to search if
there is a 'ilog4' api :) as the ilog2 of that 8-socket machine is
about 10, which is more than enough.

Thanks,
Feng

> 
> but I am not at all opposed to starting without the division by 2.
> 
> 							Thanx, Paul