linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Walker <dwalker@codeaurora.org>
To: Tejun Heo <tj@kernel.org>
Cc: mingo@elte.hu, awalls@radix.net, linux-kernel@vger.kernel.org,
	jeff@garzik.org, akpm@linux-foundation.org,
	rusty@rustcorp.com.au, cl@linux-foundation.org,
	dhowells@redhat.com, arjan@linux.intel.com,
	johannes@sipsolutions.net, oleg@redhat.com, axboe@kernel.dk
Subject: Re: Overview of concurrency managed workqueue
Date: Wed, 16 Jun 2010 09:30:38 -0700	[thread overview]
Message-ID: <1276705838.9309.94.camel@m0nster> (raw)
In-Reply-To: <4C18F2B8.9060805@kernel.org>

On Wed, 2010-06-16 at 17:50 +0200, Tejun Heo wrote:
> Hello,
> 
> On 06/16/2010 05:11 PM, Daniel Walker wrote:
> >> If you find that some work item is causing priority inversion, you
> >> need to fix the problem instead of working around in adhoc way which
> >> won't be useful to anyone else, so, no, it doesn't sound like a useful
> >> use case.
> > 
> > How do I fix the problem then? Without doing what I've already
> > suggested.
> 
> I don't know.  I suppose a high priority thread is trying to flush a
> work item or workqueue thus causing priority inversion, right?  Maybe
> we can add high priority emergency worker which gets triggered through
> priority inversion detection or maybe the code shouldn't be flushing
> in the critical path anyway.

It's not a flushing situation .. The high priority thread is a userspace
thread so it , AFAIK, can't flush any workqueues.

> >> Peter brought up the work priority issue previously and Linus was
> >> pretty clear on the issue.  They're in the discussiosn for the first
> >> or second take.
> > 
> > Send us a link to this discussion.
> 
>   http://thread.gmane.org/gmane.linux.kernel/929641

I didn't see anything in there related to this discussion.

> >>> Your completely wrong .. Look at the example I gave above .. Bottom line
> >>> is that your removing currently useful functionality, which is bad.. You
> >>> can say it's not useful, but again you also say you don't "have much
> >>> idea" regarding the cases where this is actually important.
> >>
> >> I said that I didn't have much idea about RT work priority thing, not
> >> setting priority on wq workers for adhoc workaround for whatever.
> >> IIRC, the RT work priority thing Peter was talking about was a
> >> different thing.  Sure, more complex workqueue implementation would
> >> complicate implementing work priorities, but if necessary maybe we can
> >> grow different classes of worker pools.
> > 
> > Ok .. So what would these different classes look like then ? Is that
> > something I could prioritize from userspace perhaps ?
> 
> Maybe it's me not understanding something but I don't really think
> exposing workqueue priorities to userland is a good solution at all.

Why not? They currently are to no known ill effects (none that I know
of).

> >>> Could you please just entertain the idea that maybe someone somewhere
> >>> might want to give priorities to the work items inside your system.
> >>
> >> If that's necessary, do it properly.  Give *work* priorities or at
> >> least give explicit priorities to workqueues.  That's a completely
> >> different thing from insisting fixed workqueue to kthread mapping so
> >> that three people on the whole planet can set priority on those
> >> kthreads not even knowing what the hell they do.
> > 
> > You have no idea how many people are doing this, or in what
> > circumstances .. Please don't make mass speculation over things you
> > clearly are not aware of.
> 
> Well, then please stop insisting it to be a feature to keep.  It's not
> a feature.

It may not have been a feature in the past, but it's used as a feature
now.. So it is a feature even tho you don't want it to be.

