Messages in this thread | | | Date | Mon, 11 May 2015 10:55:02 -0400 | From | Tejun Heo <> | Subject | Re: [PATCH 3/5] workqueue: ensure attrs-changing be sequentially |
| |
Hey,
Prolly a better subject is "ensure attrs changes are properly synchronized"
On Mon, May 11, 2015 at 05:35:50PM +0800, Lai Jiangshan wrote: > Current modification to attrs via sysfs is not atomically.
atomic.
> > Process A (change cpumask) | Process B (change numa affinity) > wq_cpumask_store() | > wq_sysfs_prep_attrs() | ^ misaligned
> | apply_workqueue_attrs() > apply_workqueue_attrs() | > > It results that the Process B's operation is totally reverted > without any notification.
Yeah, right.
> This behavior is acceptable but it is sometimes unexpected.
I don't think this is an acceptable behavior.
> Sequential model on non-performance-sensitive operations is more popular > and preferred. So this patch moves wq_sysfs_prep_attrs() into the protection
You can just say the previous behavior is buggy.
> under wq_pool_mutex to ensure attrs-changing be sequentially. > > This patch is also a preparation patch for next patch which change > the API of apply_workqueue_attrs(). ... > +static void apply_wqattrs_lock(void) > +{ > + /* > + * CPUs should stay stable across pwq creations and installations. > + * Pin CPUs, determine the target cpumask for each node and create > + * pwqs accordingly. > + */ > + get_online_cpus(); > + mutex_lock(&wq_pool_mutex); > +} > + > +static void apply_wqattrs_unlock(void) > +{ > + mutex_unlock(&wq_pool_mutex); > + put_online_cpus(); > +}
Separate out refactoring and extending locking coverage?
Thanks.
-- tejun
| |