Re: [linux-pm] [PATCH] Workqueue freezer support.

[Patrick Mochel <mochel@digitalimplant.org>]

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/ ?

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

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).

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,


	Pat
-
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/