> > I'm not insisting any fixed mapping, you need to open you mind to
> > _possibilities_ .. How can the work items be given priorities _inside
> > your system_! Can you give an interface in which people can set a
> > priority to a give type of work item, then maybe you system honors those
> > priorities in _some way_ ..
> > 
> > In fact, I'm only asking that you consider it, ponder it..
> 
> Oh yeah, if you're not insisting fixed mapping, then I don't have any
> problem with that.  As for what to do for priority inversions
> involving workqueues, I don't have any concrete idea (it was not in
> the mainline, so I didn't have to solve it) but one way would be
> reserving/creating temporary high priority workers and use them to
> process work items which the high priority thread is blocked on.

But it is in the mainline, that's why we're talking right now.

What I was thinking is that you could have a debugfs interface which
would list off what workqueues you system is processing and give the
user the ability to pull one or more of those workqueues into individual
threads for processing, just like it currently is. That way I can
prioritize the work items with out you having to give priorities through
your entire system.

The alternative is that you would give each work item a settable
priority and your whole system would have to honor that, which would be
a little like re-creating the scheduler.

> But, really, without knowing details of those inversion cases, it
> would be pretty difficult to design something which fits.  All that I
> can say is having shared worker pool isn't exclusive with solving the
> problem.

The cases are all in the mainline kernel, you just have to look at the
code in a different way to understand them .. If I have a userspace
thread at a high priority and I'm making calls into the kernel, some of
those call inevitably will put work items onto workqueues, right? I'm
sure you can think of 100's of ways in which this could happen .. At
that point my thread depends on the workqueue thread, since the
workqueue thread is doing processing for which I've , in some way,
requested from userspace.

Daniel


  reply	other threads:[~2010-06-16 16:30 UTC|newest]

