Re: [PATCH 2/6] workqueue: add support for lazy workqueues

[Frederic Weisbecker <fweisbec@gmail.com>]

On Thu, Aug 20, 2009 at 12:20:00PM +0200, Jens Axboe wrote:
> Lazy workqueues are like normal workqueues, except they don't
> start a thread per CPU by default. Instead threads are started
> when they are needed, and exit when they have been idle for
> some time.
> 
> Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
> ---
>  include/linux/workqueue.h |    5 ++
>  kernel/workqueue.c        |  152 ++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 147 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> index f14e20e..b2dd267 100644
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -32,6 +32,7 @@ struct work_struct {
>  #ifdef CONFIG_LOCKDEP
>  	struct lockdep_map lockdep_map;
>  #endif
> +	unsigned int cpu;
>  };
>  
>  #define WORK_DATA_INIT()	ATOMIC_LONG_INIT(0)
> @@ -172,6 +173,7 @@ enum {
>  	WQ_F_SINGLETHREAD	= 1,
>  	WQ_F_FREEZABLE		= 2,
>  	WQ_F_RT			= 4,
> +	WQ_F_LAZY		= 8,
>  };
>  
>  #ifdef CONFIG_LOCKDEP
> @@ -198,6 +200,7 @@ enum {
>  	__create_workqueue((name), WQ_F_SINGLETHREAD | WQ_F_FREEZABLE)
>  #define create_singlethread_workqueue(name)	\
>  	__create_workqueue((name), WQ_F_SINGLETHREAD)
> +#define create_lazy_workqueue(name)	__create_workqueue((name), WQ_F_LAZY)
>  
>  extern void destroy_workqueue(struct workqueue_struct *wq);
>  
> @@ -211,6 +214,8 @@ extern int queue_delayed_work_on(int cpu, struct workqueue_struct *wq,
>  
>  extern void flush_workqueue(struct workqueue_struct *wq);
>  extern void flush_scheduled_work(void);
> +extern void workqueue_set_lazy_timeout(struct workqueue_struct *wq,
> +			unsigned long timeout);
>  
>  extern int schedule_work(struct work_struct *work);
>  extern int schedule_work_on(int cpu, struct work_struct *work);
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 02ba7c9..d9ccebc 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -61,11 +61,17 @@ struct workqueue_struct {
>  	struct list_head list;
>  	const char *name;
>  	unsigned int flags;	/* WQ_F_* flags */
> +	unsigned long lazy_timeout;
> +	unsigned int core_cpu;
>  #ifdef CONFIG_LOCKDEP
>  	struct lockdep_map lockdep_map;
>  #endif
>  };
>  
> +/* Default lazy workqueue timeout */
> +#define WQ_DEF_LAZY_TIMEOUT	(60 * HZ)
> +
> +
>  /* Serializes the accesses to the list of workqueues. */
>  static DEFINE_SPINLOCK(workqueue_lock);
>  static LIST_HEAD(workqueues);
> @@ -81,6 +87,8 @@ static const struct cpumask *cpu_singlethread_map __read_mostly;
>   */
>  static cpumask_var_t cpu_populated_map __read_mostly;
>  
> +static int create_workqueue_thread(struct cpu_workqueue_struct *cwq, int cpu);
> +
>  /* If it's single threaded, it isn't in the list of workqueues. */
>  static inline bool is_wq_single_threaded(struct workqueue_struct *wq)
>  {
> @@ -141,11 +149,29 @@ static void insert_work(struct cpu_workqueue_struct *cwq,
>  static void __queue_work(struct cpu_workqueue_struct *cwq,
>  			 struct work_struct *work)
>  {
> +	struct workqueue_struct *wq = cwq->wq;
>  	unsigned long flags;
>  
> -	spin_lock_irqsave(&cwq->lock, flags);
> -	insert_work(cwq, work, &cwq->worklist);
> -	spin_unlock_irqrestore(&cwq->lock, flags);
> +	/*
> +	 * This is a lazy workqueue and this particular CPU thread has
> +	 * exited. We can't create it from here, so add this work on our
> +	 * static thread. It will create this thread and move the work there.
> +	 */
> +	if ((wq->flags & WQ_F_LAZY) && !cwq->thread) {



Isn't this part racy? If a work has just been queued but the thread
hasn't had yet enough time to start until we get there...?


> +		struct cpu_workqueue_struct *__cwq;
> +
> +		local_irq_save(flags);
> +		__cwq = wq_per_cpu(wq, wq->core_cpu);
> +		work->cpu = smp_processor_id();
> +		spin_lock(&__cwq->lock);
> +		insert_work(__cwq, work, &__cwq->worklist);
> +		spin_unlock_irqrestore(&__cwq->lock, flags);
> +	} else {
> +		spin_lock_irqsave(&cwq->lock, flags);
> +		work->cpu = smp_processor_id();
> +		insert_work(cwq, work, &cwq->worklist);
> +		spin_unlock_irqrestore(&cwq->lock, flags);
> +	}
>  }