All of lore.kernel.org
 help / color / mirror / Atom feed
From: j.w.r.degoede@hhs.nl (Hans de Goede)
To: lm-sensors@vger.kernel.org
Subject: [lm-sensors] new abituguru driver in mm kernel
Date: Wed, 26 Jul 2006 20:43:37 +0000	[thread overview]
Message-ID: <44C7D3F9.3040202@hhs.nl> (raw)
In-Reply-To: <ce9ef0d90607041146t6837a584x68d1ab7e528139ea@mail.gmail.com>



Sunil Kumar wrote:
> Hans, I have done some very rudimentary performance analysis of this driver
> and I have a feeling that some tuning may be in order. All I did was to
> look
> at gkrellm with abituguru loaded and unloaded. gkrellm updates sensors
> every
> 5 seconds (which is kinda high I think) and I see that the cpu usage is
> 0-2%
> if module is unloaded. If module is loaded the cpu usage is as high as 4%
> every 5 seconds (more than half of it red). System is quiet and nothing
> else
> is being done on it.
> 
> I suspect its to do with the while loop in abitugru_wait(). What I did was
> to make the following change on top of what you sent and cpu usage
> became 2%
> every 5 seconds. So, it remains between 0-2%.
> 

Hmm, how did you measure this? According to top gkrellm on my system
never comes above the 0.7 percent, then again on my system the wait
function usually returns pretty fast.

> I don't think there is any harm in sleeping because most sensor user
> programs are working at seconds level and we are talking about ms sleep.
> And
> in most cases first sleep for 1 jiffy will be enough. But this probably
> needs testing by our volunteers to see if there are any ill-effects,
> although it works perfectly here.
> 

My worries are because abituguru_wait gets called 148 times for one
update! So on a system running at 100 HZ, that could get translated to
blocking the program trying to read a sensor for 1.5 seconds.

However on my system the uGuru usually responds within 50 polls in
abituguru wait with 1 in 10 waits only succeeding after 90 polls. So if
we can agree to make ABIT_UGURU_WAIT_TIMEOUT_SLEEP 150 instead of 200
then I think your patch can go in to help those with uGuru's which are
slower to respond like yours is.

The 1.5 seconds mentioned above assumes 1 sleep per wait, and with your
current code it could become much more sleeps. Thinking about this some
more, with your current code it could become 200 / 150 sleeps per wait!

I think the correct fix for this would be to just lower
ABIT_UGURU_WAIT_TIMEOUT from 250 to 100 now that we have the sleep
before final try code. That way we wil not sleep more then once per
wait() and we should still get the desired effect of lowering cpu usage.
Since your last version (sleep 1 ms before final try) worked always
without errors. I'm pretty sure we can just lower
ABIT_UGURU_WAIT_TIMEOUT to 100, those missed 150 reads won't make much
of a difference timing wise when compared with that msleep(1), so things
should still work as expected. The only differences being that wait()
will go to sleep sooner, resulting in the decreased cpu usage you want.

Could you give the last version I send you with ABIT_UGURU_WAIT_TIMEOUT
changed to 100 a try? I think that should do the trick.

Regards,

Hans



  parent reply	other threads:[~2006-07-26 20:43 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-07-04 18:46 [lm-sensors] new abituguru driver in mm kernel Sunil Kumar
2006-07-04 20:10 ` Stephen Cormier
2006-07-05  6:38 ` Hans de Goede
2006-07-05 16:35 ` Sunil Kumar
2006-07-05 17:37 ` Jean Delvare
2006-07-05 17:41 ` Hans de Goede
2006-07-05 17:44 ` Hans de Goede
2006-07-05 20:11 ` Hans de Goede
2006-07-05 20:16 ` Sunil Kumar
2006-07-06  1:09 ` Sunil Kumar
2006-07-06  1:24 ` Stephen Cormier
2006-07-08 23:03 ` Sunil Kumar
2006-07-09  8:16 ` Hans de Goede
2006-07-09 14:44 ` Sunil Kumar
2006-07-09 16:41 ` Sunil Kumar
2006-07-09 17:11 ` Hans de Goede
2006-07-09 17:30 ` Sunil Kumar
2006-07-09 20:32 ` Hans de Goede
2006-07-09 20:54 ` Sunil Kumar
2006-07-10  4:33 ` Hans de Goede
2006-07-11  4:43 ` Sunil Kumar
2006-07-14 19:15 ` Hans de Goede
2006-07-14 19:33 ` Sunil Kumar
2006-07-14 19:43 ` Hans de Goede
2006-07-14 19:50 ` Sunil Kumar
2006-07-15  0:52 ` Sunil Kumar
2006-07-19  4:35 ` Hans de Goede
2006-07-19 20:34 ` Sunil Kumar
2006-07-19 22:42 ` Sunil Kumar
2006-07-19 23:02 ` Sunil Kumar
2006-07-20  5:23 ` Sunil Kumar
2006-07-20  7:54 ` Hans de Goede
2006-07-20 14:37 ` Sunil Kumar
2006-07-20 17:13 ` Sunil Kumar
2006-07-20 17:14 ` Sunil Kumar
2006-07-21  6:10 ` Hans de Goede
2006-07-21 16:15 ` Sunil Kumar
2006-07-25  3:27 ` Sunil Kumar
2006-07-26 14:30 ` Hans de Goede
2006-07-26 18:32 ` Sunil Kumar
2006-07-26 20:43 ` Hans de Goede [this message]
2006-07-27  0:48 ` Sunil Kumar
2006-07-27  8:19 ` Hans de Goede
2006-07-27 14:31 ` Sunil Kumar
2006-07-27 14:44 ` Hans de Goede
2006-07-27 16:07 ` Sunil Kumar
2006-08-01  4:01 ` Hans de Goede
2006-08-25 23:13 ` Sunil Kumar

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=44C7D3F9.3040202@hhs.nl \
    --to=j.w.r.degoede@hhs.nl \
    --cc=lm-sensors@vger.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.