Messages in this thread | | | Date | Tue, 12 May 2015 13:09:50 +0800 | From | Lai Jiangshan <> | Subject | Re: [PATCH 3/5] workqueue: ensure attrs-changing be sequentially |
| |
On 05/11/2015 10:55 PM, Tejun Heo wrote: > 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
It is aligned in email, misaligned in quoted email, and misaligned in `git log` and `git show`, aligned in `git commit` when I wrote the changelog.
I will just remove all the |.
> >> | 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.
It depends on definitions. To me, it is just a nuisance.
> >> 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. >
| |