The apply_wqattrs_lock()/unlock() helpers were introduced by commit a0111cf6710b ("workqueue: separate out and refactor the locking of applying attrs") to encapsulate the get_online_cpus() (later cpus_read_lock()) + mutex_lock(&wq_pool_mutex) acquire pair that was duplicated across the apply-attrs paths. Since commit 19af45757383 ("workqueue: Remove cpus_read_lock() from apply_wqattrs_lock()") removed the cpus_read_lock() (pwq creation and installation now operate on wq_online_cpumask, so CPU hotplug no longer needs to be excluded), the wrappers have been one-line forwarders to mutex_lock(&wq_pool_mutex)/mutex_unlock(&wq_pool_mutex). They no longer encode any non-trivial locking rule and obscure the fact that callers just take the existing wq_pool_mutex. This align with the "unnecessary" helpers that got discussed in [1] Inline the eight call sites and remove the wrappers. No functional change.
Showing the changes to kernel/workqueue.c (1 of 1 file). Show all
Unified · Split
| @@ -5300,16 +5300,6 @@ static struct pool_workqueue *alloc_unbound_pwq(struct workqueue_struct *wq, · 5299 unmodified lines | |||
| 5300 | return pwq; | 5300 | return pwq; |
| 5301 | } | 5301 | } |
| 5302 | 5302 | ||
| 5303 | static void apply_wqattrs_lock(void) | ||
| 5304 | { | ||
| 5305 | mutex_lock(&wq_pool_mutex); | ||
| 5306 | } | ||
| 5307 | |||
| 5308 | static void apply_wqattrs_unlock(void) | ||
| 5309 | { | ||
| 5310 | mutex_unlock(&wq_pool_mutex); | ||
| 5311 | } | ||
| 5312 | |||
| 5313 | /** | 5303 | /** |
| 5314 | * wq_calc_pod_cpumask - calculate a wq_attrs' cpumask for a pod | 5304 | * wq_calc_pod_cpumask - calculate a wq_attrs' cpumask for a pod |
| 5315 | * @attrs: the wq_attrs of the default pwq of the target workqueue | 5305 | * @attrs: the wq_attrs of the default pwq of the target workqueue |
| @@ -5863,7 +5853,7 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt, · 547 unmodified lines | |||
| 5863 | * wq_pool_mutex protects the workqueues list, allocations of PWQs, | 5853 | * wq_pool_mutex protects the workqueues list, allocations of PWQs, |
| 5864 | * and the global freeze state. | 5854 | * and the global freeze state. |
| 5865 | */ | 5855 | */ |
| 5866 | apply_wqattrs_lock(); | 5856 | mutex_lock(&wq_pool_mutex); |
| 5867 | 5857 | ||
| 5868 | if (alloc_and_link_pwqs(wq) < 0) | 5858 | if (alloc_and_link_pwqs(wq) < 0) |
| 5869 | goto err_unlock_free_node_nr_active; | 5859 | goto err_unlock_free_node_nr_active; |
| @@ -5877,7 +5867,7 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt, · 7 unmodified lines | |||
| 5877 | if (wq_online && init_rescuer(wq) < 0) | 5867 | if (wq_online && init_rescuer(wq) < 0) |
| 5878 | goto err_unlock_destroy; | 5868 | goto err_unlock_destroy; |
| 5879 | 5869 | ||
| 5880 | apply_wqattrs_unlock(); | 5870 | mutex_unlock(&wq_pool_mutex); |
| 5881 | 5871 | ||
| 5882 | if ((wq->flags & WQ_SYSFS) && workqueue_sysfs_register(wq)) | 5872 | if ((wq->flags & WQ_SYSFS) && workqueue_sysfs_register(wq)) |
| 5883 | goto err_destroy; | 5873 | goto err_destroy; |
| @@ -5885,7 +5875,7 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt, · 1 unmodified line | |||
| 5885 | return wq; | 5875 | return wq; |
| 5886 | 5876 | ||
| 5887 | err_unlock_free_node_nr_active: | 5877 | err_unlock_free_node_nr_active: |
| 5888 | apply_wqattrs_unlock(); | 5878 | mutex_unlock(&wq_pool_mutex); |
| 5889 | /* | 5879 | /* |
| 5890 | * Failed alloc_and_link_pwqs() may leave pending pwq->release_work, | 5880 | * Failed alloc_and_link_pwqs() may leave pending pwq->release_work, |
| 5891 | * flushing the pwq_release_worker ensures that the pwq_release_workfn() | 5881 | * flushing the pwq_release_worker ensures that the pwq_release_workfn() |
| @@ -5900,7 +5890,7 @@ err_free_wq: · 8 unmodified lines | |||
| 5900 | kfree(wq); | 5890 | kfree(wq); |
| 5901 | return NULL; | 5891 | return NULL; |
| 5902 | err_unlock_destroy: | 5892 | err_unlock_destroy: |
| 5903 | apply_wqattrs_unlock(); | 5893 | mutex_unlock(&wq_pool_mutex); |
| 5904 | err_destroy: | 5894 | err_destroy: |
| 5905 | destroy_workqueue(wq); | 5895 | destroy_workqueue(wq); |
| 5906 | return NULL; | 5896 | return NULL; |
| @@ -7301,7 +7291,7 @@ static ssize_t wq_nice_store(struct device *dev, struct device_attribute *attr, · 1394 unmodified lines | |||
| 7301 | struct workqueue_attrs *attrs; | 7291 | struct workqueue_attrs *attrs; |
| 7302 | int ret = -ENOMEM; | 7292 | int ret = -ENOMEM; |
| 7303 | 7293 | ||
| 7304 | apply_wqattrs_lock(); | 7294 | mutex_lock(&wq_pool_mutex); |
| 7305 | 7295 | ||
| 7306 | attrs = wq_sysfs_prep_attrs(wq); | 7296 | attrs = wq_sysfs_prep_attrs(wq); |
| 7307 | if (!attrs) | 7297 | if (!attrs) |
| @@ -7314,7 +7304,7 @@ static ssize_t wq_nice_store(struct device *dev, struct device_attribute *attr, · 6 unmodified lines | |||
| 7314 | ret = -EINVAL; | 7304 | ret = -EINVAL; |
| 7315 | 7305 | ||
| 7316 | out_unlock: | 7306 | out_unlock: |
| 7317 | apply_wqattrs_unlock(); | 7307 | mutex_unlock(&wq_pool_mutex); |
| 7318 | free_workqueue_attrs(attrs); | 7308 | free_workqueue_attrs(attrs); |
| 7319 | return ret ?: count; | 7309 | return ret ?: count; |
| 7320 | } | 7310 | } |
| @@ -7340,7 +7330,7 @@ static ssize_t wq_cpumask_store(struct device *dev, · 19 unmodified lines | |||
| 7340 | struct workqueue_attrs *attrs; | 7330 | struct workqueue_attrs *attrs; |
| 7341 | int ret = -ENOMEM; | 7331 | int ret = -ENOMEM; |
| 7342 | 7332 | ||
| 7343 | apply_wqattrs_lock(); | 7333 | mutex_lock(&wq_pool_mutex); |
| 7344 | 7334 | ||
| 7345 | attrs = wq_sysfs_prep_attrs(wq); | 7335 | attrs = wq_sysfs_prep_attrs(wq); |
| 7346 | if (!attrs) | 7336 | if (!attrs) |
| @@ -7351,7 +7341,7 @@ static ssize_t wq_cpumask_store(struct device *dev, · 4 unmodified lines | |||
| 7351 | ret = apply_workqueue_attrs_locked(wq, attrs); | 7341 | ret = apply_workqueue_attrs_locked(wq, attrs); |
| 7352 | 7342 | ||
| 7353 | out_unlock: | 7343 | out_unlock: |
| 7354 | apply_wqattrs_unlock(); | 7344 | mutex_unlock(&wq_pool_mutex); |
| 7355 | free_workqueue_attrs(attrs); | 7345 | free_workqueue_attrs(attrs); |
| 7356 | return ret ?: count; | 7346 | return ret ?: count; |
| 7357 | } | 7347 | } |
| @@ -7387,13 +7377,13 @@ static ssize_t wq_affn_scope_store(struct device *dev, · 29 unmodified lines | |||
| 7387 | if (affn < 0) | 7377 | if (affn < 0) |
| 7388 | return affn; | 7378 | return affn; |
| 7389 | 7379 | ||
| 7390 | apply_wqattrs_lock(); | 7380 | mutex_lock(&wq_pool_mutex); |
| 7391 | attrs = wq_sysfs_prep_attrs(wq); | 7381 | attrs = wq_sysfs_prep_attrs(wq); |
| 7392 | if (attrs) { | 7382 | if (attrs) { |
| 7393 | attrs->affn_scope = affn; | 7383 | attrs->affn_scope = affn; |
| 7394 | ret = apply_workqueue_attrs_locked(wq, attrs); | 7384 | ret = apply_workqueue_attrs_locked(wq, attrs); |
| 7395 | } | 7385 | } |
| 7396 | apply_wqattrs_unlock(); | 7386 | mutex_unlock(&wq_pool_mutex); |
| 7397 | free_workqueue_attrs(attrs); | 7387 | free_workqueue_attrs(attrs); |
| 7398 | return ret ?: count; | 7388 | return ret ?: count; |
| 7399 | } | 7389 | } |
| @@ -7418,13 +7408,13 @@ static ssize_t wq_affinity_strict_store(struct device *dev, · 18 unmodified lines | |||
| 7418 | if (sscanf(buf, "%d", &v) != 1) | 7408 | if (sscanf(buf, "%d", &v) != 1) |
| 7419 | return -EINVAL; | 7409 | return -EINVAL; |
| 7420 | 7410 | ||
| 7421 | apply_wqattrs_lock(); | 7411 | mutex_lock(&wq_pool_mutex); |
| 7422 | attrs = wq_sysfs_prep_attrs(wq); | 7412 | attrs = wq_sysfs_prep_attrs(wq); |
| 7423 | if (attrs) { | 7413 | if (attrs) { |
| 7424 | attrs->affn_strict = (bool)v; | 7414 | attrs->affn_strict = (bool)v; |
| 7425 | ret = apply_workqueue_attrs_locked(wq, attrs); | 7415 | ret = apply_workqueue_attrs_locked(wq, attrs); |
| 7426 | } | 7416 | } |
| 7427 | apply_wqattrs_unlock(); | 7417 | mutex_unlock(&wq_pool_mutex); |
| 7428 | free_workqueue_attrs(attrs); | 7418 | free_workqueue_attrs(attrs); |
| 7429 | return ret ?: count; | 7419 | return ret ?: count; |
| 7430 | } | 7420 | } |
| @@ -7465,12 +7455,12 @@ static int workqueue_set_unbound_cpumask(cpumask_var_t cpumask) · 34 unmodified lines | |||
| 7465 | cpumask_and(cpumask, cpumask, cpu_possible_mask); | 7455 | cpumask_and(cpumask, cpumask, cpu_possible_mask); |
| 7466 | if (!cpumask_empty(cpumask)) { | 7456 | if (!cpumask_empty(cpumask)) { |
| 7467 | ret = 0; | 7457 | ret = 0; |
| 7468 | apply_wqattrs_lock(); | 7458 | mutex_lock(&wq_pool_mutex); |
| 7469 | if (!cpumask_equal(cpumask, wq_unbound_cpumask)) | 7459 | if (!cpumask_equal(cpumask, wq_unbound_cpumask)) |
| 7470 | ret = workqueue_apply_unbound_cpumask(cpumask); | 7460 | ret = workqueue_apply_unbound_cpumask(cpumask); |
| 7471 | if (!ret) | 7461 | if (!ret) |
| 7472 | cpumask_copy(wq_requested_unbound_cpumask, cpumask); | 7462 | cpumask_copy(wq_requested_unbound_cpumask, cpumask); |
| 7473 | apply_wqattrs_unlock(); | 7463 | mutex_unlock(&wq_pool_mutex); |
| 7474 | } | 7464 | } |
| 7475 | 7465 | ||
| 7476 | return ret; | 7466 | return ret; |