From: Patrick McHardy <kaber@trash•net>
To: Jarek Poplawski <jarkao2@gmail•com>
Cc: David Miller <davem@davemloft•net>, Martin Devera <devik@cdi•cz>,
netdev@vger•kernel.org
Subject: Re: [PATCH 2/6] pkt_sched: sch_htb: Consider used jiffies in htb_dequeue()
Date: Tue, 09 Dec 2008 15:56:09 +0100 [thread overview]
Message-ID: <493E8709.5070601@trash.net> (raw)
In-Reply-To: <20081209144534.GC15353@ff.dom.local>
Jarek Poplawski wrote:
> To David Miller: David don't apply yet - this patch needs change.
>
> Patrick, read below:
>
> On Tue, Dec 09, 2008 at 02:20:00PM +0100, Patrick McHardy wrote:
>>>>
>>>> The focus on jiffies is wrong IMO, the reason why we get high
>>>> load is because the CPU can't keep up, delaying things even
>>>> longer is not going to help get the work done. The only reason to
>>>> look at jiffies is because its a cheap indication that it has
>>>> ran for too long and we should give other tasks a change to run
>>>> as well, but it should continue immediately after it did that.
>>>> So all it should do is make sure that the watchdog is scheduled
>>>> with a very small positive delay.
>>>>
>>> This needs additional psched_get_time(), and as I've written before
>>> there is no apparent advantage in problematic cases, but this would
>>> add more overhead for common cases.
>>>
>> htb_do_events() exceeding two jiffies is fortunately not a common
>> case. You (incorrectly) made the calculation somewhat of a common
>> case by also adding to the delay if the inner classes simply throttled
>> and already returned the exact delay they want.
>
> I see! You are right and this needs fixing.
>
>> Much better (again considering what we want to achieve here) would
>> be to not use the hrtimer watchdog at all. We want to give lower
>> priority tasks a chance to run, so ideally we'd use a low priority
>> task for wakeup.
>
> I'm not sure I get ot right: for precise scheduling hrtimers look
> useful. Do you really mean "at all"?
I meant "at all" for the wakeup after we've decided HTB has too much
work to do at once. A work queue seems better suited since that makes
sure we allow other processes to run, but don't wait unnecessarily
long when there is no other work.
>>>> As for the implementation: the increase in delay (the snippet
>>>> above) is also done in the case that no packets were available
>>>> for other reasons (throttling), in which case we might needlessly
>>>> delay for an extra jiffie if jiffies wrapped while it tried to
>>>> dequeue.
>>>>
>>> But in another similar cases there could be no change in jiffies, but
>>> almost a jiffie used for counting, so wrong schedule time as well.
>> Its not "wrong". We don't want to delay. Its a courtesy to the
>> remaining system.
>
> In this case it's probably self-courtesy too: this ksoftirqd takes
> most of the time and it's useless.
Well, it calls back to HTB, which continues to do real work. But
leaving HTB, scheduling a timer just to be called immediately again
is useless overhead, I agree.
>>> Approximatly this all should be fine, and it still can be tuned later.
>>> IMHO, this all should not affect "common" cases, which are expected to
>>> use less then jiffie here.
>>>
>> Jiffies might wrap even if it only took only a few nanoseconds.
>> And its not fine, in the case of throttled classes there's no
>> reason to add extra delay *at all*.
>
> Yes, you are right with this. I can try too fix this tomorrow, unless
> you prefer to send your version of this patch.
I don't have a version of my own, so please go ahead :)
next prev parent reply other threads:[~2008-12-09 14:56 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-12-09 10:21 [PATCH 2/6] pkt_sched: sch_htb: Consider used jiffies in htb_dequeue() Jarek Poplawski
2008-12-09 10:28 ` Patrick McHardy
2008-12-09 11:32 ` Jarek Poplawski
2008-12-09 12:25 ` Patrick McHardy
2008-12-09 13:08 ` Jarek Poplawski
2008-12-09 13:20 ` Patrick McHardy
2008-12-09 14:45 ` Jarek Poplawski
2008-12-09 14:56 ` Patrick McHardy [this message]
2008-12-10 10:52 ` [PATCH 8/6] " Jarek Poplawski
2009-01-12 10:17 ` [PATCH 8/6 resend] pkt_sched: sch_htb: Break all htb_do_events() after 2 jiffies Jarek Poplawski
2009-01-13 5:54 ` David Miller
2008-12-10 6:35 ` [PATCH 2/6] pkt_sched: sch_htb: Consider used jiffies in htb_dequeue() David Miller
2008-12-10 9:11 ` Jarek Poplawski
2008-12-10 9:14 ` David Miller
2008-12-10 9:35 ` [PATCH 7/6] " Jarek Poplawski
2008-12-10 14:38 ` Patrick McHardy
2008-12-16 23:57 ` David Miller
2008-12-17 7:03 ` Jarek Poplawski
2008-12-17 7:38 ` David Miller
2009-01-12 6:56 ` Patrick McHardy
2009-01-12 10:10 ` Jarek Poplawski
2009-01-12 10:22 ` Patrick McHardy
2009-01-12 11:08 ` Jarek Poplawski
2009-01-12 13:10 ` Patrick McHardy
2009-01-28 12:52 ` [PATCH net-next] pkt_sched: sch_htb: Warn on too many events Jarek Poplawski
2009-01-28 16:18 ` Patrick McHardy
2009-01-30 10:17 ` [PATCH 1/3 v2 " Jarek Poplawski
2009-02-01 9:13 ` David Miller
2009-01-30 10:17 ` [PATCH 2/3 " Jarek Poplawski
2009-02-01 9:13 ` David Miller
2009-01-30 10:17 ` [PATCH 3/3 " Jarek Poplawski
2009-02-01 9:13 ` David Miller
2009-01-28 13:23 ` [PATCH 7/6] Re: [PATCH 2/6] pkt_sched: sch_htb: Consider used jiffies in htb_dequeue() Jarek Poplawski
2009-01-28 16:20 ` Patrick McHardy
2009-01-12 10:29 ` Jarek Poplawski
2009-01-12 10:32 ` David Miller
2009-01-12 10:59 ` Jarek Poplawski
2009-01-12 11:04 ` David Miller
2009-01-12 10:16 ` [PATCH 7/6 resend] pkt_sched: sch_htb: Consider used jiffies in htb_do_events() Jarek Poplawski
2009-01-13 5:54 ` David Miller
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=493E8709.5070601@trash.net \
--to=kaber@trash$(echo .)net \
--cc=davem@davemloft$(echo .)net \
--cc=devik@cdi$(echo .)cz \
--cc=jarkao2@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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