linux-kernel - Re: [RFC PATCH] workqueue: Add support for exposing singlethread workqueues in sysfs

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <X75Ymvo1AhSLAKNP@mtj.duckdns.org>
Date:   Wed, 25 Nov 2020 08:14:02 -0500
From:   Tejun Heo <tj@...nel.org>
To:     Tariq Toukan <tariqt@...dia.com>
Cc:     Lai Jiangshan <jiangshanlai@...il.com>,
        linux-kernel@...r.kernel.org, Saeed Mahameed <saeedm@...dia.com>,
        Eran Ben Elisha <eranbe@...dia.com>,
        Moshe Shemesh <moshe@...dia.com>, netdev@...r.kernel.org
Subject: Re: [RFC PATCH] workqueue: Add support for exposing singlethread
 workqueues in sysfs

Hello,

This generally looks fine to me. Some nits below.

On Tue, Oct 06, 2020 at 03:06:07PM +0300, Tariq Toukan wrote:
> @@ -432,6 +433,9 @@ struct workqueue_struct *alloc_workqueue(const char *fmt,
>  			WQ_MEM_RECLAIM, 1, (name))
>  #define create_singlethread_workqueue(name)				\
>  	alloc_ordered_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, name)
> +#define create_singlethread_sysfs_workqueue(name)			\
> +	alloc_ordered_workqueue("%s", __WQ_MAX_ACTIVE_RO |		\
> +				__WQ_LEGACY | WQ_MEM_RECLAIM, name)

Please don't add a new wrapper. Just convert the user to call
alloc_ordered_workqueue() directly. I don't think we need __WQ_MAX_ACTIVE_RO
as a separate flag. The behavior can be implied in __WQ_ORDERED_EXPLICIT,
and __WQ_LEGACY is there just to disable dependency check because users of
older interace aren't marking MEM_RECLAIM correctly.

> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index c41c3c17b86a..a80d34726e68 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -4258,6 +4258,9 @@ struct workqueue_struct *alloc_workqueue(const char *fmt,
>  	if ((flags & WQ_POWER_EFFICIENT) && wq_power_efficient)
>  		flags |= WQ_UNBOUND;
>  
> +	if (flags & __WQ_MAX_ACTIVE_RO)
> +		flags |= WQ_SYSFS;

Just let the user set WQ_SYSFS like other users?

Thanks.

-- 
tejun