Commit 13e2e556 authored by Tejun Heo's avatar Tejun Heo

workqueue: fix unbound workqueue attrs hashing / comparison

29c91e99 ("workqueue: implement attribute-based unbound worker_pool
management") implemented attrs based worker_pool matching.  It tried
to avoid false negative when comparing cpumasks with custom hash
function; unfortunately, the hash and comparison functions fail to
ignore CPUs which are not possible.  It incorrectly assumed that
bitmap_copy() skips leftover bits in the last word of bitmap and
cpumask_equal() ignores impossible CPUs.

This patch updates attrs->cpumask handling such that impossible CPUs
are properly ignored.

* Hash and copy functions no longer do anything special.  They expect
  their callers to clear impossible CPUs.

* alloc_workqueue_attrs() initializes the cpumask to cpu_possible_mask
  instead of setting all bits and explicit cpumask_setall() for
  unbound_std_wq_attrs[] in init_workqueues() is dropped.

* apply_workqueue_attrs() is now responsible for ignoring impossible
  CPUs.  It makes a copy of @attrs and clears impossible CPUs before
  doing anything else.
Signed-off-by: default avatarTejun Heo <tj@kernel.org>
parent bc0caf09
...@@ -3302,7 +3302,7 @@ struct workqueue_attrs *alloc_workqueue_attrs(gfp_t gfp_mask) ...@@ -3302,7 +3302,7 @@ struct workqueue_attrs *alloc_workqueue_attrs(gfp_t gfp_mask)
if (!alloc_cpumask_var(&attrs->cpumask, gfp_mask)) if (!alloc_cpumask_var(&attrs->cpumask, gfp_mask))
goto fail; goto fail;
cpumask_setall(attrs->cpumask); cpumask_copy(attrs->cpumask, cpu_possible_mask);
return attrs; return attrs;
fail: fail:
free_workqueue_attrs(attrs); free_workqueue_attrs(attrs);
...@@ -3316,33 +3316,14 @@ static void copy_workqueue_attrs(struct workqueue_attrs *to, ...@@ -3316,33 +3316,14 @@ static void copy_workqueue_attrs(struct workqueue_attrs *to,
cpumask_copy(to->cpumask, from->cpumask); cpumask_copy(to->cpumask, from->cpumask);
} }
/*
* Hacky implementation of jhash of bitmaps which only considers the
* specified number of bits. We probably want a proper implementation in
* include/linux/jhash.h.
*/
static u32 jhash_bitmap(const unsigned long *bitmap, int bits, u32 hash)
{
int nr_longs = bits / BITS_PER_LONG;
int nr_leftover = bits % BITS_PER_LONG;
unsigned long leftover = 0;
if (nr_longs)
hash = jhash(bitmap, nr_longs * sizeof(long), hash);
if (nr_leftover) {
bitmap_copy(&leftover, bitmap + nr_longs, nr_leftover);
hash = jhash(&leftover, sizeof(long), hash);
}
return hash;
}
/* hash value of the content of @attr */ /* hash value of the content of @attr */
static u32 wqattrs_hash(const struct workqueue_attrs *attrs) static u32 wqattrs_hash(const struct workqueue_attrs *attrs)
{ {
u32 hash = 0; u32 hash = 0;
hash = jhash_1word(attrs->nice, hash); hash = jhash_1word(attrs->nice, hash);
hash = jhash_bitmap(cpumask_bits(attrs->cpumask), nr_cpu_ids, hash); hash = jhash(cpumask_bits(attrs->cpumask),
BITS_TO_LONGS(nr_cpumask_bits) * sizeof(long), hash);
return hash; return hash;
} }
...@@ -3652,7 +3633,8 @@ static void init_and_link_pwq(struct pool_workqueue *pwq, ...@@ -3652,7 +3633,8 @@ static void init_and_link_pwq(struct pool_workqueue *pwq,
int apply_workqueue_attrs(struct workqueue_struct *wq, int apply_workqueue_attrs(struct workqueue_struct *wq,
const struct workqueue_attrs *attrs) const struct workqueue_attrs *attrs)
{ {
struct pool_workqueue *pwq, *last_pwq; struct workqueue_attrs *new_attrs;
struct pool_workqueue *pwq = NULL, *last_pwq;
struct worker_pool *pool; struct worker_pool *pool;
/* only unbound workqueues can change attributes */ /* only unbound workqueues can change attributes */
...@@ -3663,15 +3645,21 @@ int apply_workqueue_attrs(struct workqueue_struct *wq, ...@@ -3663,15 +3645,21 @@ int apply_workqueue_attrs(struct workqueue_struct *wq,
if (WARN_ON((wq->flags & __WQ_ORDERED) && !list_empty(&wq->pwqs))) if (WARN_ON((wq->flags & __WQ_ORDERED) && !list_empty(&wq->pwqs)))
return -EINVAL; return -EINVAL;
/* make a copy of @attrs and sanitize it */
new_attrs = alloc_workqueue_attrs(GFP_KERNEL);
if (!new_attrs)
goto enomem;
copy_workqueue_attrs(new_attrs, attrs);
cpumask_and(new_attrs->cpumask, new_attrs->cpumask, cpu_possible_mask);
pwq = kmem_cache_zalloc(pwq_cache, GFP_KERNEL); pwq = kmem_cache_zalloc(pwq_cache, GFP_KERNEL);
if (!pwq) if (!pwq)
return -ENOMEM; goto enomem;
pool = get_unbound_pool(attrs); pool = get_unbound_pool(new_attrs);
if (!pool) { if (!pool)
kmem_cache_free(pwq_cache, pwq); goto enomem;
return -ENOMEM;
}
init_and_link_pwq(pwq, wq, pool, &last_pwq); init_and_link_pwq(pwq, wq, pool, &last_pwq);
if (last_pwq) { if (last_pwq) {
...@@ -3681,6 +3669,11 @@ int apply_workqueue_attrs(struct workqueue_struct *wq, ...@@ -3681,6 +3669,11 @@ int apply_workqueue_attrs(struct workqueue_struct *wq,
} }
return 0; return 0;
enomem:
kmem_cache_free(pwq_cache, pwq);
free_workqueue_attrs(new_attrs);
return -ENOMEM;
} }
static int alloc_and_link_pwqs(struct workqueue_struct *wq) static int alloc_and_link_pwqs(struct workqueue_struct *wq)
...@@ -4450,10 +4443,7 @@ static int __init init_workqueues(void) ...@@ -4450,10 +4443,7 @@ static int __init init_workqueues(void)
struct workqueue_attrs *attrs; struct workqueue_attrs *attrs;
BUG_ON(!(attrs = alloc_workqueue_attrs(GFP_KERNEL))); BUG_ON(!(attrs = alloc_workqueue_attrs(GFP_KERNEL)));
attrs->nice = std_nice[i]; attrs->nice = std_nice[i];
cpumask_setall(attrs->cpumask);
unbound_std_wq_attrs[i] = attrs; unbound_std_wq_attrs[i] = attrs;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment