lkml.org 
[lkml]   [2009]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [git pull] x86 fixes
    * Andrew Morton <akpm@linux-foundation.org> wrote:

    > On Mon, 26 Jan 2009 20:20:16 +0100
    > Ingo Molnar <mingo@elte.hu> wrote:
    >
    > >
    > > * Andrew Morton <akpm@linux-foundation.org> wrote:
    > >
    > > > On Mon, 26 Jan 2009 18:17:23 +0100
    > > > Ingo Molnar <mingo@elte.hu> wrote:
    > > >
    > > > > Rusty Russell (2):
    > > > > ...
    > > > > work_on_cpu: Use our own workqueue.
    > > >
    > > > wtf?
    > >
    > > an x86 fix depends on it:
    > >
    > > 7285908: cpufreq: use work_on_cpu in acpi-cpufreq.c for drv_read and drv_write
    > >
    >
    > Right. And these are currently under active (albeit rather slow)
    > discussion.
    >
    > The changelogs suck, nobody can be assed actually telling us what the
    > bug is and the patches just casually toss yet another gaggle of kernel
    > threads into there.

    One problem is that for example do_dbs_timer() [used both in the
    cpufreq_ondemand and cpufreq_conservative cpufreq drivers] gets queued
    into the generic kevent workqueue via schedule_work(). But work_on_cpu()
    needs to serialize on the worklet - i.e. it needs to do a flush_work() -
    and does this with the cpufreq lock held.

    So we have a 'worklet inversion' bug here - and this got reported as a
    hard to debug boot hang on some systems.
    The root cause is that kevents is not that do_dbs_timer() uses
    schedule_work() - the root cause is that kevents workqueue is a too
    generic workqueue that is the union of all casual workqueue users in the
    kernel. That is fine (and it is its purpose) but it should not be used for
    core kernel facilities such as work_on_cpu() - precisely because doing so
    would limit that facility's genericity.

    This bug was always there but dormant until work_on_cpu() was used from a
    deep enough codepath.

    So the solution here is to isolate work_on_cpu()s mechanisms from the
    'misc' workqueue that schedule_work() deals with - this is what Rusty's
    patch does.

    Your observation about there being too many workqueue threads is correct
    but this commit is IMO a valid use of the workqueue facility. This
    workqueue it only gets created on CONFIG_SMP so there cost is about ~10K
    RAM per CPU.

    I also agree that the commit should have included this description.

    Ingo


    \
     
     \ /
      Last update: 2009-01-26 21:01    [W:2.384 / U:0.296 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site