Received-SPF: pass (google.com: domain of linux-kernel+bounces-182000-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Fri, 17 May 2024 18:32:21 +0800
From: Wendy Wang <wendy.wang@intel.com>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
CC: Linux PM <linux-pm@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>, Lukasz Luba <lukasz.luba@arm.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>, Srinivas Pandruvada
	<srinivas.pandruvada@linux.intel.com>, Zhang Rui <rui.zhang@intel.com>
Subject: Re: [PATCH v1] thermal: core: Fix the handling of invalid trip points
Message-ID: <ZkcuVPWNLOxxBu9x@p-catlow01.sh.intel.com>
References: <12441937.O9o76ZdvQC@kreacher>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <12441937.O9o76ZdvQC@kreacher>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?10oOetQi4y+9rVFMe6Oznpc+9UdY93z5KcuKV3LKcz09RIglzGPQvwemtyAX?=
 =?us-ascii?Q?6ZKIWbTUmpnILB/VvgPpeULwWcd1bUpmv99ARtzr2neO761inLnnHPKnfrbC?=
 =?us-ascii?Q?cr33fP2ilYizWvYIonXrWdRTzqvPKVByEwG3yJmCX52maeldM+PGuJcaDa3b?=
 =?us-ascii?Q?rv2BXDSZuriuBBj7b12XInnmWLJDlMCrTv3xfy7l7wdkcI9e9R56Iu1jeJGJ?=
 =?us-ascii?Q?sZ/j6v455oVB3rqObm36W6v/gtejYBjSBGUg5bcMOvcthymUGBXkFuOEbWz6?=
 =?us-ascii?Q?mSVwRfEXjxE+b55qxBSC9kfM4VNp0FBZ3VfFfR7n3VmBlVuZzd+VfhEP68FG?=
 =?us-ascii?Q?A2xR34hQvG+4N7QWQd3xgmm0DueFThpxu9GpD5oZv4YrdiQIg1FtjFTsreRS?=
 =?us-ascii?Q?RusmiJGHWiugrYSN/MWESc62LUdXJzVuTLlu2NdwUOdWsjRgYSSg6PCiHu3x?=
 =?us-ascii?Q?Hcj0tcx/+0k/DfYWexjFPOIatZY/vto9RhhKFlsZ25jr57VV/pmYfondQiVE?=
 =?us-ascii?Q?Kj+DmKAwfoEJGCTkh6g9DBUjhpHvH0hQeG8Y1ipqnoVUZIyQJc36452uI2h0?=
 =?us-ascii?Q?yJZR3mtwv+nIMEc6T0JIyELx1jdzvgOMsYvz0y16/MA4JlV6zGTEaYp5r492?=
 =?us-ascii?Q?gfcif9rO4D3JwooiLbnFfXo2mrUNJI4nwww+qNfAnC8Wu/bR6T88dT9Rs2MH?=
 =?us-ascii?Q?CDUdC6+421xd6QccoosRywVrZi/7Klq5gblITXKAC43gFkQjL+06d7zBw/Dk?=
 =?us-ascii?Q?4SwVE/S0KraZZGAxRlS6jLmZiT3CcXYdYk+OpF7IASijn3QHGzTFb5Kj+mtb?=
 =?us-ascii?Q?Aoa4yLrpMdPJ4o85Wu0ddbG0SQHksoz+rPnJT8818XN1TRj5zAHBWM8dHETQ?=
 =?us-ascii?Q?J9xIlSPuloQAzLhbAd/zGvuwOZN/utVtQ7QAYrQKvGS9Q5dsUhKs+T6G0T3i?=
 =?us-ascii?Q?q/O7+pb3HVjmz+wWIW1cIx7x3pimFaPmnRhLhPHbA8gX4YNBIaJ3M1OHgjMq?=
 =?us-ascii?Q?K++rQJcOdxvZO47Gcd53Dhq78aouodG1eUAzin++u3LuZOnUVnS2GbZJO1tI?=
 =?us-ascii?Q?jiN5cfV9wGN+y5HXlHvOfPN84ftAHr+AzPO0boJi0D6G9dp9cJoOkHJ2aQHO?=
 =?us-ascii?Q?u8fnzAyNo7TK4w+6cUAvMExuaL4aWR6e6Ud2TeH2akozFS6WMAPdoLrd3NlL?=
 =?us-ascii?Q?U5mX3lBvmF+8N2ry+dRm2R53QlT8nNk1CQ88GB764XkInjc6Kb0Fv4/npAyM?=
 =?us-ascii?Q?mi3A8FRjmw0HW06Bn30zuAga38JPwMTCkiHw9OlonvQcyjnf5tz74x5laH4U?=
 =?us-ascii?Q?XR1AHCw6HBfDTGFyPAOHxM3fZ5R8tmFzt7w4+b81bYt2PIw3z/AqyrJWTRTg?=
 =?us-ascii?Q?C1osB4TzgzbFrpUn9A3S4l9gg+f9RU9MYwM1UX2601Rgbh7e6TgfwYsk4Th+?=
 =?us-ascii?Q?nt/Ze6VcvJYpJTcGEOeImSGq1RycoARo+DwSpxTU/SCKeyDiByQucP/3cBLb?=
 =?us-ascii?Q?SCdPveWN8QlG7atpp39Pkge75S84+wAwVBjU9Myloo4jygqZp9jAq0zVCp0h?=
 =?us-ascii?Q?p/nvz2DK4i0T9RNUjGc1k1EFZm5HJtIlFNvnYtNd?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b477907f-8a06-49cb-0424-08dc765cb590
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB8490.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 May 2024 10:32:53.5770
 (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: ViV7trF5FzywszIu9W2wqh6/z8QPOs8rgAep7Uk1dKxg5DXqnivj2dnKJuYuPbQvT61fFfiGY2wmbxkRQoK1Ww==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR11MB8353
X-OriginatorOrg: intel.com

Hi Rafael,

I have tested your patch '[PATCH v1] thermal: core: Fix the handling of invalid trip points'
on my server, which is running the v6.9 kernel, the thermal throttling test case has passed.

Tested-by: Wendy Wang <wendy.wang@intel.com>

On 2024-05-16 at 19:20:19 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Commit 9ad18043fb35 ("thermal: core: Send trip crossing notifications
> at init time if needed") overlooked the case when a trip point that
> has started as invalid is set to a valid temperature later.  Namely,
> the initial threshold value for all trips is zero, so if a previously
> invalid trip becomes valid and its (new) low temperature is above the
> zone temperature, a spurious trip crossing notification will occur and
> it may trigger the WARN_ON() in handle_thermal_trip().
> 
> To address this, set the initial threshold for all trips to INT_MAX.
> 
> There is also the case when a valid writable trip becomes invalid that
> requires special handling.  First, in accordance with the change
> mentioned above, the trip's threshold needs to be set to INT_MAX to
> avoid the same issue.  Second, if the trip in question is passive and
> it has been crossed by the thermal zone temperature on the way up, the
> zone's passive count has been incremented and it is in the passive
> polling mode, so its passive count needs to be adjusted to allow the
> passive polling to be turned off eventually.
> 
> Fixes: 9ad18043fb35 ("thermal: core: Send trip crossing notifications at init time if needed")
> Fixes: 042a3d80f118 ("thermal: core: Move passive polling management to the core")
> Reported-by: Zhang Rui <rui.zhang@intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/thermal/thermal_core.c |    9 ++++++++-
>  drivers/thermal/thermal_trip.c |   18 ++++++++++++++++++
>  2 files changed, 26 insertions(+), 1 deletion(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -1401,8 +1401,15 @@ thermal_zone_device_register_with_trips(
>  	tz->device.class = thermal_class;
>  	tz->devdata = devdata;
>  	tz->num_trips = num_trips;
> -	for_each_trip_desc(tz, td)
> +	for_each_trip_desc(tz, td) {
>  		td->trip = *trip++;
> +		/*
> +		 * Mark all thresholds as invalid to start with even though
> +		 * this only matters for the trips that start as invalid and
> +		 * become valid later.
> +		 */
> +		td->threshold = INT_MAX;
> +	}
>  
>  	thermal_set_delay_jiffies(&tz->passive_delay_jiffies, passive_delay);
>  	thermal_set_delay_jiffies(&tz->polling_delay_jiffies, polling_delay);
> Index: linux-pm/drivers/thermal/thermal_trip.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_trip.c
> +++ linux-pm/drivers/thermal/thermal_trip.c
> @@ -137,6 +137,24 @@ void thermal_zone_set_trip_temp(struct t
>  	if (trip->temperature == temp)
>  		return;
>  
> +	if (temp == THERMAL_TEMP_INVALID) {
> +		struct thermal_trip_desc *td = trip_to_trip_desc(trip);
> +
> +		if (trip->type == THERMAL_TRIP_PASSIVE &&
> +		    tz->temperature >= td->threshold) {
> +			/*
> +			 * The trip has been crossed, so the thermal zone's
> +			 * passive count needs to be adjusted.
> +			 */
> +			tz->passive--;
> +			WARN_ON_ONCE(tz->passive < 0);
> +		}
> +		/*
> +		 * Invalidate the threshold to avoid triggering a spurious
> +		 * trip crossing notification when the trip becomes valid.
> +		 */
> +		td->threshold = INT_MAX;
> +	}
>  	WRITE_ONCE(trip->temperature, temp);
>  	thermal_notify_tz_trip_change(tz, trip);
>  }
> 
> 
>