linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Viresh Kumar <viresh.kumar@linaro.org>
To: Ionela Voinescu <ionela.voinescu@arm.com>
Cc: Rafael Wysocki <rjw@rjwysocki.net>,
	linux-pm@vger.kernel.org,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Qian Cai <quic_qiancai@quicinc.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH V3 2/4] cpufreq: cppc: Pass structure instance by reference
Date: Thu, 24 Jun 2021 07:52:52 +0530	[thread overview]
Message-ID: <20210624022252.zrxsftrvcd43eqra@vireshk-i7> (raw)
In-Reply-To: <20210623134533.GB12411@arm.com>

On 23-06-21, 14:45, Ionela Voinescu wrote:
> On Monday 21 Jun 2021 at 14:49:35 (+0530), Viresh Kumar wrote:
> > Don't pass structure instance by value, pass it by reference instead.
> >
> 
> Might be best to justify the change a bit :)

I had it and removed later as I thought it would be obvious :)

> For me this is a judgement call, and I don't really see the reasons for
> changing it: we don't care if the structure is modified or not, as we're
> not reusing the data after the call to cppc_get_rate_from_fbctrs().
> More so, in this scenario we might not even want for the called function
> to modify the counter values. Also there is no further call to a function
> in cppc_get_rate_from_fbctrs(), that might require references to the
> fb_ctrs.
> 
> So what is the reason behind this change?

How about this commit log then:

Theoretically speaking, call by reference is cheaper/faster than call by value
for structures as the later requires the compiler to make a new copy of the
whole structure (which has four u64 values here), to be used by the called
function, while with call by reference we just need to pass a single pointer
(u64 on 64-bit architectures) to the existing structure.

Yes, on modern architectures, the compilers will likely end up using the
processor registers for passing this structure as it isn't doesn't have lot of
fields and it shouldn't be bad eventually, but nevertheless the code should do
the right thing without assuming about the compiler's or architecture's
optimizations.

Don't pass structure instance by value, pass it by reference instead.

-- 
viresh

  reply	other threads:[~2021-06-24  2:22 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-21  9:19 [PATCH V3 0/4] cpufreq: cppc: Add support for frequency invariance Viresh Kumar
2021-06-21  9:19 ` [PATCH V3 1/4] cpufreq: cppc: Fix potential memleak in cppc_cpufreq_cpu_init Viresh Kumar
2021-06-23 13:44   ` Ionela Voinescu
2021-06-24  2:08     ` Viresh Kumar
2021-06-24  2:10   ` [PATCH V3.1 " Viresh Kumar
2021-06-25 10:33     ` Ionela Voinescu
2021-06-21  9:19 ` [PATCH V3 2/4] cpufreq: cppc: Pass structure instance by reference Viresh Kumar
2021-06-23 13:45   ` Ionela Voinescu
2021-06-24  2:22     ` Viresh Kumar [this message]
2021-06-25 10:30       ` Ionela Voinescu
2021-06-21  9:19 ` [PATCH V3 3/4] arch_topology: Avoid use-after-free for scale_freq_data Viresh Kumar
2021-06-23 13:50   ` Ionela Voinescu
2021-06-21  9:19 ` [PATCH V3 4/4] cpufreq: CPPC: Add support for frequency invariance Viresh Kumar
2021-06-24  9:48   ` Ionela Voinescu
2021-06-24 13:04     ` Viresh Kumar
2021-06-25  8:54       ` Ionela Voinescu
2021-06-25 16:54         ` Viresh Kumar
2021-06-28 10:49           ` Ionela Voinescu
2021-06-29  4:32             ` Viresh Kumar
2021-06-29  8:47               ` Ionela Voinescu
2021-06-29  8:53                 ` Viresh Kumar
2021-06-21 20:48 ` [PATCH V3 0/4] cpufreq: cppc: " Qian Cai
2021-06-22  6:52   ` Viresh Kumar
2021-06-23  4:16   ` Viresh Kumar
2021-06-23 12:57     ` Qian Cai
2021-06-24  2:54       ` Viresh Kumar
2021-06-24  9:49         ` Vincent Guittot
2021-06-24 10:48           ` Ionela Voinescu
2021-06-24 11:15             ` Vincent Guittot
2021-06-24 11:23               ` Ionela Voinescu
2021-06-24 11:59                 ` Vincent Guittot
2021-06-24 15:17             ` Qian Cai
2021-06-25 10:21               ` Ionela Voinescu
2021-06-25 13:31                 ` Qian Cai
2021-06-25 14:37                   ` Ionela Voinescu
2021-06-25 16:56                     ` Qian Cai
2021-06-26  2:29                     ` Qian Cai
2021-06-26 13:41                       ` Qian Cai
2021-06-29  4:55                         ` Viresh Kumar
2021-06-29  4:52                       ` Viresh Kumar
2021-06-29  9:06                       ` Ionela Voinescu
2021-06-29 13:38                         ` Qian Cai
2021-06-29  4:45                   ` Viresh Kumar
2021-06-24 20:44             ` Qian Cai
2021-06-28 11:54 ` Ionela Voinescu
2021-06-28 12:14   ` Vincent Guittot
2021-06-28 12:17     ` Vincent Guittot
2021-06-28 13:08     ` Ionela Voinescu
2021-06-28 21:37       ` Ionela Voinescu
2021-06-29  8:45         ` Vincent Guittot
2021-06-29  5:20   ` Viresh Kumar
2021-06-29  8:46     ` Ionela Voinescu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210624022252.zrxsftrvcd43eqra@vireshk-i7 \
    --to=viresh.kumar@linaro.org \
    --cc=ionela.voinescu@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=quic_qiancai@quicinc.com \
    --cc=rjw@rjwysocki.net \
    --cc=vincent.guittot@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).