Received: by 2002:a05:7412:d024:b0:f9:90c9:de9f with SMTP id bd36csp200342rdb; Wed, 20 Dec 2023 09:45:48 -0800 (PST) X-Google-Smtp-Source: AGHT+IHo+2j5o6nmIYNN+8vTy99HiQaDWXZRqW1QKPRJ/+DpsY79e55vCvLRzEdPeznqkt8cEsTv X-Received: by 2002:a17:903:1111:b0:1d3:a11a:6bbc with SMTP id n17-20020a170903111100b001d3a11a6bbcmr9188325plh.127.1703094348531; Wed, 20 Dec 2023 09:45:48 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1703094348; cv=none; d=google.com; s=arc-20160816; b=xbCHZC/iMXXUsTZcpiXjZ/kL7eFMPv5ApLGnI0UHKT+aqY11ZLVMHGSN37RNCMKupP bD4WFHeGcbhBX5SkAowEnBarav7zBdijLuq+uX7CBKf4ri43iKzzx64t47C/hG5ImNhf mww+ed+CBsPwqAYuK1wBsirOhrSp+jxYmqYabYoxi5QSk6x6h+kMEmXiFIcwBixeizDS X6YsxXu5QQy2uKt1qM6hyiT7958gMXuf0TbNUfD13aPgvoqjVOjDTVmaaWNTSmbgP+ZU ERu1nh19wkV5R6Rtb9JfxRgol39cnH6OSsoxE+3OmWrDfNIknBoTBIh8nj4nxaKRq9Zf jg1w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:date:message-id; bh=rsUeEEXg8IVQoxUP8yveVQ655vgBXqLOmhkmAnKc8y0=; fh=s+3ccwKL/Xc2i8uH9tAeSOb9IGpvLhPrVmnBNs/vSzQ=; b=JGHutW4t04bWFo/isspkG9zE50rCOr6DE+LeuHjJT1EaY01s5/wG/jm4DOxASkbYIA z6JFJpHozeC4AhYk92ecHRQBZVPwKOANEkPeTTWGkZ2fHcZXCmXYcaKQNZt4/Uiv63qI moJvSOdWw1KljeNvEXslxNWmKt9rQvjoZwdlyqTdA566SJqriqJgrkycnJeJ/zN4DPpY ejU6e4kfO4zjfBFXtv6HhWV+GVH7B+rzNEVflpHJBzivogGb2N4lCX6uugS+26E+64dz gBm1ShTWbOvFcXZVYZCfP1uH9qfsGWFmBTpfCxgLcAg8mJOZ4I6aURODcM8Z0P7RVDxX /TSA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel+bounces-7363-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-7363-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Return-Path: Received: from sy.mirrors.kernel.org (sy.mirrors.kernel.org. [2604:1380:40f1:3f00::1]) by mx.google.com with ESMTPS id n2-20020a170902d2c200b001d0c3e70d3asi22253348plc.490.2023.12.20.09.45.48 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Dec 2023 09:45:48 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-7363-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel+bounces-7363-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-7363-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sy.mirrors.kernel.org (Postfix) with ESMTPS id 6DF6AB2488C for ; Wed, 20 Dec 2023 17:45:44 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id D1148481B5; Wed, 20 Dec 2023 17:45:34 +0000 (UTC) X-Original-To: linux-kernel@vger.kernel.org Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 4232447A61; Wed, 20 Dec 2023 17:45:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6C79F2F4; Wed, 20 Dec 2023 09:46:16 -0800 (PST) Received: from [10.57.82.217] (unknown [10.57.82.217]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9CF5E3F64C; Wed, 20 Dec 2023 09:45:30 -0800 (PST) Message-ID: <70910a73-64ff-476e-af84-7e227bdf8509@arm.com> Date: Wed, 20 Dec 2023 17:46:37 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/8] thermal: core: Add governor callback for thermal zone change Content-Language: en-US To: "Rafael J. Wysocki" Cc: linux-kernel@vger.kernel.org, daniel.lezcano@linaro.org, linux-pm@vger.kernel.org, rui.zhang@intel.com References: <20231212134844.1213381-1-lukasz.luba@arm.com> <20231212134844.1213381-2-lukasz.luba@arm.com> <4e5f7d1b-1534-432b-92c1-880c863d79a2@arm.com> From: Lukasz Luba In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 12/20/23 17:44, Rafael J. Wysocki wrote: > On Wed, Dec 20, 2023 at 5:16 PM Lukasz Luba wrote: >> >> >> >> On 12/20/23 13:51, Rafael J. Wysocki wrote: >>> On Tue, Dec 12, 2023 at 2:48 PM Lukasz Luba wrote: >>>> >>>> Add a new callback which can update governors when there is a change in >>>> the thermal zone internals, e.g. thermal cooling instance list changed. >>> >>> I would say what struct type the callback is going to be added to. >> >> OK, I'll add that. >> >>> >>>> That makes possible to move some heavy operations like memory allocations >>>> related to the number of cooling instances out of the throttle() callback. >>>> >>>> Reuse the 'enum thermal_notify_event' and extend it with a new event: >>>> THERMAL_INSTANCE_LIST_UPDATE. >>> >>> I think that this is a bit too low-level (see below). >> >> Yes, I agree (based on below). >> >>> >>>> Both callback code paths (throttle() and update_tz()) are protected with >>>> the same thermal zone lock, which guaranties the consistency. >>>> >>>> Signed-off-by: Lukasz Luba >>>> --- >>>> drivers/thermal/thermal_core.c | 13 +++++++++++++ >>>> include/linux/thermal.h | 5 +++++ >>>> 2 files changed, 18 insertions(+) >>>> >>>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c >>>> index 625ba07cbe2f..592c956f6fd5 100644 >>>> --- a/drivers/thermal/thermal_core.c >>>> +++ b/drivers/thermal/thermal_core.c >>>> @@ -314,6 +314,14 @@ static void handle_non_critical_trips(struct thermal_zone_device *tz, >>>> def_governor->throttle(tz, trip); >>>> } >>>> >>> >>> I needed a bit more time to think about this. >> >> OK. >> >>> >>>> +static void handle_instances_list_update(struct thermal_zone_device *tz) >>>> +{ >>>> + if (!tz->governor || !tz->governor->update_tz) >>>> + return; >>>> + >>>> + tz->governor->update_tz(tz, THERMAL_INSTANCE_LIST_UPDATE); >>>> +} >>> >>> So I would call the above something more generic, like >>> thermal_governor_update_tz() and I would pass the "reason" argument to >>> it. >> >> That sounds better, I agree. >> >>> >>>> + >>>> void thermal_zone_device_critical(struct thermal_zone_device *tz) >>>> { >>>> /* >>>> @@ -723,6 +731,8 @@ int thermal_bind_cdev_to_trip(struct thermal_zone_device *tz, >>>> list_add_tail(&dev->tz_node, &tz->thermal_instances); >>>> list_add_tail(&dev->cdev_node, &cdev->thermal_instances); >>>> atomic_set(&tz->need_update, 1); >>>> + >>>> + handle_instances_list_update(tz); >>> >>> In particular for this, I would use a special "reason" value, like >>> THERMAL_TZ_BIND_CDEV. >>> >>> Yes, the list of instances will change as a result of the binding, but >>> that is an internal detail specific to the current implementation. >> >> I see. With that new more generic thermal_governor_update_tz() would >> be better then, right? > > I think so, IIUC. OK, thanks!