linux-kernel - Re: [PATCH v2.6.36-rc7 2/2] isdn/capi: make kcapi use a separate workqueue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Sat, 16 Oct 2010 13:00:53 +0200
From:	Jan Kiszka <jan.kiszka@....de>
To:	Tejun Heo <tj@...nel.org>
CC:	Karsten Keil <isdn@...ux-pingi.de>,
	Armin Schindler <mac@...ware.de>,
	lkml <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH v2.6.36-rc7 2/2] isdn/capi: make kcapi use a separate
 workqueue

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@...nel.org>
> ---
>  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@....de>


Download attachment "signature.asc" of type "application/pgp-signature" (260 bytes)