Re: [PATCH v2.6.36-rc7 2/2] isdn/capi: make kcapi use a separateworkqueue

[Jan Kiszka]

Am 15.10.2010 14:43, Tejun Heo wrote:
> flush_scheduled_work() is deprecated and will be removed.  Because
> kcapi uses fire-and-forget type works, it's impossible to flush each
> work explicitly.  Create and use a dedicated workqueue instead.
> 
> Please note that with recent workqueue changes, each workqueue doesn't
> reserve a lot of resources and using it as a flush domain is fine.
> 
> Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>
> ---
>  drivers/isdn/capi/kcapi.c |   26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> Index: work/drivers/isdn/capi/kcapi.c
> ===================================================================
> --- work.orig/drivers/isdn/capi/kcapi.c
> +++ work/drivers/isdn/capi/kcapi.c
> @@ -38,6 +38,7 @@
>  #include <linux/rcupdate.h>
> 
>  static int showcapimsgs = 0;
> +static struct workqueue_struct *kcapi_wq;
> 
>  MODULE_DESCRIPTION("CAPI4Linux: kernel CAPI layer");
>  MODULE_AUTHOR("Carsten Paeth");
> @@ -282,7 +283,7 @@ static int notify_push(unsigned int even
>  	event->type = event_type;
>  	event->controller = controller;
> 
> -	schedule_work(&event->work);
> +	queue_work(kcapi_wq, &event->work);
>  	return 0;
>  }
> 
> @@ -399,7 +400,7 @@ void capi_ctr_handle_message(struct capi
>  		goto error;
>  	}
>  	skb_queue_tail(&ap->recv_queue, skb);
> -	schedule_work(&ap->recv_work);
> +	queue_work(kcapi_wq, &ap->recv_work);
>  	rcu_read_unlock();
> 
>  	return;
> @@ -734,7 +735,7 @@ u16 capi20_release(struct capi20_appl *a
> 
>  	mutex_unlock(&capi_controller_lock);
> 
> -	flush_scheduled_work();
> +	flush_workqueue(kcapi_wq);
>  	skb_queue_purge(&ap->recv_queue);
> 
>  	if (showcapimsgs & 1) {
> @@ -1276,21 +1277,30 @@ static int __init kcapi_init(void)
>  {
>  	int err;
> 
> +	kcapi_wq = alloc_workqueue("kcapi", 0, 0);
> +	if (!kcapi_wq)
> +		return -ENOMEM;
> +
>  	register_capictr_notifier(&capictr_nb);
> 
>  	err = cdebug_init();
> -	if (!err)
> -		kcapi_proc_init();
> -	return err;
> +	if (err) {
> +		unregister_capictr_notifier(&capictr_nb);

Actually, this and the one on exit are unneeded as the notifier is
driven by this module itself. Anyway, they are also harmless. So let's
leave them in to avoid future confusions.

> +		destroy_workqueue(kcapi_wq);
> +		return err;
> +	}
> +
> +	kcapi_proc_init();
> +	return 0;
>  }
> 
>  static void __exit kcapi_exit(void)
>  {
>          kcapi_proc_exit();
> 
> -	/* make sure all notifiers are finished */
> -	flush_scheduled_work();
> +	unregister_capictr_notifier(&capictr_nb);
>  	cdebug_exit();
> +	destroy_workqueue(kcapi_wq);
>  }
> 
>  module_init(kcapi_init);

Looks and works fine.

Acked-by: Jan Kiszka <jan.kiszka@xxxxxx>

Attachment: signature.asc
Description: OpenPGP digital signature