All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: Alan Stern <stern@rowland.harvard.edu>,
	Ming Lei <ming.lei@redhat.com>,
	USB list <linux-usb@vger.kernel.org>,
	Kernel development list <linux-kernel@vger.kernel.org>
Subject: usb: don't offload isochronous urb completions to ksoftirq
Date: Tue, 12 Jun 2018 19:52:42 +0200	[thread overview]
Message-ID: <20180612175242.GA12855@kroah.com> (raw)

On Tue, Jun 12, 2018 at 01:19:28PM -0400, Mikulas Patocka wrote:
> 
> 
> On Tue, 12 Jun 2018, Alan Stern wrote:
> 
> > On Tue, 12 Jun 2018, Mikulas Patocka wrote:
> > 
> > > > How about making the softirq thread's priority adjustable?
> > > 
> > > But you would have to argue with softirq maintainers about it - and you 
> > > say that you don't have time for that.
> > 
> > But maybe _you_ do...
> 
> ksoftirqd has priority 0 - it is not suitable for real-time tasks, such as 
> audio.
> 
> In my opinion, it is much easier to fix this in the ehci driver (by not 
> offloading isochronous completions), than to design a new 
> real-time-capable ksoftirqd.

Ok, but what happens when you plug your device into a xhci controller?
Do we also need to change that?  Only touching a specific host
controller is not good, you will be playing "whack a mole" for forever.

Isoc packets are, by definition, not supposed to be guaranteed at all.
So if they are "slow" or dropped or delayed somehow, that's fine.  The
sound protocol should be fine with it.

Now yes, in reality, as you have found out, things can be "tight" on
low-powered processors under heavy load.  But what you are doing here is
a priority inversion.  You do not solve such a thing by going around and
raising everything else up as well, this is supposed to be a "general
purpose" kernel.  You can tune a specific machine/device just fine this
way, but not by messing with the kernel for the most part.

> > > > As for coordinating with the softirq maintainers -- whether I want to 
> > > > or not isn't the issue.  Right now I don't have _time_ to do it.
> > > > 
> > > > Alan Stern
> > > 
> > > I am wondering - whats the purpose of that patch 
> > > 428aac8a81058e2303677a8fbf26670229e51d3a at all? The patch shows some 
> > > performance difference, but they are minor, about 1%.
> > > 
> > > If you want to call the urb callback as soon as possible - why don't you 
> > > just call it? Why do you need to offload the callback to a softirq thread?
> > 
> > Please read the Changelog entry for commit 94dfd7edfd5c.  Basically the 
> > idea was to reduce overall latency by not doing as much work in an 
> > interrupt handler.
> > 
> > Alan Stern
> 
> snd_complete_urb is doing nothing but submitting the same urb again. Is 
> resubmitting the urb really causing so much latency that you can't do it 
> in the interrupt handler?

snd_complete_urb() does much more than just submition of the same urb.
Perhaps if this is a real problem, the sound driver should have more
than one urb pending?  Is there a pool here that is somehow getting used
up?

thanks,

greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: Alan Stern <stern@rowland.harvard.edu>,
	Ming Lei <ming.lei@redhat.com>,
	USB list <linux-usb@vger.kernel.org>,
	Kernel development list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] usb: don't offload isochronous urb completions to ksoftirq
Date: Tue, 12 Jun 2018 19:52:42 +0200	[thread overview]
Message-ID: <20180612175242.GA12855@kroah.com> (raw)
In-Reply-To: <alpine.LRH.2.02.1806121304530.1848@file01.intranet.prod.int.rdu2.redhat.com>

On Tue, Jun 12, 2018 at 01:19:28PM -0400, Mikulas Patocka wrote:
> 
> 
> On Tue, 12 Jun 2018, Alan Stern wrote:
> 
> > On Tue, 12 Jun 2018, Mikulas Patocka wrote:
> > 
> > > > How about making the softirq thread's priority adjustable?
> > > 
> > > But you would have to argue with softirq maintainers about it - and you 
> > > say that you don't have time for that.
> > 
> > But maybe _you_ do...
> 
> ksoftirqd has priority 0 - it is not suitable for real-time tasks, such as 
> audio.
> 
> In my opinion, it is much easier to fix this in the ehci driver (by not 
> offloading isochronous completions), than to design a new 
> real-time-capable ksoftirqd.

Ok, but what happens when you plug your device into a xhci controller?
Do we also need to change that?  Only touching a specific host
controller is not good, you will be playing "whack a mole" for forever.

Isoc packets are, by definition, not supposed to be guaranteed at all.
So if they are "slow" or dropped or delayed somehow, that's fine.  The
sound protocol should be fine with it.

Now yes, in reality, as you have found out, things can be "tight" on
low-powered processors under heavy load.  But what you are doing here is
a priority inversion.  You do not solve such a thing by going around and
raising everything else up as well, this is supposed to be a "general
purpose" kernel.  You can tune a specific machine/device just fine this
way, but not by messing with the kernel for the most part.

> > > > As for coordinating with the softirq maintainers -- whether I want to 
> > > > or not isn't the issue.  Right now I don't have _time_ to do it.
> > > > 
> > > > Alan Stern
> > > 
> > > I am wondering - whats the purpose of that patch 
> > > 428aac8a81058e2303677a8fbf26670229e51d3a at all? The patch shows some 
> > > performance difference, but they are minor, about 1%.
> > > 
> > > If you want to call the urb callback as soon as possible - why don't you 
> > > just call it? Why do you need to offload the callback to a softirq thread?
> > 
> > Please read the Changelog entry for commit 94dfd7edfd5c.  Basically the 
> > idea was to reduce overall latency by not doing as much work in an 
> > interrupt handler.
> > 
> > Alan Stern
> 
> snd_complete_urb is doing nothing but submitting the same urb again. Is 
> resubmitting the urb really causing so much latency that you can't do it 
> in the interrupt handler?

