netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: David Miller <davem@davemloft.net>
Cc: tglx@linutronix.de, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, kaber@trash.net,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [patch 1/3] net: serialize hrtimer callback in sched_cbq
Date: Tue, 14 Jul 2009 18:28:10 +0200	[thread overview]
Message-ID: <1247588890.7500.186.camel@twins> (raw)
In-Reply-To: <20090714.090055.56906831.davem@davemloft.net>

On Tue, 2009-07-14 at 09:00 -0700, David Miller wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> Date: Tue, 14 Jul 2009 10:55:14 +0200 (CEST)

> > On Sun, 12 Jul 2009, David Miller wrote:
> > 
> >> What should probably happen is that the hrtimer merely fires off work
> >> at software interrupt context (perhaps a tasklet or similar), and that
> >> software interrupt code take the qdisc's root lock throughout it's
> >> execution.
> > 
> > Sigh, I almost expected that the removal of the callback modes will
> > fire back some day.
> 
> Well this makes hrtimers decidedly less useful for networking and we
> have a ton of bugs right now, basically in every hrtimer used by the
> networking currently.
> 
> The only way we can use them, as things currently stand, is as
> triggers for softirq work.
> 
> Is it really that troublesome to provide this kind of facility
> generically, rather than having various subsystems replicate such code
> where they want to use hrtimers and are restricted to softirqs?

Linus really hated the softirq mode, which is what prompted me to change
that.

Now, it might be he only hated the particular interface and the
resulting code, but I think to remember he simply thought the whole
thing daft.

I can look into adding it back if we can agree on the interface and code
impact, but looking at:

# git grep hrtimer_init net/ | sort -u
net/can/bcm.c:          hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
net/can/bcm.c:          hrtimer_init(&op->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
net/sched/sch_api.c:    hrtimer_init(&wd->timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
net/sched/sch_cbq.c:    hrtimer_init(&q->delay_timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);

I wonder if its worth the impact on the core kernel code, or whether its
better for these few timers to kick off a tasklet or the like.

Further, I don't think a lot of subsystems would need this, as the
general trend is away from softirqs/tasklets and towards
threads/workqueues as most people want to schedule. And for those
hardirq hrtimers are good enough as a wakeup source.

  reply	other threads:[~2009-07-14 16:28 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-09 21:59 [patch 0/3] net: Sanitizing hrtimer usage in net/sched/sch_cbq.c Thomas Gleixner
2009-07-09 21:59 ` [patch 1/3] net: serialize hrtimer callback in sched_cbq Thomas Gleixner
2009-07-12 20:55   ` David Miller
2009-07-14  8:22     ` Patrick McHardy
2009-07-14  8:30       ` Peter Zijlstra
2009-07-14 16:01       ` David Miller
2009-07-14  8:55     ` Thomas Gleixner
2009-07-14 16:00       ` David Miller
2009-07-14 16:28         ` Peter Zijlstra [this message]
2009-07-14 16:42           ` Linus Torvalds
2009-07-17 12:14             ` Peter Zijlstra
2009-07-17 13:26               ` Oliver Hartkopp
2009-07-17 15:44               ` Linus Torvalds
2009-07-22  3:18               ` David Miller
2009-07-22  6:29                 ` Peter Zijlstra
2009-07-22 12:28                 ` [PATCH] softirq: tasklet_hrtimer Peter Zijlstra
2009-07-22 15:39                   ` David Miller
2009-07-22 16:01                   ` Linus Torvalds
2009-07-15  9:56         ` [patch 1/3] net: serialize hrtimer callback in sched_cbq Oliver Hartkopp
2009-07-09 21:59 ` [patch 2/3] net: sanitize hrtimer usage " Thomas Gleixner
2009-07-09 21:59 ` [patch 3/3] net: use HRTIMER_RESTART " Thomas Gleixner
2009-07-10  0:39 ` [patch 0/3] net: Sanitizing hrtimer usage in net/sched/sch_cbq.c David Miller
2009-07-12 20:57 ` 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=1247588890.7500.186.camel@twins \
    --to=peterz@infradead.org \
    --cc=davem@davemloft.net \
    --cc=kaber@trash.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.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).