mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@tv-sign.ru>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Michal Piotrowski <michal.k.k.piotrowski@gmail.com>,
	Alex Dubov <oakad@yahoo.com>, Pierre Ossman <drzeus@drzeus.cx>,
	Pavel Machek <pavel@ucw.cz>, Gautham R Shenoy <ego@in.ibm.com>
Subject: Re: Freezeable workqueues [Was: 2.6.22-rc1: Broken suspend on SMP with tifm]
Date: Sun, 20 May 2007 23:54:17 +0400	[thread overview]
Message-ID: <20070520195417.GB83@tv-sign.ru> (raw)
In-Reply-To: <200705152254.34402.rjw@sisk.pl>

On 05/15, Rafael J. Wysocki wrote:
>
> On Monday, 14 May 2007 23:48, Oleg Nesterov wrote:
> > 
> > So, in the long term, should we change this only user, or we think we better fix
> > freezeable wqs again?
> 
> Long term, I'd like to have freezable workqueues, so that people don't have to
> use "raw" kernel threads only because they need some synchronization with
> hibertnation/suspend.  Plus some cases in which workqueues are used by
> fs-related code make me worry.

OK, so we should fix them. It would be great to also fix the last known problem
as well (work->func() vs hotplug callback deadlocks).

I am a bit afraid of too many yes/no options for the freezer, a couple of naive
questions.

1. Can't we make all wqs freezable? I still can't see the reason to have both
   freezable and not freezable wqs.

2. Why do we need CPU_TASKS_FROZEN? Can't we change cpu-hotplug to always
   freeze tasks right now, without any additional changes?

   Any subsystem should handle correctly the case when _cpu_down() (say)
   is called with tasks_frozen == 1 anyway. So, why can't we simplify
   things and do

   	_cpu_down(int tasks_frozen)

 		if (!tasks_frozen)
 			freeze_processes();
 		...

  right now?

> [*] The problem is, though, that freezable workqueus have some potential to fail
> the freezer.  Namely, suppose task A calls flush_workqueue() on a freezable
> workqueue, finds some work items in there, inserts the barrier and waits for
> completion (TASK_UNINTERRUPTIBLE).  In the meantime, TIF_FREEZE is set on
> the worker thread, which is then woken up and goes to the refrigerator.  Thus
> if A is not NOFREEZE, the freezing of tasks will fail (A must be a kernel
> thread for this to happen, but still).  Worse yet, if A is NOFREEZE, it will be
> blocked until the worker thread is woken up.

Yes, this is yet another dependency which freezer can't handle. Probably it is
better to ignore this problem for now.

> To avoid this, I think, we may need to redesign the freezer, so that freezable
> worker threads are frozen after all of the other kernel threads.

I doubt we can find a very clean way to do this. Besides, what if work->func()
does flush_workqueue(another_wq) ? How can we decide which wq to freeze first?

>                                                                   Additionally,
> we'd need to make a rule that NOFREEZE kernel threads must not call
> flush_workqueue() or cancel_work_sync() on freezable workqueues.

cancel_work_sync() is OK, it can be used safely even if workqueue is frozen.
flush_workqueue() and destroy_workqueue() are not.

Oleg.


  reply	other threads:[~2007-05-20 19:54 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-13 19:32 2.6.22-rc1: Broken suspend on SMP with tifm Rafael J. Wysocki
2007-05-13 20:08 ` Oleg Nesterov
2007-05-13 20:30   ` Oleg Nesterov
2007-05-13 20:50     ` Rafael J. Wysocki
2007-05-13 20:50       ` Oleg Nesterov
2007-05-13 21:22         ` Rafael J. Wysocki
2007-05-13 21:34           ` Oleg Nesterov
2007-05-13 21:50             ` Rafael J. Wysocki
2007-05-13 21:54               ` Oleg Nesterov
2007-05-13 22:21                 ` Rafael J. Wysocki
2007-05-13 22:32                   ` Oleg Nesterov
2007-05-14  3:24                     ` Alex Dubov
2007-05-14  5:57             ` Rafael J. Wysocki
2007-05-14 16:55               ` Freezeable workqueues [Was: 2.6.22-rc1: Broken suspend on SMP with tifm] Oleg Nesterov
2007-05-14 21:27                 ` Rafael J. Wysocki
2007-05-14 21:48                   ` Oleg Nesterov
2007-05-15  0:56                     ` Alex Dubov
2007-05-15 20:54                       ` Rafael J. Wysocki
2007-05-15 20:54                     ` Rafael J. Wysocki
2007-05-20 19:54                       ` Oleg Nesterov [this message]
2007-05-20 20:48                         ` Rafael J. Wysocki
2007-05-20 21:06                           ` Oleg Nesterov
2007-05-20 21:49                             ` Rafael J. Wysocki
2007-05-13 20:33   ` 2.6.22-rc1: Broken suspend on SMP with tifm Rafael J. Wysocki
2007-05-13 21:52 ` [PATCH] for 2.6.22, make freezeable workqueues singlethread Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070520195417.GB83@tv-sign.ru \
    --to=oleg@tv-sign.ru \
    --cc=akpm@linux-foundation.org \
    --cc=drzeus@drzeus.cx \
    --cc=ego@in.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.k.k.piotrowski@gmail.com \
    --cc=oakad@yahoo.com \
    --cc=pavel@ucw.cz \
    --cc=rjw@sisk.pl \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®