snd_complete_urb() does much more than just submition of the same urb.
Perhaps if this is a real problem, the sound driver should have more
than one urb pending?  Is there a pool here that is somehow getting used
up?

thanks,

greg k-h

             reply	other threads:[~2018-06-12 17:52 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-12 17:52 Greg Kroah-Hartman [this message]
2018-06-12 17:52 ` [PATCH] usb: don't offload isochronous urb completions to ksoftirq Greg Kroah-Hartman
  -- strict thread matches above, loose matches on Subject: below --
2018-06-15 21:13 Steven Rostedt
2018-06-15 21:13 ` High-priority softirqs [was: [PATCH] usb: don't offload isochronous urb completions to ksoftirq] Steven Rostedt
2018-06-15 21:05 usb: don't offload isochronous urb completions to ksoftirq Mikulas Patocka
2018-06-15 21:05 ` High-priority softirqs [was: [PATCH] usb: don't offload isochronous urb completions to ksoftirq] Mikulas Patocka
2018-06-15 20:54 usb: don't offload isochronous urb completions to ksoftirq Alan Stern
2018-06-15 20:54 ` High-priority softirqs [was: [PATCH] usb: don't offload isochronous urb completions to ksoftirq] Alan Stern
2018-06-15 17:40 usb: don't offload isochronous urb completions to ksoftirq Sebastian Andrzej Siewior
2018-06-15 17:40 ` High-priority softirqs [was: [PATCH] usb: don't offload isochronous urb completions to ksoftirq] Sebastian Sewior
2018-06-15 17:34 usb: don't offload isochronous urb completions to ksoftirq Steven Rostedt
2018-06-15 17:34 ` High-priority softirqs [was: [PATCH] usb: don't offload isochronous urb completions to ksoftirq] Steven Rostedt
2018-06-15 17:28 usb: don't offload isochronous urb completions to ksoftirq Thomas Gleixner
2018-06-15 17:28 ` High-priority softirqs [was: [PATCH] usb: don't offload isochronous urb completions to ksoftirq] Thomas Gleixner
2018-06-15 17:17 usb: don't offload isochronous urb completions to ksoftirq Alan Stern
2018-06-15 17:17 ` High-priority softirqs [was: [PATCH] usb: don't offload isochronous urb completions to ksoftirq] Alan Stern
2018-06-15 16:46 usb: don't offload isochronous urb completions to ksoftirq Steven Rostedt
2018-06-15 16:46 ` [PATCH] " Steven Rostedt
2018-06-15 16:41 Mikulas Patocka
2018-06-15 16:41 ` [PATCH] " Mikulas Patocka
2018-06-14 22:35 Steven Rostedt
2018-06-14 22:35 ` [PATCH] " Steven Rostedt
2018-06-14 22:23 Mikulas Patocka
2018-06-14 22:23 ` [PATCH] " Mikulas Patocka
2018-06-13 22:31 Steven Rostedt
2018-06-13 22:31 ` [PATCH] " Steven Rostedt
2018-06-13 19:30 Mikulas Patocka
2018-06-13 19:30 ` [PATCH] " Mikulas Patocka
2018-06-13 18:54 Alan Stern
2018-06-13 18:54 ` [PATCH] " Alan Stern
2018-06-13 16:35 Mikulas Patocka
2018-06-13 16:35 ` [PATCH] " Mikulas Patocka
2018-06-13 14:13 Alan Stern
2018-06-13 14:13 ` [PATCH] " Alan Stern
2018-06-13 13:57 Mikulas Patocka
2018-06-13 13:57 ` [PATCH] " Mikulas Patocka
2018-06-12 20:06 Alan Stern
2018-06-12 20:06 ` [PATCH] " Alan Stern
2018-06-12 19:03 Mikulas Patocka
2018-06-12 19:03 ` [PATCH] " Mikulas Patocka
2018-06-12 18:50 Mikulas Patocka
2018-06-12 18:50 ` [PATCH] " Mikulas Patocka
2018-06-12 18:44 Alan Stern
2018-06-12 18:44 ` [PATCH] " Alan Stern
2018-06-12 17:19 Mikulas Patocka
2018-06-12 17:19 ` [PATCH] " Mikulas Patocka
2018-06-12 16:38 Alan Stern
2018-06-12 16:38 ` [PATCH] " Alan Stern
2018-06-12 16:03 Mikulas Patocka
2018-06-12 16:03 ` [PATCH] " Mikulas Patocka
2018-06-12 15:11 Alan Stern
2018-06-12 15:11 ` [PATCH] " Alan Stern
2018-06-12 14:44 Mikulas Patocka
2018-06-12 14:44 ` [PATCH] " Mikulas Patocka
2018-06-12 14:38 Alan Stern
2018-06-12 14:38 ` [PATCH] " Alan Stern
2018-06-12 14:29 Mikulas Patocka
2018-06-12 14:29 ` [PATCH] " Mikulas Patocka

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=20180612175242.GA12855@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    --cc=mpatocka@redhat.com \
    --cc=stern@rowland.harvard.edu \
    /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.