From: Corey Minyard <minyard@acm.org>
To: Tejun Heo <tj@kernel.org>
Cc: openipmi-developer@lists.sourceforge.net, kernel-team@fb.com,
linux-kernel@vger.kernel.org
Subject: Re: [Openipmi-developer] [PATCH] ipmi_si_intf: use usleep_range() instead of busy looping
Date: Tue, 9 Jul 2019 18:07:03 -0500 [thread overview]
Message-ID: <20190709230703.GF19430@minyard.net> (raw)
In-Reply-To: <20190709221147.GM657710@devbig004.ftw2.facebook.com>
On Tue, Jul 09, 2019 at 03:11:47PM -0700, Tejun Heo wrote:
> On Tue, Jul 09, 2019 at 04:46:02PM -0500, Corey Minyard wrote:
> > On Tue, Jul 09, 2019 at 02:06:43PM -0700, Tejun Heo wrote:
> > > ipmi_thread() uses back-to-back schedule() to poll for command
> > > completion which, on some machines, can push up CPU consumption and
> > > heavily tax the scheduler locks leading to noticeable overall
> > > performance degradation.
> > >
> > > This patch replaces schedule() with usleep_range(100, 200). This
> > > allows the sensor readings to finish resonably fast and the cpu
> > > consumption of the kthread is kept under several percents of a core.
> >
> > The IPMI thread was not really designed for sensor reading, it was
> > designed so that firmware updates would happen in a reasonable time
> > on systems without an interrupt on the IPMI interface. This change
> > will degrade performance for that function. IIRC correctly the
> > people who did the patch tried this and it slowed things down too
> > much.
>
> Also, can you point me to the exact patch? I'm kinda curious what
> kind of timning they used.
I believe the change was 33979734cd35ae "IPMI: use schedule in kthread"
The original change that added the kthread was a9a2c44ff0a1350
"ipmi: add timer thread".
I mis-remembered this, we switched from doing a udelay() to
schedule(), but that udelay was 1us, so that's probably not helpful
information.
-corey
>
> Thanks.
>
> --
> tejun
>
>
> _______________________________________________
> Openipmi-developer mailing list
> Openipmi-developer@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openipmi-developer
next prev parent reply other threads:[~2019-07-09 23:07 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-09 21:06 [PATCH] ipmi_si_intf: use usleep_range() instead of busy looping Tejun Heo
2019-07-09 21:46 ` Corey Minyard
2019-07-09 22:09 ` Tejun Heo
2019-07-09 23:01 ` Corey Minyard
2019-07-10 14:22 ` Tejun Heo
2019-07-10 20:11 ` Corey Minyard
2019-08-01 17:40 ` Corey Minyard
2019-08-05 18:18 ` Tejun Heo
2019-08-05 21:18 ` Corey Minyard
2019-08-07 18:27 ` Tejun Heo
2019-07-09 22:11 ` Tejun Heo
2019-07-09 23:07 ` Corey Minyard [this message]
2019-07-10 14:12 ` [Openipmi-developer] " Tejun Heo
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=20190709230703.GF19430@minyard.net \
--to=minyard@acm.org \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=openipmi-developer@lists.sourceforge.net \
--cc=tj@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.