From: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
To: Mimi Zohar <zohar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Cc: Dan Morav <dmorav-KrzQf0k3Iz9BDgjK7y7TUQ@public.gmane.org>,
Thomas-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
linux-ima-devel
<linux-ima-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>,
linux-security-module
<linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
tpmdd-devel
<tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>,
linux-fsdevel
<linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
Subject: Re: [RFC PATCH] tpm: msleep() delays - replace with usleep_range() in i2c nuvoton driver
Date: Thu, 2 Mar 2017 10:33:47 +0200 [thread overview]
Message-ID: <20170302083347.q33wiwsdcwbxyrp6@intel.com> (raw)
In-Reply-To: <1487957342.3193.200.camel-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
On Fri, Feb 24, 2017 at 12:29:02PM -0500, Mimi Zohar wrote:
> On Fri, 2017-02-24 at 19:01 +0200, Jarkko Sakkinen wrote:
> > On Thu, Feb 23, 2017 at 06:46:18PM -0500, Mimi Zohar wrote:
> > > Commit 500462a9de65 "timers: Switch to a non-cascading wheel" replaced
> > > the 'classic' timer wheel, which aimed for near 'exact' expiry of the
> > > timers. Their analysis was that the vast majority of timeout timers
> > > are used as safeguards, not as real timers, and are cancelled or
> > > rearmed before expiration. The only exception noted to this were
> > > networking timers with a small expiry time.
> > >
> > > Not included in the analysis was the TPM polling timer, which resulted
> > > in a longer normal delay and, every so often, a very long delay. The
> > > non-cascading wheel delay is based on CONFIG_HZ. For a description of
> > > the different rings and their delays, refer to the comments in
> > > kernel/time/timer.c.
> > >
> > > Below are the delays given for rings 0 - 2, which explains the longer
> > > "normal" delays and the very, long delays as seen on systems with
> > > CONFIG_HZ 250.
> > >
> > > * HZ 1000 steps
> > > * Level Offset Granularity Range
> > > * 0 0 1 ms 0 ms - 63 ms
> > > * 1 64 8 ms 64 ms - 511 ms
> > > * 2 128 64 ms 512 ms - 4095 ms (512ms - ~4s)
> > >
> > > * HZ 250
> > > * Level Offset Granularity Range
> > > * 0 0 4 ms 0 ms - 255 ms
> > > * 1 64 32 ms 256 ms - 2047 ms (256ms - ~2s)
> > > * 2 128 256 ms 2048 ms - 16383 ms (~2s - ~16s)
> > >
> > > Below is a comparison of extending the TPM with 1000 measurements,
> > > using msleep() vs. usleep_delay() when configured for 1000 hz vs. 250
> > > hz, before and after commit 500462a9de65.
> > >
> > > linux-4.7 | msleep() usleep_range()
> > > 1000 hz: 0m44.628s | 1m34.497s 29.243s
> > > 250 hz: 1m28.510s | 4m49.269s 32.386s
> > >
> > > linux-4.7 | min-max (msleep) min-max (usleep_range)
> > > 1000 hz: 0:017 - 2:760s | 0:015 - 3:967s 0:014 - 0:418s
> > > 250 hz: 0:028 - 1:954s | 0:040 - 4:096s 0:016 - 0:816s
> > >
> > > This patch replaces the msleep() with usleep_range() calls in the
> > > i2c nuvoton driver with a consistent max range value.
> > >
> > > Signed-of-by: Mimi Zohar <zohar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> > > Reviewed-by: Nayna Jain <nayna-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> >
> > So why doesn't it go to level 0 with msleep()? I quickly skimmed
> > through __mod_timer() and for me it looked like that level 0 would be
> > calculated (when it is eventually called starting from msleep()).
> > What did I miss?
>
> I've just added some printk's in kernel/time/timer.c. It looks like it
> is level 0. The delay seems to be caused by schedule() in
> schedule_timeout().
>
> setup_timer_on_stack(&timer, process_timeout, (unsigned
> long)current);
> __mod_timer(&timer, expire, false, false);
> schedule(); <===
> del_singleshot_timer_sync(&timer);
>
> /* Remove the timer from the object tracker */
> destroy_timer_on_stack(&timer);
>
>
> printks output:
> 124.901002] calc_wheel_index: level 0 timer: c000003fab32b150 expires
> 4294923520 new expires 4294923520 now 4294923518
> [ 124.901003] __mod_timer: exit timer c000003fab32b1a0 now 4294923518
>
> < call to schedule() >
>
> [ 128.607463] schedule_timeout: before destroy timer: c000003fab32b150
> expires 4294923520 now 4294924439 <=== notice that the "now" time is
> way beyond the expires time.
>
> Mimi
Hey, I totally forgot this patch! Sorry.
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
/Jarkko
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
next prev parent reply other threads:[~2017-03-02 8:33 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-23 23:46 [RFC PATCH] tpm: msleep() delays - replace with usleep_range() in i2c nuvoton driver Mimi Zohar
2017-02-24 0:00 ` Jason Gunthorpe
2017-02-24 0:26 ` Mimi Zohar
2017-04-06 15:27 ` [tpmdd-devel] " Nayna
2017-02-24 17:01 ` Jarkko Sakkinen
[not found] ` <20170224170100.prwbcsffxdqjudc2-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2017-02-24 17:29 ` Mimi Zohar
[not found] ` <1487957342.3193.200.camel-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2017-03-02 8:33 ` Jarkko Sakkinen [this message]
[not found] ` <20170302083347.q33wiwsdcwbxyrp6-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2017-03-07 13:39 ` Mimi Zohar
[not found] ` <1488893960.3216.45.camel-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2017-03-09 11:05 ` Jarkko Sakkinen
2017-03-10 11:41 ` Mimi Zohar
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=20170302083347.q33wiwsdcwbxyrp6@intel.com \
--to=jarkko.sakkinen-vuqaysv1563yd54fqh9/ca@public.gmane.org \
--cc=Thomas-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
--cc=dmorav-KrzQf0k3Iz9BDgjK7y7TUQ@public.gmane.org \
--cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-ima-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
--cc=linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org \
--cc=tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
--cc=zohar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.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).