Re: [alsa-devel] [PATCH v4 11/18] input: Add initial support for TWL6040 vibrator
On Wednesday 15 June 2011 10:23:01 Tejun Heo wrote:
On Wed, Jun 15, 2011 at 10:18:58AM +0200, Tejun Heo wrote:
No human being can feel 120usec difference and I can't see how using HIGHPRI is justified here (which is what the code is doing _accidentally_ by using singlethread_workqueue).
Ooh, one more thing, and even if you insist on using HIGHPRI (please don't), you don't need to create workqueue for each device. You can just create one for the whole driver in init and destroy it from exit. What matters is the HIGHPRI attribute of the workqueue. The number of workqueues is completely irrelevant.
Fair enough. I'll move to create_workqueue. If we later find issues with this (in a 'live' system), we can figure out a way to fix it.
Thank you for your time on this. I'll make the changes accordingly.
Regards, Péter
Hello,
On Thu, Jun 16, 2011 at 02:13:59PM +0300, Péter Ujfalusi wrote:
On Wednesday 15 June 2011 10:23:01 Tejun Heo wrote:
On Wed, Jun 15, 2011 at 10:18:58AM +0200, Tejun Heo wrote:
No human being can feel 120usec difference and I can't see how using HIGHPRI is justified here (which is what the code is doing _accidentally_ by using singlethread_workqueue).
Ooh, one more thing, and even if you insist on using HIGHPRI (please don't), you don't need to create workqueue for each device. You can just create one for the whole driver in init and destroy it from exit. What matters is the HIGHPRI attribute of the workqueue. The number of workqueues is completely irrelevant.
Fair enough. I'll move to create_workqueue.
I suppose you meant alloc_workqueue()? :)
Sorry about the confusing names, I'm still in the (slow) process of deprecating older APIs.
Thanks.
Hello,
On Thursday 16 June 2011 14:02:44 Tejun Heo wrote:
I'll move to create_workqueue.
I suppose you meant alloc_workqueue()? :)
Oh, yes. I mean that.
Sorry about the confusing names, I'm still in the (slow) process of deprecating older APIs.
Will do this soon, and I ping the other maintainers, if they see other issues with the series, so I can make the change at the same time.
Thanks, Péter
Hello Tejun,
On Thursday 16 June 2011 16:06:00 Ujfalusi, Peter wrote:
I suppose you meant alloc_workqueue()? :)
Oh, yes. I mean that.
Just avoid another series... I have looked at the alloc_workqueue, and I'm not really sure what parameters should I use. #define create_singlethread_workqueue(name) \ alloc_workqueue((name), WQ_UNBOUND | WQ_MEM_RECLAIM, 1)
I would use something like this instead of create_singlethread_workqueue:
alloc_workqueue("twl6040-vibra", WQ_UNBOUND | WQ_MEM_RECLAIM | WQ_HIGHPRI, 1);
Is this correct?
Thanks, Péter
Hello, Péter.
On Fri, Jun 17, 2011 at 12:39:57PM +0300, Péter Ujfalusi wrote:
Just avoid another series... I have looked at the alloc_workqueue, and I'm not really sure what parameters should I use. #define create_singlethread_workqueue(name) \ alloc_workqueue((name), WQ_UNBOUND | WQ_MEM_RECLAIM, 1)
I would use something like this instead of create_singlethread_workqueue:
alloc_workqueue("twl6040-vibra", WQ_UNBOUND | WQ_MEM_RECLAIM | WQ_HIGHPRI, 1);
Just use the default params
alloc_workqueue("twl6040-vibra", 0, 0);
Thanks.
On Fri, Jun 17, 2011 at 01:59:47PM +0300, Péter Ujfalusi wrote:
On Friday 17 June 2011 11:43:32 Tejun Heo wrote:
Just use the default params
alloc_workqueue("twl6040-vibra", 0, 0);
Thanks.
I'll do this for the next posting
Tejun,
For my education, what is the benefit of creating a dedicated workqueue with alloc_workqueue (which, as far as I understand, does not end up having dedicated worker threads but will use the common pool) and simply queueing the jobs on system-wide workqueue?
Thanks.
Hello,
2011/6/18 Dmitry Torokhov dmitry.torokhov@gmail.com:
For my education, what is the benefit of creating a dedicated workqueue with alloc_workqueue (which, as far as I understand, does not end up having dedicated worker threads but will use the common pool) and simply queueing the jobs on system-wide workqueue?
In this case, nothing really, but Péter seems to want to have a dedicated workqueue so that he can later flip HIGHPRI easily if necessary. Usually what a separate workqueue buys are...
* It serves as a flushing domain. ie. You can flush work items queued to the same workqueue together. This is useful when individual work items can't be flushed (e.g. they free themselves) or doing so is inefficient.
* It serves as an attribute domain. ie. You can set WQ_* flags and @max_active. If using the default values, nothing really is different from using system_wq.
Thanks.
participants (3)
-
Dmitry Torokhov
-
Péter Ujfalusi
-
Tejun Heo