Singlethread vs. freezable workqueues

Singlethread vs. freezable workqueues

[Alan Stern]

Raphael:

Are we now committed to making freezable workqueues always 
singlethreaded?  Is it at all likely to change back?  Or should I 
introduce a "create_singlethread_freezeable_workqueue" macro?

Alan Stern

Re: Singlethread vs. freezable workqueues

[Rafael J. Wysocki]

Hi,

On Monday, 21 May 2007 21:33, Alan Stern wrote:
> Raphael:
> 
> Are we now committed to making freezable workqueues always 
> singlethreaded?  Is it at all likely to change back?  Or should I 
> introduce a "create_singlethread_freezeable_workqueue" macro?

This was done as a quick fix of an issue with one driver that started to use
(broken) freezable workqueues when we were not watching. ;-)

We are going to have multithread freezable workqueues as well, but that'll
take some time.  We've discussed this a bit with Oleg and I believe he has an
idea of how it can be done cleanly.

Greetings,
Rafael

Re: Singlethread vs. freezable workqueues

[Alan Stern]

On Mon, 21 May 2007, Rafael J. Wysocki wrote:

> Hi,
> 
> On Monday, 21 May 2007 21:33, Alan Stern wrote:
> > Raphael:
> > 
> > Are we now committed to making freezable workqueues always 
> > singlethreaded?  Is it at all likely to change back?  Or should I 
> > introduce a "create_singlethread_freezeable_workqueue" macro?
> 
> This was done as a quick fix of an issue with one driver that started to use
> (broken) freezable workqueues when we were not watching. ;-)
> 
> We are going to have multithread freezable workqueues as well, but that'll
> take some time.  We've discussed this a bit with Oleg and I believe he has an
> idea of how it can be done cleanly.

The reason I asked is because, prior to the release of 2.6.22, I sent 
in a patch which added a create_singlethread_freezeable_workqueue() 
macro.  Now it isn't needed, since create_freezeable_workqueue() does 
the same thing.  Is there any reason to keep the macro?

I'm worried that if I get rid of it and simply define the workqueue as 
freezable, then at some time in the future it might turn into a 
multithread workqueue without my knowledge.

Alan Stern

Re: Singlethread vs. freezable workqueues

[Rafael J. Wysocki]

On Monday, 21 May 2007 22:23, Alan Stern wrote:
> On Mon, 21 May 2007, Rafael J. Wysocki wrote:
> 
> > Hi,
> > 
> > On Monday, 21 May 2007 21:33, Alan Stern wrote:
> > > Raphael:
> > > 
> > > Are we now committed to making freezable workqueues always 
> > > singlethreaded?  Is it at all likely to change back?  Or should I 
> > > introduce a "create_singlethread_freezeable_workqueue" macro?
> > 
> > This was done as a quick fix of an issue with one driver that started to use
> > (broken) freezable workqueues when we were not watching. ;-)
> > 
> > We are going to have multithread freezable workqueues as well, but that'll
> > take some time.  We've discussed this a bit with Oleg and I believe he has an
> > idea of how it can be done cleanly.
> 
> The reason I asked is because, prior to the release of 2.6.22, I sent 
> in a patch which added a create_singlethread_freezeable_workqueue() 
> macro.  Now it isn't needed, since create_freezeable_workqueue() does 
> the same thing.  Is there any reason to keep the macro?
> 
> I'm worried that if I get rid of it and simply define the workqueue as 
> freezable, then at some time in the future it might turn into a 
> multithread workqueue without my knowledge.

If we introduce multithread freezable workqueues again, we'll have to review
all current users of create_freezeable_workqueue() anyway to see whether or
not they need to use a singlethread workqueue.  Still, if you added a comment
saying that your workqueue had to be singlethread, that would certainly help. ;-)

Greetings,
Rafael

Re: Singlethread vs. freezable workqueues

[Oleg Nesterov]

On 05/21, Rafael J. Wysocki wrote:
>
> On Monday, 21 May 2007 22:23, Alan Stern wrote:
> > 
> > The reason I asked is because, prior to the release of 2.6.22, I sent 
> > in a patch which added a create_singlethread_freezeable_workqueue() 
> > macro.  Now it isn't needed, since create_freezeable_workqueue() does 
> > the same thing.  Is there any reason to keep the macro?
> > 
> > I'm worried that if I get rid of it and simply define the workqueue as 
> > freezable, then at some time in the future it might turn into a 
> > multithread workqueue without my knowledge.
> 
> If we introduce multithread freezable workqueues again, we'll have to review
> all current users of create_freezeable_workqueue() anyway to see whether or
> not they need to use a singlethread workqueue.  Still, if you added a comment
> saying that your workqueue had to be singlethread, that would certainly help. ;-)

Probably Alan is right? Perhaps it is better to rename it to
create_singlethread_freezeable_workqueue(). This way we don't need to audit
the users when we add multithread freezable wqs.

Also, this name is more consistent wrt create_workqueue/create_singlethread_workqueue.

Oleg.

Re: Singlethread vs. freezable workqueues

