Re: [PATCH 1/1] EDAC: fix edac core deadlock when removing a device

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 18 Dec 2008 13:09:18 +0800
Harry Ciao <qingtao.cao@windriver.com> wrote:

> To remove an edac device, all pending work must be complete. At that point, 
> it is safe to remove the edac_dev structure.
> 
> If the pending work is not properly cleared and proper care is not taken 
> when waiting for it's completion, the following stack trace result:
>
> ...
> 
> --- a/drivers/edac/edac_device.c
> +++ b/drivers/edac/edac_device.c
> @@ -394,6 +394,12 @@ static void edac_device_workq_function(struct work_struct *work_req)
>  
>  	mutex_lock(&device_ctls_mutex);
>  
> +	/* If we are being removed, bail out immediately */
> +	if (edac_dev->op_state == OP_OFFLINE) {
> +		mutex_unlock(&device_ctls_mutex);
> +		return;
> +	}
> +
>  	/* Only poll controllers that are running polled and have a check */
>  	if ((edac_dev->op_state == OP_RUNNING_POLL) &&
>  		(edac_dev->edac_check != NULL)) {
> @@ -585,14 +591,14 @@ struct edac_device_ctl_info *edac_device_del_device(struct device *dev)
>  	/* mark this instance as OFFLINE */
>  	edac_dev->op_state = OP_OFFLINE;
>  
> -	/* clear workq processing on this instance */
> -	edac_device_workq_teardown(edac_dev);
> -
>  	/* deregister from global list */
>  	del_edac_device_from_global_list(edac_dev);
>  
>  	mutex_unlock(&device_ctls_mutex);
>  
> +	/* clear workq processing on this instance */
> +	edac_device_workq_teardown(edac_dev);
> +
>  	/* Tear down the sysfs entries for this instance */
>  	edac_device_remove_sysfs(edac_dev);
>  

Is the change to edac_device_workq_function() necessary? 
edac_device_workq_teardown()'s call to cancel_delayed_work() will
ensure that edac_device_workq_function() isn't running.

Incidentally, edac_device_workq_teardown() is an open-coded
cancel_delayed_work_sync().