Date: Tue, 12 May 2015 09:22:09 -0400
From: Tejun Heo <tj@kernel.org>
To: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/5] workqueue: don't expose workqueue_attrs to users
Message-ID: <20150512132209.GM11388@htj.duckdns.org>
References: <1431336953-3260-1-git-send-email-laijs@cn.fujitsu.com>
 <1431336953-3260-5-git-send-email-laijs@cn.fujitsu.com>
 <20150511145952.GE11388@htj.duckdns.org>
 <55516240.6000601@cn.fujitsu.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <55516240.6000601@cn.fujitsu.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1449
Lines: 37

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/