[Alan Stern]

On Tue, 22 May 2007, Oleg Nesterov wrote:

> Probably Alan is right? Perhaps it is better to rename it to
> create_singlethread_freezeable_workqueue(). This way we don't need to audit
> the users when we add multithread freezable wqs.
> 
> Also, this name is more consistent wrt
> create_workqueue/create_singlethread_workqueue.

I already sent in a revised patch and a request to revert the 
create_singlethread_freezeable_workqueue() macro!

Whatever you guys decide, please let Greg and me know as soon as
possible.  This particular fix is needed before 2.6.22 gets released.

Alan Stern

Re: Singlethread vs. freezable workqueues

[Oleg Nesterov]

On 05/22, Alan Stern wrote:
>
> On Tue, 22 May 2007, Oleg Nesterov wrote:
> 
> > Probably Alan is right? Perhaps it is better to rename it to
> > create_singlethread_freezeable_workqueue(). This way we don't need to audit
> > the users when we add multithread freezable wqs.
> > 
> > Also, this name is more consistent wrt
> > create_workqueue/create_singlethread_workqueue.
> 
> I already sent in a revised patch and a request to revert the 
> create_singlethread_freezeable_workqueue() macro!
> 
> Whatever you guys decide, please let Greg and me know as soon as
> possible.  This particular fix is needed before 2.6.22 gets released.

Ah, I am sorry for the confusion, I didn't see the start of this thread.

Please do what you think right.

Oleg.

Re: Singlethread vs. freezable workqueues

[Oleg Nesterov]

On 05/21, Rafael J. Wysocki wrote:
> 
> On Monday, 21 May 2007 21:33, Alan Stern wrote:
> > Raphael:
> > 
> > Are we now committed to making freezable workqueues always 
> > singlethreaded?  Is it at all likely to change back?  Or should I 
> > introduce a "create_singlethread_freezeable_workqueue" macro?
> 
> This was done as a quick fix of an issue with one driver that started to use
> (broken) freezable workqueues when we were not watching. ;-)
> 
> We are going to have multithread freezable workqueues as well, but that'll
> take some time.  We've discussed this a bit with Oleg and I believe he has an
> idea of how it can be done cleanly.

No, I don't have an idea how to do this cleanly currently.

We can fix them right now with Rafael's "take_over_work() + migrate_sequence"
patch, feel free to send it. Not perfect, but should work.

Perhaps it makes sense to make some other changes first. For example, kill
CPU_TASKS_FROZEN bit.

Next. We can't make all wqs freezeable, but if we add freezer_exempt/PF_FE_XXX
we can freeze them all for cpu_up/cpu_down, this also make things simpler.

Note that we don't have a good way to use take_over_work() for !freezeable wq.
2.6.21 does kthread_stop() first, this is deadlockable because we may have
a work_struct on ->worklist which also calls kthread_stop(). Perhaps it makes
sense to wait until kthread_stop() will be reworked (should be soon).

Perhaps we can think a bit more :)

Oleg.

Re: Singlethread vs. freezable workqueues

[Rafael J. Wysocki]

On Monday, 21 May 2007 22:54, Oleg Nesterov wrote:
> On 05/21, Rafael J. Wysocki wrote:
> > 
> > On Monday, 21 May 2007 21:33, Alan Stern wrote:
> > > Raphael:
> > > 
> > > Are we now committed to making freezable workqueues always 
> > > singlethreaded?  Is it at all likely to change back?  Or should I 
> > > introduce a "create_singlethread_freezeable_workqueue" macro?
> > 
> > This was done as a quick fix of an issue with one driver that started to use
> > (broken) freezable workqueues when we were not watching. ;-)
> > 
> > We are going to have multithread freezable workqueues as well, but that'll
> > take some time.  We've discussed this a bit with Oleg and I believe he has an
> > idea of how it can be done cleanly.
> 
> No, I don't have an idea how to do this cleanly currently.
> 
> We can fix them right now with Rafael's "take_over_work() + migrate_sequence"
> patch, feel free to send it. Not perfect, but should work.

Well, there's no hurry with that, I think.  If there's a clear case in which
such a workqueue would be useful, we can do this.  For now, I don't see one.

> Perhaps it makes sense to make some other changes first. For example, kill
> CPU_TASKS_FROZEN bit.

That's possible, but the microcode driver uses it in a nontrivial way and that
should be taken into consideration.

> Next. We can't make all wqs freezeable, but if we add freezer_exempt/PF_FE_XXX
> we can freeze them all for cpu_up/cpu_down, this also make things simpler.

Yes.

> Note that we don't have a good way to use take_over_work() for !freezeable wq.
> 2.6.21 does kthread_stop() first, this is deadlockable because we may have
> a work_struct on ->worklist which also calls kthread_stop(). Perhaps it makes
> sense to wait until kthread_stop() will be reworked (should be soon).

Yes, I think we should wait until the kthread-related things settle.

> Perhaps we can think a bit more :)

Sure, that won't hurt. :-)

Greetings,
Rafael