Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
MIME-Version: 1.0
References: <1957441.PYKUYFuaPT@kreacher> <2590280.Lt9SDvczpP@kreacher>
 <9f1374db-f866-ff52-e86c-99fecee24d1b@linaro.org> <CAJZ5v0hw3LGByW=18tiFEB7JQ9rOVo6TqCGhzxWcrUQYb+VGEw@mail.gmail.com>
 <f193e4a5-b315-a1bd-7488-7ea0d5e267f8@linaro.org>
In-Reply-To: <f193e4a5-b315-a1bd-7488-7ea0d5e267f8@linaro.org>
From:   "Rafael J. Wysocki" <rafael@kernel.org>
Date:   Wed, 27 Sep 2023 18:14:06 +0200
Message-ID: <CAJZ5v0h_HB6DX3NemyojE8aZrQeH05aAsdKLE-q-gp4UC+NBJQ@mail.gmail.com>
Subject: Re: [PATCH v1 07/13] thermal: gov_power_allocator: Use trip pointers
 instead of trip indices
To:     Daniel Lezcano <daniel.lezcano@linaro.org>
Cc:     "Rafael J. Wysocki" <rafael@kernel.org>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Linux PM <linux-pm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Linux ACPI <linux-acpi@vger.kernel.org>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
        Zhang Rui <rui.zhang@intel.com>,
        Lukasz Luba <lukasz.luba@arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On Wed, Sep 27, 2023 at 5:46 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 27/09/2023 17:27, Rafael J. Wysocki wrote:
> > On Wed, Sep 27, 2023 at 5:10 PM Daniel Lezcano
> > <daniel.lezcano@linaro.org> wrote:
> >>
> >> On 21/09/2023 19:55, Rafael J. Wysocki wrote:
> >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>>
> >>> Eliminate the __thermal_zone_get_trip() usage that adds unnecessary
> >>> overhead (due to pointless bounds checking and copying of trip point
> >>> data) from the power allocator thermal governor and generally make it
> >>> use trip pointers instead of trip indices where applicable.
> >>
> >> Actually the __thermal_zone_get_trip() change was done on purpose to
> >> replace the 'throttle' callback index parameter by the trip pointer and
> >> removing those call to __thermal_zone_get_trip() while the code was
> >> using the trip pointer.
> >>
> >> IMO, the changes should focus on changing the trip_index parameter by
> >> the trip pointer directly in the throttle ops.
> >
> > So you would like .throttle() to take a trip pointer argument instead
> > of an index?
> >
> > The difficulty here is that the user space governor needs to expose
> > the index to user space anyway, so it would need to find it if it gets
> > a trip pointer instead.
> >
> > Not a big deal I suppose, but a bit of extra overhead.
> >
> > Also it is easier to switch the governors over to using trip pointers
> > internally and then change the .throttle() argument on top of that.
> >
> >> The pointer can be
> >> retrieved in the handle_thermal_trip() function and passed around for
> >> the rest of the actions on this trip point
> >
> > Right, except for the user space governor which needs a trip index.
> > And the indices are used for tracing too.
>
> Given the userspace governor is going obsolete and the notifications are
> for the userspace, which is slow, we can retrieve the index from the
> throttling ops

OK

Given that patches [01-05/13] are not controversial, I'll respon the
governor patches into a separate series on top of them.

I would much appreciate it if you could take a look at patch [10/13]
and the remaining ACPi thermal patches in this series [11-13/13].
They don't depend on the governor changes.