Date: Mon, 11 May 2015 10:59:52 -0400
From: Tejun Heo <tj@kernel.org>
To: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/5] workqueue: don't expose workqueue_attrs to users
Message-ID: <20150511145952.GE11388@htj.duckdns.org>
References: <1431336953-3260-1-git-send-email-laijs@cn.fujitsu.com>
 <1431336953-3260-5-git-send-email-laijs@cn.fujitsu.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1431336953-3260-5-git-send-email-laijs@cn.fujitsu.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1815
Lines: 48

On Mon, May 11, 2015 at 05:35:51PM +0800, Lai Jiangshan wrote:
> workqueue_attrs is an internal-like structure and is exposed with
> apply_workqueue_attrs() whose user has to investigate the structure
> before use.
> 
> And the apply_workqueue_attrs() API is inconvenient with the structure.
> The user (although there is no user yet currently) has to assemble
> several LoC to use:
> 	attrs = alloc_workqueue_attrs();
> 	if (!attrs)
> 		return;
> 	attrs->nice = ...;
> 	copy cpumask;
> 	attrs->no_numa = ...;
> 	apply_workqueue_attrs();
> 	free_workqueue_attrs();
> 
> It is too elaborate. This patch changes apply_workqueue_attrs() API,
> and one-line-code is enough to be called from user:
> 	apply_workqueue_attrs(wq, cpumask, nice, numa);
> 
> This patch also reduces the code of workqueue.c, about -50 lines.
> wq_sysfs_prep_attrs() is removed, wq_[nice|cpumask|numa]_store()
> directly access to the ->unbound_attrs with the protection
> of apply_wqattrs_lock();
> 
> This patch is also a preparation patch of next patch which
> remove no_numa out from the structure workqueue_attrs which
> requires apply_workqueue_attrs() has an argument to pass numa affinity.

I'm not sure about this.  Yeah, sure, it's a bit more lines of code
but at the same time this'd allow us to make the public interface
atomic too.  What we prolly should do is changing the interface so
that we do

	attrs = prepare_workqueue_attrs(gfp_mask);	/* allocate, lock & copy */
	/* modify attrs as desired */
	commit_workqueue_attrs(attrs);			/* apply, unlock and free */

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/