Messages in this thread | | | Date | Wed, 13 May 2015 09:43:19 +0800 | From | Lai Jiangshan <> | Subject | Re: [PATCH 4/5] workqueue: don't expose workqueue_attrs to users |
| |
On 05/12/2015 09:22 PM, Tejun Heo wrote: > Hello, Lai. > > On Tue, May 12, 2015 at 10:15:28AM +0800, Lai Jiangshan wrote: >>> I'm not sure about this. Yeah, sure, it's a bit more lines of code >>> but at the same time this'd allow us to make the public interface >>> atomic too. What we prolly should do is changing the interface so >>> that we do >>> >>> attrs = prepare_workqueue_attrs(gfp_mask); /* allocate, lock & copy */ >>> /* modify attrs as desired */ >>> commit_workqueue_attrs(attrs); /* apply, unlock and free */ >> >> I think the workqueue.c has too much complicated and rarely used APIs >> and exposes too much in this way. No one can set the nice value >> and the cpuallowed of a task atomically. > > What do you mean no one can?
normal/general task. not kworker.
no one can set the nice value and the cpumallowed of a normal task atomically.
The kernel doesn't have such APIs:
lock_and_get_task_cpus_allowed(task); /* modify cpumask */ set_cpus_allowed_ptr_and_unlock();
> >> If the user want atomic-able, Her/he can just disable WQ_SYSFS >> on its workqueue and maintain a copy of the cpumask, nice, numa values >> under its own lock. > > So, we're now requiring workqueue users to take care of > synchronization, disabling and reinstating WQ_SYSFS (what if userland > hits those knobs at the same time?)
I think there is no userland knobs when !WQ_SYSFS.
> and poking into workqueue struct to determine the current values of the
I think the copy version of cpumask, nice, numa values are same as the workqueue struct have. No poking is required. (Its own lock-protect-region is the ONLY entry to call apply_workqueue_attrs()).
> attributes that the user is not > intereted in changing? This is a horrible interface.
| |