lkml.org 
[lkml]   [2005]   [Aug]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [linux-pm] [PATCH] Workqueue freezer support.
    From
    Date
    Hi.

    Sorry for the slow response. Busy still.

    On Sat, 2005-08-06 at 15:06, Patrick Mochel wrote:
    > On Fri, 5 Aug 2005, Nigel Cunningham wrote:
    >
    > > Hi.
    > >
    > > I finally found some time to finish this off. I don't really like the
    > > end result - the macros looked clearer to me - but here goes. If it
    > > looks okay, I'll seek sign offs from each of the affected driver
    > > maintainers and from Ingo. Anyone else?
    >
    > What are your feelings about this: http://lwn.net/Articles/145417/ ?

    I'm sure it could work, but I do worry a little about the possibilities
    for exploits. It seems to me that if someone can get root, they an
    insmod a module that could schedule any kind of work via any process.
    Tracing that sort of security hole could be intractable. Christoph, is
    that something you've considered/have thoughts on? Perhaps I'm just
    being paranoid :>

    > It seems like a cleaner way to do things. How would these patches change
    > if that were merged?

    We would still need some way to mark which threads to freeze, so we'd
    still need the same sort of thing as far as kthread_run etc goes.

    We'd also still mark unfreezable threads with NOFREEZE and not mark
    other threads, so no change there.

    The first substantial change I can see would be switching from
    try_to_freeze to try_todo_list (could this be 'run_todo_list'? Try
    implies failure is possible - if it is possible, why do we ignore
    whether it fails?).

    The second substantial change would be in the area of the refrigeration
    code itself, ala Christoph's patch.

    One more question regarding Christoph's patch - would it be worth moving
    the refrigerator related code from sched.h to a separate file? We
    already have so many things that depend on sched.h, and this will add
    more. It will also make maintenance less painful because you won't have
    to recompile everything that depends on sched.h when all you did was
    modify (say) freezing().

    > Concerning the patch specifically:
    >
    > > diff -ruNp 400-workthreads.patch-old/include/linux/kthread.h 400-workthreads.patch-new/include/linux/kthread.h
    > > --- 400-workthreads.patch-old/include/linux/kthread.h 2004-11-03 21:51:12.000000000 +1100
    > > +++ 400-workthreads.patch-new/include/linux/kthread.h 2005-08-03 11:52:01.000000000 +1000
    > > @@ -23,10 +23,20 @@
    > > *
    > > * Returns a task_struct or ERR_PTR(-ENOMEM).
    > > */
    > > +struct task_struct *__kthread_create(int (*threadfn)(void *data),
    > > + void *data,
    > > + unsigned long freezer_flags,
    > > + const char namefmt[],
    > > + va_list * args);
    > > +
    >
    > When comparing this to this:
    >
    > > diff -ruNp 400-workthreads.patch-old/include/linux/workqueue.h 400-workthreads.patch-new/include/linux/workqueue.h
    > > --- 400-workthreads.patch-old/include/linux/workqueue.h 2005-06-20 11:47:30.000000000 +1000
    > > +++ 400-workthreads.patch-new/include/linux/workqueue.h 2005-08-03 11:49:34.000000000 +1000
    > > @@ -51,9 +51,12 @@ struct work_struct {
    > > } while (0)
    > >
    > > extern struct workqueue_struct *__create_workqueue(const char *name,
    > > - int singlethread);
    > > -#define create_workqueue(name) __create_workqueue((name), 0)
    > > -#define create_singlethread_workqueue(name) __create_workqueue((name), 1)
    > > + int singlethread,
    > > + unsigned long freezer_flag);
    > > +#define create_workqueue(name) __create_workqueue((name), 0, 0)
    > > +#define create_nofreeze_workqueue(name) __create_workqueue((name), 0, PF_NOFREEZE)
    > > +#define create_singlethread_workqueue(name) __create_workqueue((name), 1, 0)
    > > +#define create_nofreeze_singlethread_workqueue(name) __create_workqueue((name), 1, PF_NOFREEZE)
    >
    > And to this:
    >
    >
    > > static struct task_struct *create_workqueue_thread(struct workqueue_struct *wq,
    > > - int cpu)
    > > + int cpu,
    > > + unsigned long freezer_flags)
    > > {
    >
    > There is a slight discrepancy in the API changes. Obviously, we don't want
    > to change every caller of create_workqueue() to support this. But, perhaps
    > you could standardize the handling of the freezer flags (by e.g. creating
    > a separate _nofreeze version for each).

    Missed that one, sorry.

    > Also, functions that take a va_list parameter pass the structure (array)
    > rather than the pointer. See the definition of vprintk() in
    > include/linux/kernel.h for an example.

    Thanks for the pointer.

    Nigel

    >
    > Thanks,
    >
    >
    > Pat
    >
    > ______________________________________________________________________
    > _______________________________________________
    > linux-pm mailing list
    > linux-pm@lists.osdl.org
    > https://lists.osdl.org/mailman/listinfo/linux-pm
    --
    Evolution.
    Enumerate the requirements.
    Consider the interdependencies.
    Calculate the probabilities.

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-08-08 02:49    [W:0.027 / U:0.608 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site