Re: Conversion questions for create_singlethread_workqueue()

Re: Conversion questions for create_singlethread_workqueue()

[Tejun Heo]

(cc'ing outreachy mailing list)

Hello, Eva.

On Sat, Mar 12, 2016 at 01:54:27AM +0800, Eva Rachel Retuya wrote:
> About the questions to ask covered on the task page, here are my findings:
> http://kernelnewbies.org/Tejun_Heo
> * WQ_MEM_RECLAIM may not need to be set
> * There are work items dealing with temperature throttling (tt_work, ct_enter,
>   ct_exit) -- This is just my opinion but I think setting the WQ_HIGHPRI flag
>   on them may help stabilize the hardware faster.

Hmmm... yeah, that makes sense.

> * I think yes based from my understanding of the workqueue usage.
> * I don't think so unless there is specific order to be followed during
>   submission to the workqueue.
> 
> My questions are:
> * How will I know whether order of execution is important here? What
>   characteristics in the code to look out for?

In a lot of cases, those use cases would be allocating and queueing
work items dynamically and the fact that those dynamic work items need
ordering should be relatively clear from the code and comments.

Here, there are only fixed number of work items per device; however,
it *could* be that different work items working on the same domain
could have ordering dependencies.  For example, it seems plausible
that if ct_enter and ct_exit are queued in certain order, their
execution order shouldn't be swapped.

I think the main thing to look for is whether there are multiple work
items doing similar or related operations.

> * If order of execution is not important, does choosing the default concurrency
>   limit pertain to the minimal attributes if I convert to using
>   alloc_workqueue()?

Yeah, just specify 0 for @max_active.

> * About the flags, what gives the indication that the work item is of high
>   priority or is cpu intensive? Say if I review the function definition of the
>   work item, what piece of code to take note if any.

In most cases, those shouldn't be necessary but as you pointed out
above, latency can be critical for hardware monitor and control
mechanisms, so that's a good example for HIGHPRI.  CPU_INTENSIVE is
for things like en/decrypting IOs for dm-crypt, so stuff which can
really consume a lot of cpu cycles.

> * Will it be possible to use system_wq with that number of work items? I'm
>   having the impression that sending numerous work items would fit a dedicated
>   workqueue rather than sending them globally. I need confirmation about this.

So, concurrency is limited by the number of work items.  Here, it's
only a handful per device.  Shouldn't be a problem.  Cases where one
should avoid using system_wq is are when work items are dynamically
allocated and queued and the concurrency can reach, say, high tens or
hundreds.

> * How do I gauge the time of execution? I have read that I cannot queue works
>   which can run for too long (in the case of system_wq).

As long as it's not a long-running service process which can run in
loop for over minutes, it should be fine.  This restriction is mostly
because of flush_scheduled_work() users which btw need to be converted
too.

> * After the conversion, how do I test this on my hardware?

I usually try to use it and plant some printks and verify that it
behaves as expected in corner cases.  cc'ing maintainers and asking
specific questions about the work items' chracteristics could be very
helpful.

> Right now I am leaning towards converting it to alloc_workqueue().

Thanks.

-- 
tejun

Re: Conversion questions for create_singlethread_workqueue()

[Eva Rachel Retuya]

On Fri, Mar 11, 2016 at 01:49:06PM -0500, Tejun Heo wrote:
> (cc'ing outreachy mailing list)
> 
> Hello, Eva.
> 
> On Sat, Mar 12, 2016 at 01:54:27AM +0800, Eva Rachel Retuya wrote:
> > About the questions to ask covered on the task page, here are my findings:
> > http://kernelnewbies.org/Tejun_Heo
> > * WQ_MEM_RECLAIM may not need to be set
> > * There are work items dealing with temperature throttling (tt_work, ct_enter,
> >   ct_exit) -- This is just my opinion but I think setting the WQ_HIGHPRI flag
> >   on them may help stabilize the hardware faster.
> 
> Hmmm... yeah, that makes sense.
> 
> > * I think yes based from my understanding of the workqueue usage.
> > * I don't think so unless there is specific order to be followed during
> >   submission to the workqueue.
> > 
> > My questions are:
> > * How will I know whether order of execution is important here? What
> >   characteristics in the code to look out for?
> 
> In a lot of cases, those use cases would be allocating and queueing
> work items dynamically and the fact that those dynamic work items need
> ordering should be relatively clear from the code and comments.
> 
> Here, there are only fixed number of work items per device; however,
> it *could* be that different work items working on the same domain
> could have ordering dependencies.  For example, it seems plausible
> that if ct_enter and ct_exit are queued in certain order, their
> execution order shouldn't be swapped.
> 
> I think the main thing to look for is whether there are multiple work
> items doing similar or related operations.
> 
> > * If order of execution is not important, does choosing the default concurrency
> >   limit pertain to the minimal attributes if I convert to using
> >   alloc_workqueue()?
> 
> Yeah, just specify 0 for @max_active.
> 
> > * About the flags, what gives the indication that the work item is of high
> >   priority or is cpu intensive? Say if I review the function definition of the
> >   work item, what piece of code to take note if any.
> 
> In most cases, those shouldn't be necessary but as you pointed out
> above, latency can be critical for hardware monitor and control
> mechanisms, so that's a good example for HIGHPRI.  CPU_INTENSIVE is
> for things like en/decrypting IOs for dm-crypt, so stuff which can
> really consume a lot of cpu cycles.
> 
> > * Will it be possible to use system_wq with that number of work items? I'm
> >   having the impression that sending numerous work items would fit a dedicated
> >   workqueue rather than sending them globally. I need confirmation about this.
> 
> So, concurrency is limited by the number of work items.  Here, it's
> only a handful per device.  Shouldn't be a problem.  Cases where one
> should avoid using system_wq is are when work items are dynamically
> allocated and queued and the concurrency can reach, say, high tens or
> hundreds.
> 
> > * How do I gauge the time of execution? I have read that I cannot queue works
> >   which can run for too long (in the case of system_wq).
> 
> As long as it's not a long-running service process which can run in
> loop for over minutes, it should be fine.  This restriction is mostly
> because of flush_scheduled_work() users which btw need to be converted
> too.
> 
> > * After the conversion, how do I test this on my hardware?
> 
> I usually try to use it and plant some printks and verify that it
> behaves as expected in corner cases.  cc'ing maintainers and asking
> specific questions about the work items' chracteristics could be very
> helpful.
> 
> > Right now I am leaning towards converting it to alloc_workqueue().
> 
> Thanks.
> 
> -- 
> tejun

Thank you for the guidance. I just submitted a patch regarding this
conversion I worked on. Please have a look and let me know if there are
workqueue-related functions I need to modify other than this. I reviewed
the workqueue header file and found the usage still aligns with the
change I propose unless there is something I missed.

Eva