Re: [PATCH 4/5] workqueue: don't expose workqueue_attrs to users

[Tejun Heo <tj@kernel.org>]

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?

> 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?) and poking into workqueue struct
to determine the current values of the attributes that the user is not
intereted in changing?  This is a horrible interface.

Thanks.

-- 
tejun