Re: Fw: [PATCH -mm] workqueue: debug possible endless loop in cancel_rearming_delayed_work

[Jarek Poplawski <jarkao2@o2.pl>]

2 cents more...

On Tue, Apr 24, 2007 at 10:55:37PM +0400, Oleg Nesterov wrote:
...
> --- OLD/kernel/workqueue.c~1_CRDW	2007-04-13 17:43:23.000000000 +0400
> +++ OLD/kernel/workqueue.c	2007-04-24 22:41:15.000000000 +0400
> @@ -242,11 +242,11 @@ static void run_workqueue(struct cpu_wor
...
>  
> +static void needs_a_good_name(struct workqueue_struct *wq,

If you don't prefer something original, I think this
could be something like:

wait_on_work_[on_each | each_cpu | per_cpu] etc.

...
>  /**
> @@ -532,22 +534,34 @@ EXPORT_SYMBOL(flush_scheduled_work);
>  /**
>   * cancel_rearming_delayed_work - kill off a delayed work whose handler rearms the delayed work.
>   * @dwork: the delayed work struct
> - *
> - * Note that the work callback function may still be running on return from
> - * cancel_delayed_work(). Run flush_workqueue() or cancel_work_sync() to wait
> - * on it.

Probably there should be added a few words about changes.

>   */
>  void cancel_rearming_delayed_work(struct delayed_work *dwork)
>  {
> -	struct cpu_workqueue_struct *cwq = get_wq_data(&dwork->work);
> -
> -	/* Was it ever queued ? */
> -	if (cwq != NULL) {
> -		struct workqueue_struct *wq = cwq->wq;
> -
> -		while (!cancel_delayed_work(dwork))
> -			flush_workqueue(wq);
> -	}
> +	struct work_struct *work = &dwork->work;
> +	struct cpu_workqueue_struct *cwq = get_wq_data(work);
> +	int retry;
> +
> +	if (!cwq)
> +		return;
> +
> +	do {
> +		retry = 1;
> +		spin_lock_irq(&cwq->lock);
> +		/* CPU_DEAD in progress may change cwq */
> +		if (likely(cwq == get_wq_data(work))) {
> +			list_del_init(&work->entry);
> +			__set_bit(WORK_STRUCT_PENDING, work_data_bits(work));
> +			retry = try_to_del_timer_sync(&dwork->timer) < 0;
> +		}

		else
			retry = 0;

> +		spin_unlock_irq(&cwq->lock);
> +	} while (unlikely(retry));
...

Cheers,
Jarek P.