Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753479AbbEKJdC (ORCPT ); Mon, 11 May 2015 05:33:02 -0400 Received: from cn.fujitsu.com ([59.151.112.132]:12690 "EHLO heian.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1753433AbbEKJce (ORCPT ); Mon, 11 May 2015 05:32:34 -0400 X-IronPort-AV: E=Sophos;i="5.04,848,1406563200"; d="scan'208";a="91971903" From: Lai Jiangshan To: CC: Lai Jiangshan , Tejun Heo Subject: [PATCH 4/5] workqueue: don't expose workqueue_attrs to users Date: Mon, 11 May 2015 17:35:51 +0800 Message-ID: <1431336953-3260-5-git-send-email-laijs@cn.fujitsu.com> X-Mailer: git-send-email 2.1.0 In-Reply-To: <1431336953-3260-1-git-send-email-laijs@cn.fujitsu.com> References: <1431336953-3260-1-git-send-email-laijs@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [10.167.226.103] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 14805 Lines: 428 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. Signed-off-by: Lai Jiangshan --- include/linux/workqueue.h | 18 +---- kernel/workqueue.c | 173 ++++++++++++++++++---------------------------- 2 files changed, 70 insertions(+), 121 deletions(-) diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h index 4618dd6..32e7c4b 100644 --- a/include/linux/workqueue.h +++ b/include/linux/workqueue.h @@ -119,20 +119,6 @@ struct delayed_work { int cpu; }; -/* - * A struct for workqueue attributes. This can be used to change - * attributes of an unbound workqueue. - * - * Unlike other fields, ->no_numa isn't a property of a worker_pool. It - * only modifies how apply_workqueue_attrs() select pools and thus doesn't - * participate in pool hash calculations or equality comparisons. - */ -struct workqueue_attrs { - int nice; /* nice level */ - cpumask_var_t cpumask; /* allowed CPUs */ - bool no_numa; /* disable NUMA affinity */ -}; - static inline struct delayed_work *to_delayed_work(struct work_struct *work) { return container_of(work, struct delayed_work, work); @@ -420,10 +406,8 @@ __alloc_workqueue_key(const char *fmt, unsigned int flags, int max_active, extern void destroy_workqueue(struct workqueue_struct *wq); -struct workqueue_attrs *alloc_workqueue_attrs(gfp_t gfp_mask); -void free_workqueue_attrs(struct workqueue_attrs *attrs); int apply_workqueue_attrs(struct workqueue_struct *wq, - const struct workqueue_attrs *attrs); + const cpumask_var_t cpumask, int nice, bool numa); int workqueue_set_unbound_cpumask(cpumask_var_t cpumask); extern bool queue_work_on(int cpu, struct workqueue_struct *wq, diff --git a/kernel/workqueue.c b/kernel/workqueue.c index efd9a3a..b08df98 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -106,6 +106,20 @@ enum { }; /* + * A struct for workqueue attributes. This can be used to change + * attributes of an unbound workqueue. + * + * Unlike other fields, ->no_numa isn't a property of a worker_pool. It + * only modifies how apply_workqueue_attrs() select pools and thus doesn't + * participate in pool hash calculations or equality comparisons. + */ +struct workqueue_attrs { + int nice; /* nice level */ + cpumask_var_t cpumask; /* allowed CPUs */ + bool no_numa; /* disable NUMA affinity */ +}; + +/* * Structure fields follow one of the following exclusion rules. * * I: Modifiable by initialization/destruction paths and read-only for @@ -307,6 +321,8 @@ static bool workqueue_freezing; /* PL: have wqs started freezing? */ static cpumask_var_t wq_unbound_cpumask; /* PL: low level cpumask for all unbound wqs */ +static const int std_nice[NR_STD_WORKER_POOLS] = { 0, HIGHPRI_NICE_LEVEL }; + /* the per-cpu worker pools */ static DEFINE_PER_CPU_SHARED_ALIGNED(struct worker_pool [NR_STD_WORKER_POOLS], cpu_worker_pools); @@ -316,12 +332,6 @@ static DEFINE_IDR(worker_pool_idr); /* PR: idr of all pools */ /* PL: hash of all unbound pools keyed by pool->attrs */ static DEFINE_HASHTABLE(unbound_pool_hash, UNBOUND_POOL_HASH_ORDER); -/* I: attributes used when instantiating standard unbound pools on demand */ -static struct workqueue_attrs *unbound_std_wq_attrs[NR_STD_WORKER_POOLS]; - -/* I: attributes used when instantiating ordered pools on demand */ -static struct workqueue_attrs *ordered_wq_attrs[NR_STD_WORKER_POOLS]; - struct workqueue_struct *system_wq __read_mostly; EXPORT_SYMBOL(system_wq); struct workqueue_struct *system_highpri_wq __read_mostly; @@ -338,8 +348,6 @@ struct workqueue_struct *system_freezable_power_efficient_wq __read_mostly; EXPORT_SYMBOL_GPL(system_freezable_power_efficient_wq); static int worker_thread(void *__worker); -static void copy_workqueue_attrs(struct workqueue_attrs *to, - const struct workqueue_attrs *from); static void workqueue_sysfs_unregister(struct workqueue_struct *wq); #define CREATE_TRACE_POINTS @@ -3022,7 +3030,7 @@ EXPORT_SYMBOL_GPL(execute_in_process_context); * * Undo alloc_workqueue_attrs(). */ -void free_workqueue_attrs(struct workqueue_attrs *attrs) +static void free_workqueue_attrs(struct workqueue_attrs *attrs) { if (attrs) { free_cpumask_var(attrs->cpumask); @@ -3039,7 +3047,7 @@ void free_workqueue_attrs(struct workqueue_attrs *attrs) * * Return: The allocated new workqueue_attr on success. %NULL on failure. */ -struct workqueue_attrs *alloc_workqueue_attrs(gfp_t gfp_mask) +static struct workqueue_attrs *alloc_workqueue_attrs(gfp_t gfp_mask) { struct workqueue_attrs *attrs; @@ -3568,7 +3576,7 @@ static void apply_wqattrs_cleanup(struct apply_wqattrs_ctx *ctx) /* allocate the attrs and pwqs for later installation */ static struct apply_wqattrs_ctx * apply_wqattrs_prepare(struct workqueue_struct *wq, - const struct workqueue_attrs *attrs) + const cpumask_var_t cpumask, int nice, bool numa) { struct apply_wqattrs_ctx *ctx; struct workqueue_attrs *new_attrs; @@ -3588,8 +3596,9 @@ apply_wqattrs_prepare(struct workqueue_struct *wq, * If the user configured cpumask doesn't overlap with the * wq_unbound_cpumask, we fallback to the wq_unbound_cpumask. */ - copy_workqueue_attrs(new_attrs, attrs); - cpumask_and(new_attrs->cpumask, new_attrs->cpumask, wq_unbound_cpumask); + cpumask_and(new_attrs->cpumask, cpumask, wq_unbound_cpumask); + new_attrs->nice = nice; + new_attrs->no_numa = !numa; if (unlikely(cpumask_empty(new_attrs->cpumask))) cpumask_copy(new_attrs->cpumask, wq_unbound_cpumask); @@ -3604,15 +3613,14 @@ apply_wqattrs_prepare(struct workqueue_struct *wq, for_each_node(node) { ctx->pwq_tbl[node] = alloc_node_unbound_pwq(wq, ctx->dfl_pwq, - !attrs->no_numa, - node, -1, false); + numa, node, -1, + false); if (!ctx->pwq_tbl[node]) goto out_free; } /* save the user configured attrs and sanitize it. */ - copy_workqueue_attrs(new_attrs, attrs); - cpumask_and(new_attrs->cpumask, new_attrs->cpumask, cpu_possible_mask); + cpumask_and(new_attrs->cpumask, cpumask, cpu_possible_mask); ctx->attrs = new_attrs; ctx->wq = wq; @@ -3664,7 +3672,8 @@ static void apply_wqattrs_unlock(void) } static int apply_workqueue_attrs_locked(struct workqueue_struct *wq, - const struct workqueue_attrs *attrs) + const cpumask_var_t cpumask, + int nice, bool numa) { struct apply_wqattrs_ctx *ctx; int ret = -ENOMEM; @@ -3677,7 +3686,7 @@ static int apply_workqueue_attrs_locked(struct workqueue_struct *wq, if (WARN_ON((wq->flags & __WQ_ORDERED) && !list_empty(&wq->pwqs))) return -EINVAL; - ctx = apply_wqattrs_prepare(wq, attrs); + ctx = apply_wqattrs_prepare(wq, cpumask, nice, numa); /* the ctx has been prepared successfully, let's commit it */ if (ctx) { @@ -3693,11 +3702,13 @@ static int apply_workqueue_attrs_locked(struct workqueue_struct *wq, /** * apply_workqueue_attrs - apply new workqueue_attrs to an unbound workqueue * @wq: the target workqueue - * @attrs: the workqueue_attrs to apply, allocated with alloc_workqueue_attrs() + * @cpumask: allowed cpumask for the workers for the target workqueue + * @nice: the ncie value for the workers for the target workqueue + * @numa: enable/disable per NUMA node pwqs * - * Apply @attrs to an unbound workqueue @wq. Unless disabled, on NUMA + * Apply the attrs to an unbound workqueue @wq. Unless disabled, on NUMA * machines, this function maps a separate pwq to each NUMA node with - * possibles CPUs in @attrs->cpumask so that work items are affine to the + * possibles CPUs in @cpumask so that work items are affine to the * NUMA node it was issued on. Older pwqs are released as in-flight work * items finish. Note that a work item which repeatedly requeues itself * back-to-back will stay on its current pwq. @@ -3707,12 +3718,12 @@ static int apply_workqueue_attrs_locked(struct workqueue_struct *wq, * Return: 0 on success and -errno on failure. */ int apply_workqueue_attrs(struct workqueue_struct *wq, - const struct workqueue_attrs *attrs) + const cpumask_var_t cpumask, int nice, bool numa) { int ret; apply_wqattrs_lock(); - ret = apply_workqueue_attrs_locked(wq, attrs); + ret = apply_workqueue_attrs_locked(wq, cpumask, nice, numa); apply_wqattrs_unlock(); return ret; @@ -3787,14 +3798,16 @@ static int alloc_and_link_pwqs(struct workqueue_struct *wq) } return 0; } else if (wq->flags & __WQ_ORDERED) { - ret = apply_workqueue_attrs(wq, ordered_wq_attrs[highpri]); + ret = apply_workqueue_attrs(wq, cpu_possible_mask, + std_nice[highpri], false); /* there should only be single pwq for ordering guarantee */ WARN(!ret && (wq->pwqs.next != &wq->dfl_pwq->pwqs_node || wq->pwqs.prev != &wq->dfl_pwq->pwqs_node), "ordering guarantee broken for workqueue %s\n", wq->name); return ret; } else { - return apply_workqueue_attrs(wq, unbound_std_wq_attrs[highpri]); + return apply_workqueue_attrs(wq, cpu_possible_mask, + std_nice[highpri], true); } } @@ -4764,7 +4777,9 @@ static int workqueue_apply_unbound_cpumask(void) if (wq->flags & __WQ_ORDERED) continue; - ctx = apply_wqattrs_prepare(wq, wq->unbound_attrs); + ctx = apply_wqattrs_prepare(wq, wq->unbound_attrs->cpumask, + wq->unbound_attrs->nice, + !wq->unbound_attrs->no_numa); if (!ctx) { ret = -ENOMEM; break; @@ -4923,43 +4938,22 @@ static ssize_t wq_nice_show(struct device *dev, struct device_attribute *attr, return written; } -/* prepare workqueue_attrs for sysfs store operations */ -static struct workqueue_attrs *wq_sysfs_prep_attrs(struct workqueue_struct *wq) -{ - struct workqueue_attrs *attrs; - - attrs = alloc_workqueue_attrs(GFP_KERNEL); - if (!attrs) - return NULL; - - mutex_lock(&wq->mutex); - copy_workqueue_attrs(attrs, wq->unbound_attrs); - mutex_unlock(&wq->mutex); - return attrs; -} - static ssize_t wq_nice_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct workqueue_struct *wq = dev_to_wq(dev); - struct workqueue_attrs *attrs; - int ret = -ENOMEM; + int nice, ret; - apply_wqattrs_lock(); - - attrs = wq_sysfs_prep_attrs(wq); - if (!attrs) - goto out_unlock; - if (sscanf(buf, "%d", &attrs->nice) == 1 && - attrs->nice >= MIN_NICE && attrs->nice <= MAX_NICE) - ret = apply_workqueue_attrs_locked(wq, attrs); - else - ret = -EINVAL; + if (!(sscanf(buf, "%d", &nice) == 1 && + nice >= MIN_NICE && nice <= MAX_NICE)) + return -EINVAL; -out_unlock: + apply_wqattrs_lock(); + ret = apply_workqueue_attrs_locked(wq, wq->unbound_attrs->cpumask, + nice, !wq->unbound_attrs->no_numa); apply_wqattrs_unlock(); - free_workqueue_attrs(attrs); + return ret ?: count; } @@ -4981,22 +4975,21 @@ static ssize_t wq_cpumask_store(struct device *dev, const char *buf, size_t count) { struct workqueue_struct *wq = dev_to_wq(dev); - struct workqueue_attrs *attrs; - int ret = -ENOMEM; - - apply_wqattrs_lock(); - - attrs = wq_sysfs_prep_attrs(wq); - if (!attrs) - goto out_unlock; + cpumask_var_t cpumask; + int ret; - ret = cpumask_parse(buf, attrs->cpumask); - if (!ret) - ret = apply_workqueue_attrs_locked(wq, attrs); + if (!alloc_cpumask_var(&cpumask, GFP_KERNEL)) + return -ENOMEM; + ret = cpumask_parse(buf, cpumask); + if (ret) + return ret; -out_unlock: + apply_wqattrs_lock(); + ret = apply_workqueue_attrs_locked(wq, cpumask, wq->unbound_attrs->nice, + !wq->unbound_attrs->no_numa); apply_wqattrs_unlock(); - free_workqueue_attrs(attrs); + + free_cpumask_var(cpumask); return ret ?: count; } @@ -5018,24 +5011,16 @@ static ssize_t wq_numa_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct workqueue_struct *wq = dev_to_wq(dev); - struct workqueue_attrs *attrs; - int v, ret = -ENOMEM; + int v, ret; - apply_wqattrs_lock(); - - attrs = wq_sysfs_prep_attrs(wq); - if (!attrs) - goto out_unlock; - - ret = -EINVAL; - if (sscanf(buf, "%d", &v) == 1) { - attrs->no_numa = !v; - ret = apply_workqueue_attrs_locked(wq, attrs); - } + if (sscanf(buf, "%d", &v) != 1) + return -EINVAL; -out_unlock: + apply_wqattrs_lock(); + ret = apply_workqueue_attrs_locked(wq, wq->unbound_attrs->cpumask, + wq->unbound_attrs->nice, !!v); apply_wqattrs_unlock(); - free_workqueue_attrs(attrs); + return ret ?: count; } @@ -5237,7 +5222,6 @@ static void __init wq_numa_init(void) static int __init init_workqueues(void) { - int std_nice[NR_STD_WORKER_POOLS] = { 0, HIGHPRI_NICE_LEVEL }; int i, cpu; WARN_ON(__alignof__(struct pool_workqueue) < __alignof__(long long)); @@ -5281,25 +5265,6 @@ static int __init init_workqueues(void) } } - /* create default unbound and ordered wq attrs */ - for (i = 0; i < NR_STD_WORKER_POOLS; i++) { - struct workqueue_attrs *attrs; - - BUG_ON(!(attrs = alloc_workqueue_attrs(GFP_KERNEL))); - attrs->nice = std_nice[i]; - unbound_std_wq_attrs[i] = attrs; - - /* - * An ordered wq should have only one pwq as ordering is - * guaranteed by max_active which is enforced by pwqs. - * Turn off NUMA so that dfl_pwq is used for all nodes. - */ - BUG_ON(!(attrs = alloc_workqueue_attrs(GFP_KERNEL))); - attrs->nice = std_nice[i]; - attrs->no_numa = true; - ordered_wq_attrs[i] = attrs; - } - system_wq = alloc_workqueue("events", 0, 0); system_highpri_wq = alloc_workqueue("events_highpri", WQ_HIGHPRI, 0); system_long_wq = alloc_workqueue("events_long", 0, 0); -- 2.1.0 -- 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/