linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Daniel Walker <dwalker@codeaurora.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 20:46:01 +0200	[thread overview]
Message-ID: <4C191BE9.1060400@kernel.org> (raw)
In-Reply-To: <1276712547.9309.172.camel@m0nster>

Hello,

On 06/16/2010 08:22 PM, Daniel Walker wrote:
> There's so many different ways that threads can interact .. Can you
> imagine a thread waiting in userspace for something to complete in the
> kernel, that does actually happen pretty often ;) .
> 
> I was just now randomly trolling through drivers and found this one,
> drivers/spi/amba-pl022.c ..
> 
> It processes some data in the interrupt, but sometimes it offloads the
> processing to a workqueue from the interrupt (or tasklet) .. If for
> example I'm a userspace thread waiting for that data then I would have
> to wait for that workqueue to complete (and it's priority plays a major
> roll in when it completes).

Yeah, and it would wait that by flushing the work, right?  If the
waiting part is using completion or some other event notification,
you'll just need to update the driver so that the kernel can determine
who's waiting for what so that it can bump the waited one's priority.
Otherwise, the problem can't be solved.

>> It was about using wq for cpu intensive / RT stuff.  Linus said,
>>
>>   So stop arguing about irrelevancies. Nobody uses workqueues for RT
>>   or for CPU-intensive crap. It's not what they were designed for, or
>>   used for.
> 
> Which is not relevant to this discussion .. We're talking about
> re-prioritizing the workqueue threads. We're _not_ talking about
> workqueues designed specifically for real time purposes.

Well, it's somewhat related,

* Don't depend on works or workqueues for RT stuff.  It's not designed
  for that.

* If you really wanna solve the problem, please go ahead and _solve_
  it yourself.  (read the rest of the mail)

>> * fragile as hell
> 
> Changing the thread priorities shouldn't be fragile , if it is right now
> then the threads are broken .. Can you exaplin in which cases you've
> seen it being fragile?

Because the workqueue might just go away in the next release or other
unrelated work which shouldn't get high priority might be scheduled
there.  Maybe the name of the workqueue changes or it gets merged with
another workqueue.  Maybe it gets split.  Maybe the system suspends
and resumes and nobody knows that workers die and are created again
over those events.  Maybe the backend implementaiton changes so that
workers are pooled.

>> * depends heavily on unrelated implementation details
> 
> I have no idea what this means.

(continued) because all those are implementation details which are NOT
PART OF THE INTERFACE in any way.

>> * has extremely limited test coverage
> 
> Simple, just write tests.

Yeah, and test your few configurations with those,

>> * doesn't help progressing mainline at all
> 
> progressing where?

(continued) and other people experiencing the same problem will have
to do about the same thing and won't know whether there nice + pidof
will work with the next kernel upgrade.

Gee, I don't know.  These are pretty evident problems to me.  Aren't
they obvious?

>> That's exactly like grepping /proc/kallsyms to determine some feature
>> and claiming it's a feature whether the kernel intends it or not.
>> Sure, use it all you want.  Just don't expect it to be there on the
>> next release.
> 
> Your assume there's no value in changing the priorities which is wrong.
> Your assuming way to much . Changing the priorities is useful.

And you're assuming grepping /proc/kallsyms is not useful?  It's
useful in its adhoc unsupported hacky way.

>> You're basically saying that "I don't know how those inheritance
>> inversions are happening but if I turn these magic knobs they seem to
>> go away so I want those magic knobs".  Maybe the RT part of the code
>> shouldn't be depending on that many random things to begin with?  And
>> if there are actually things which are necessary, it's better idea to
>> solve it properly through identifying problem points and properly
>> inheriting priority instead of turning knobs until it somehow works?
> 
> I think your mis-interpreting me .. If I write a thread (in userspace)
> which I put into RT priorities I don't have a lot of control over what
> dependencies the kernel may put on my thread. Think from a users
> perspective not from a kernel developers perspective.
>
> I'm not saying changing a workqueue priority would be a final solution,
> but it is a way to prioritize things immediately and has worked in prior
> kernels up till your patches.

* Make the kernel or driver or whatever you use in the RT path track
  priority is the right thing to do.

* I'm very sorry I'm breaking your hacky workaround but seriously
  that's another problem to solve.  Let's talk about the problem
  itself instead of your hacky workaround.  (I think for most cases
  not using workqueue in RT path would be the right thing to do.)

>> If you wanna work on such things, be my guest.  I'll be happy to work
>> with you but please stop talking about setting priorities of
>> workqueues from userland.  That's just nuts.
> 
> You just don't understand it.. How can you expect your patches to go
> into mainline with this attitude toward usages you just don't
> understand?

I'll keep your doubts on mind but I'm really understanding what you're
saying.  You just don't understand that I understand and disagree. :-)

Thanks.

-- 
tejun

  reply	other threads:[~2010-06-16 18:46 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
2010-06-16 16:55                             ` Tejun Heo
2010-06-16 18:22                               ` Daniel Walker
2010-06-16 18:46                                 ` Tejun Heo [this message]
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=4C191BE9.1060400@kernel.org \
    --to=tj@kernel.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=dwalker@codeaurora.org \
    --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 \
    /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).