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 <la...@cn.fujitsu.com>
---
 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to