From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
To: ego@linux.vnet.ibm.com, Michael Ellerman <mpe@ellerman.id.au>
Cc: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>,
linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>,
Tyrel Datwyler <tyreld@linux.ibm.com>
Subject: Re: [PATCH v4 6/6] pseries/sysfs: Minimise IPI noise while reading [idle_][s]purr
Date: Thu, 02 Apr 2020 13:04:34 +0530 [thread overview]
Message-ID: <1585811157.uig8s95yst.naveen@linux.ibm.com> (raw)
In-Reply-To: <20200401120127.GC17237@in.ibm.com>
Gautham R Shenoy wrote:
> Hello Naveen,
>
>
> On Wed, Apr 01, 2020 at 03:28:48PM +0530, Naveen N. Rao wrote:
>> Gautham R. Shenoy wrote:
>> >From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>> >
> [..snip..]
>
>> >+
>> >+static ssize_t show_purr(struct device *dev,
>> >+ struct device_attribute *attr, char *buf)
>> > {
>> >- u64 *ret = val;
>> >+ struct cpu *cpu = container_of(dev, struct cpu, dev);
>> >+ struct util_acct_stats *stats;
>> >
>> >- *ret = read_this_idle_purr();
>> >+ stats = get_util_stats_ptr(cpu->dev.id);
>> >+ return sprintf(buf, "%llx\n", stats->latest_purr);
>>
>> This alters the behavior of the current sysfs purr file. I am not sure if it
>> is reasonable to return the same PURR value across a 10ms window.
>
>
> It does reduce it to 10ms window. I am not sure if anyone samples PURR
> etc faster than that rate.
>
> I measured how much time it takes to read the purr, spurr, idle_purr,
> idle_spurr files back-to-back. It takes not more than 150us. From
> lparstat will these values be read back-to-back ? If so, we can reduce
> the staleness_tolerance to something like 500us and still avoid extra
> IPIs. If not, what is the maximum delay between the first sysfs file
> read and the last sysfs file read ?
Oh, for lparstat usage, this is perfectly fine.
I meant that there could be other users of [s]purr who might care. I
don't know of one, but since this is an existing sysfs interface, I
wanted to point out that the behavior might change.
>
>>
>> I wonder if we should introduce a sysctl interface to control thresholding.
>> It can default to 0, which disables thresholding so that the existing
>> behavior continues. Applications (lparstat) can optionally set it to suit
>> their use.
>
> We would be introducing 3 new sysfs interfaces that way instead of
> two.
>
> /sys/devices/system/cpu/purr_spurr_staleness
> /sys/devices/system/cpu/cpuX/idle_purr
> /sys/devices/system/cpu/cpuX/idle_spurr
>
> I don't have a problem with this. Nathan, Michael, thoughts on this?
>
>
> The alternative is to have a procfs interface, something like
> /proc/powerpc/resource_util_stats
>
> which gives a listing similar to /proc/stat, i.e
>
> CPUX <purr> <idle_purr> <spurr> <idle_spurr>
>
> Even in this case, the values can be obtained in one-shot with a
> single IPI and be printed in the row corresponding to the CPU.
Right -- and that would be optimal requiring a single system call, at
the cost of using a legacy interface.
The other option would be to drop this patch and to just go with patches
1-5 introducing the new sysfs interfaces for idle_[s]purr. It isn't
entirely clear how often this would be used, or its actual impact. We
can perhaps consider this optimization if and when this causes
problems...
Thanks,
Naveen
next prev parent reply other threads:[~2020-04-02 7:34 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-27 11:32 [PATCH v4 0/6] [PATCH v4 0/6] Track and expose idle PURR and SPURR ticks Gautham R. Shenoy
2020-03-27 11:32 ` [PATCH v4 1/6] powerpc: Move idle_loop_prolog()/epilog() functions to header file Gautham R. Shenoy
2020-03-27 11:32 ` [PATCH v4 2/6] powerpc/idle: Add accessor function to always read latest idle PURR Gautham R. Shenoy
2020-04-01 9:42 ` Naveen N. Rao
2020-04-03 6:15 ` Gautham R Shenoy
2020-04-03 10:34 ` Naveen N. Rao
2020-04-03 11:24 ` Gautham R Shenoy
2020-03-27 11:32 ` [PATCH v4 3/6] powerpc/pseries: Account for SPURR ticks on idle CPUs Gautham R. Shenoy
2020-03-27 11:32 ` [PATCH v4 4/6] powerpc/sysfs: Show idle_purr and idle_spurr for every CPU Gautham R. Shenoy
2020-03-27 11:32 ` [PATCH v4 5/6] Documentation: Document sysfs interfaces purr, spurr, idle_purr, idle_spurr Gautham R. Shenoy
2020-04-01 9:45 ` Naveen N. Rao
2020-03-27 11:32 ` [PATCH v4 6/6] pseries/sysfs: Minimise IPI noise while reading [idle_][s]purr Gautham R. Shenoy
2020-04-01 9:58 ` Naveen N. Rao
2020-04-01 12:01 ` Gautham R Shenoy
2020-04-02 7:34 ` Naveen N. Rao [this message]
2020-04-03 6:28 ` Gautham R Shenoy
2020-04-03 18:10 ` Nathan Lynch
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=1585811157.uig8s95yst.naveen@linux.ibm.com \
--to=naveen.n.rao@linux.vnet.ibm.com \
--cc=ego@linux.vnet.ibm.com \
--cc=kamalesh@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=svaidy@linux.vnet.ibm.com \
--cc=tyreld@linux.ibm.com \
/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).