Thread overview: 129+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-14 21:37 [PATCHSET] workqueue: concurrency managed workqueue, take#5 Tejun Heo
2010-06-14 21:37 ` [PATCH 01/30] kthread: implement kthread_data() Tejun Heo
2010-06-14 21:37 ` [PATCH 02/30] acpi: use queue_work_on() instead of binding workqueue worker to cpu0 Tejun Heo
2010-06-14 21:37 ` [PATCH 03/30] workqueue: kill RT workqueue Tejun Heo
2010-06-14 21:37 ` [PATCH 04/30] workqueue: misc/cosmetic updates Tejun Heo
2010-06-14 21:37 ` [PATCH 05/30] workqueue: merge feature parameters into flags Tejun Heo
2010-06-14 21:37 ` [PATCH 06/30] workqueue: define masks for work flags and conditionalize STATIC flags Tejun Heo
2010-06-14 21:37 ` [PATCH 07/30] workqueue: separate out process_one_work() Tejun Heo
2010-06-14 21:37 ` [PATCH 08/30] workqueue: temporarily disable workqueue tracing Tejun Heo
2010-06-15 13:29   ` Frederic Weisbecker
2010-06-15 16:37     ` Tejun Heo
2010-06-14 21:37 ` [PATCH 09/30] workqueue: kill cpu_populated_map Tejun Heo
2010-06-14 21:37 ` [PATCH 10/30] workqueue: update cwq alignement Tejun Heo
2010-06-14 21:37 ` [PATCH 11/30] workqueue: reimplement workqueue flushing using color coded works Tejun Heo
2010-06-14 21:37 ` [PATCH 12/30] workqueue: introduce worker Tejun Heo
2010-06-14 21:37 ` [PATCH 13/30] workqueue: reimplement work flushing using linked works Tejun Heo
2010-06-14 21:37 ` [PATCH 14/30] workqueue: implement per-cwq active work limit Tejun Heo
2010-06-14 21:37 ` [PATCH 15/30] workqueue: reimplement workqueue freeze using max_active Tejun Heo
2010-06-14 21:37 ` [PATCH 16/30] workqueue: introduce global cwq and unify cwq locks Tejun Heo
2010-06-14 21:37 ` [PATCH 17/30] workqueue: implement worker states Tejun Heo
2010-06-14 21:37 ` [PATCH 18/30] workqueue: reimplement CPU hotplugging support using trustee Tejun Heo
2010-06-14 21:37 ` [PATCH 19/30] workqueue: make single thread workqueue shared worker pool friendly Tejun Heo
2010-06-14 21:37 ` [PATCH 20/30] workqueue: add find_worker_executing_work() and track current_cwq Tejun Heo
2010-06-14 21:37 ` [PATCH 21/30] workqueue: carry cpu number in work data once execution starts Tejun Heo
2010-06-14 21:37 ` [PATCH 22/30] workqueue: implement WQ_NON_REENTRANT Tejun Heo
2010-06-14 21:37 ` [PATCH 23/30] workqueue: use shared worklist and pool all workers per cpu Tejun Heo
2010-06-14 21:37 ` [PATCH 24/30] workqueue: implement concurrency managed dynamic worker pool Tejun Heo
2010-06-14 21:37 ` [PATCH 25/30] workqueue: increase max_active of keventd and kill current_is_keventd() Tejun Heo
2010-06-14 21:37 ` [PATCH 26/30] workqueue: add system_wq, system_long_wq and system_nrt_wq Tejun Heo
2010-06-14 21:37 ` [PATCH 27/30] workqueue: implement DEBUGFS/workqueue Tejun Heo
2010-06-15 13:54   ` Frederic Weisbecker
2010-06-15 16:42     ` Tejun Heo
2010-06-14 21:37 ` [PATCH 28/30] workqueue: implement several utility APIs Tejun Heo
2010-06-14 21:37 ` [PATCH 29/30] libata: take advantage of cmwq and remove concurrency limitations Tejun Heo
2010-06-14 21:37 ` [PATCH 30/30] async: use workqueue for worker pool Tejun Heo
2010-06-14 21:58 ` [PATCHSET] workqueue: concurrency managed workqueue, take#5 Andrew Morton
2010-06-14 22:17   ` Tejun Heo
2010-06-14 22:31     ` Daniel Walker
2010-06-14 22:33       ` Tejun Heo
2010-06-14 22:35         ` Daniel Walker
2010-06-14 22:44           ` Tejun Heo
2010-06-14 22:49             ` Daniel Walker
2010-06-14 22:52               ` Tejun Heo
2010-06-14 22:35     ` Andrew Morton
2010-06-14 22:43       ` Tejun Heo
2010-06-14 23:06         ` Andrew Morton
2010-06-15 12:53         ` tytso
2010-06-15 16:15           ` [PATCH] SubmittingPatches: add more about patch descriptions Randy Dunlap
2010-06-15 16:33             ` Christoph Lameter
2010-06-15 18:15   ` [PATCHSET] workqueue: concurrency managed workqueue, take#5 Stefan Richter
2010-06-15 19:39     ` Tejun Heo
2010-06-15  1:20 ` Jeff Garzik
2010-06-15 18:25 ` Overview of concurrency managed workqueue Tejun Heo
2010-06-15 18:40   ` Christoph Lameter
2010-06-15 18:44     ` Tejun Heo
2010-06-15 19:43   ` Daniel Walker
2010-06-16 12:10     ` Tejun Heo
2010-06-16 13:27       ` Daniel Walker
2010-06-16 13:30         ` Tejun Heo
2010-06-16 13:41           ` Daniel Walker
2010-06-16 13:45             ` Tejun Heo
2010-06-16 14:05               ` Daniel Walker
2010-06-16 14:15                 ` Tejun Heo
2010-06-16 14:34                   ` Daniel Walker
2010-06-16 14:50                     ` Tejun Heo
2010-06-16 15:11                       ` Daniel Walker
2010-06-16 15:50                         ` Tejun Heo
2010-06-16 16:30                           ` Daniel Walker [this message]
2010-06-16 16:55                             ` Tejun Heo
2010-06-16 18:22                               ` Daniel Walker
2010-06-16 18:46                                 ` Tejun Heo
2010-06-16 19:20                                   ` Tejun Heo
2010-06-16 19:46                                     ` Daniel Walker
2010-06-16 19:58                                       ` Tejun Heo
2010-06-17  5:29                                     ` Florian Mickler
2010-06-17  6:21                                       ` Florian Mickler
2010-06-17  8:28                                       ` Tejun Heo
2010-06-17 18:03                                       ` Daniel Walker
2010-06-18  6:36                                         ` Florian Mickler
2010-06-18 16:38                                           ` Daniel Walker
2010-06-16 19:36                                   ` Daniel Walker
2010-06-16 19:52                                     ` Tejun Heo
2010-06-16 20:19                                       ` Daniel Walker
2010-06-16 20:24                                         ` Tejun Heo
2010-06-16 20:40                                           ` Daniel Walker
2010-06-16 21:41                                             ` Tejun Heo
2010-06-17 23:15                               ` Andrew Morton
2010-06-18  8:03                                 ` Tejun Heo
2010-06-18  8:22                                   ` Tejun Heo
2010-06-18 17:29                                   ` Daniel Walker
2010-06-16 18:31                     ` Stefan Richter
2010-06-16 18:41                       ` Daniel Walker
2010-06-17 12:01                 ` Andy Walls
2010-06-17 16:56                   ` Daniel Walker
2010-06-17 23:16                   ` Andrew Morton
2010-06-18  7:16                     ` Tejun Heo
2010-06-18  7:31                       ` Andrew Morton
2010-06-18  8:09                         ` Tejun Heo
2010-06-18 17:02                           ` Andrew Morton
2010-06-18 17:28                             ` Tejun Heo
2010-06-19 15:53                               ` [PATCH] kthread: implement kthread_worker Tejun Heo
2010-06-21 20:33                                 ` Randy Dunlap
2010-06-22  7:31                                   ` Tejun Heo
2010-06-19  8:38                   ` Overview of concurrency managed workqueue Andi Kleen
2010-06-19  8:40                     ` Tejun Heo
2010-06-19  8:55                       ` Andi Kleen
2010-06-19  9:01                         ` Tejun Heo
2010-06-19  9:08                           ` Andi Kleen
2010-06-19  9:12                             ` Tejun Heo
2010-06-19  9:15                               ` Andi Kleen
2010-06-19  9:17                                 ` Tejun Heo
2010-06-19  9:27                                   ` Andi Kleen
2010-06-19  9:42                                     ` Tejun Heo
2010-06-19 12:20                                       ` Andi Kleen
2010-06-19 12:48                                         ` Tejun Heo
2010-06-17 22:28               ` Daniel Walker
2010-06-16  6:55   ` Florian Mickler
2010-06-16 12:22     ` Tejun Heo
2010-06-16 13:37   ` Johannes Berg
2010-06-16 13:39     ` Tejun Heo
2010-06-16 13:42       ` Johannes Berg
2010-06-17 23:14   ` Andrew Morton
2010-06-17 23:25     ` Joel Becker
2010-06-17 23:56       ` Andrew Morton
2010-06-18  7:15         ` Tejun Heo
2010-06-18  7:31     ` Tejun Heo
2010-06-15 18:29 ` [PATCHSET] workqueue: concurrency managed workqueue, take#5 Stefan Richter
2010-06-15 18:40   ` Tejun Heo
2010-06-15 20:29   ` Stefan Richter

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=1276705838.9309.94.camel@m0nster \
    --to=dwalker@codeaurora.org \
    --cc=akpm@linux-foundation.org \
    --cc=arjan@linux.intel.com \
    --cc=awalls@radix.net \
    --cc=axboe@kernel.dk \
    --cc=cl@linux-foundation.org \
    --cc=dhowells@redhat.com \
    --cc=jeff@garzik.org \
    --cc=johannes@sipsolutions.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=oleg@redhat.com \
    --cc=rusty@rustcorp.com.au \
    --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 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).