linux-kernel - Re: [PATCH v2 16/16] thermal/traces: Replace the thermal zone structure parameter with the field value

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 22 Feb 2023 21:07:58 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org, Amit Kucheria <amitk@...nel.org>,
        Zhang Rui <rui.zhang@...el.com>,
        Lukasz Luba <lukasz.luba@....com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        "open list:TRACING" <linux-trace-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 16/16] thermal/traces: Replace the thermal zone
 structure parameter with the field value

On Wed, Feb 22, 2023 at 9:02 PM Daniel Lezcano
<daniel.lezcano@...aro.org> wrote:
>
> On 22/02/2023 20:51, Rafael J. Wysocki wrote:
> > On Tue, Feb 21, 2023 at 7:08 PM Daniel Lezcano
> > <daniel.lezcano@...aro.org> wrote:
> >>
> >> In the work of the thermal zone device self-encapsulation, let's pass
> >> the field value instead of dereferencing them in the traces which
> >> force us to export publicly the thermal_zone_device structure.
> >>
> >> No fonctionnal change intended.
> >>
> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org>
> >> ---
> >>   drivers/thermal/gov_fair_share.c              |  2 +-
> >>   drivers/thermal/gov_power_allocator.c         |  4 ++--
> >>   drivers/thermal/gov_step_wise.c               |  2 +-
> >>   drivers/thermal/thermal_core.c                |  5 ++--
> >>   include/trace/events/thermal.h                | 24 +++++++++----------
> >>   .../trace/events/thermal_power_allocator.h    | 12 +++++-----
> >>   6 files changed, 25 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/drivers/thermal/gov_fair_share.c b/drivers/thermal/gov_fair_share.c
> >> index aad7d5fe3a14..e6c21abaaa80 100644
> >> --- a/drivers/thermal/gov_fair_share.c
> >> +++ b/drivers/thermal/gov_fair_share.c
> >> @@ -35,7 +35,7 @@ static int get_trip_level(struct thermal_zone_device *tz)
> >>           * point, in which case, trip_point = count - 1
> >>           */
> >>          if (count > 0)
> >> -               trace_thermal_zone_trip(tz, count - 1, trip.type);
> >> +               trace_thermal_zone_trip(tz->type, tz->id, count - 1, trip.type);
> >
> > Haven't you introduced an accessor for tz->id in this series?  Why not
> > use it here?
> >
> > And there can be an analogous accessor for tz->type.
> >
> > If there are accessors like that, they should be used consistently
> > everywhere as applicable IMO.
>
> governors are part of the thermal core code, so they are authorized to
> access the thermal structure internals, that is why they are not passing
> through the accessors.

I'm not talking about "authorization", but about consistency.

If accessors are used consistently, it is sufficient to grep for an
accessor to find all places where the given field is accessed, for
example.