From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751875AbXBRTCE (ORCPT ); Sun, 18 Feb 2007 14:02:04 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751867AbXBRTCE (ORCPT ); Sun, 18 Feb 2007 14:02:04 -0500 Received: from ogre.sisk.pl ([217.79.144.158]:49694 "EHLO ogre.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751875AbXBRTBn (ORCPT ); Sun, 18 Feb 2007 14:01:43 -0500 From: "Rafael J. Wysocki" To: Oleg Nesterov Subject: Re: freezer problems Date: Sun, 18 Feb 2007 19:56:10 +0100 User-Agent: KMail/1.9.5 Cc: ego@in.ibm.com, akpm@osdl.org, paulmck@us.ibm.com, mingo@elte.hu, vatsa@in.ibm.com, dipankar@in.ibm.com, venkatesh.pallipadi@intel.com, linux-kernel@vger.kernel.org, Pavel Machek References: <20070214144031.GA15257@in.ibm.com> <200702181314.29214.rjw@sisk.pl> <20070218145246.GA80@tv-sign.ru> In-Reply-To: <20070218145246.GA80@tv-sign.ru> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200702181956.11273.rjw@sisk.pl> Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org On Sunday, 18 February 2007 15:52, Oleg Nesterov wrote: > On 02/18, Rafael J. Wysocki wrote: > > > > On Sunday, 18 February 2007 12:31, Oleg Nesterov wrote: > > > > > > > > > > However, this means that sys_vfork() makes impossible to freeze processes > > > > > until child exits/execs. Not good. > > > > > > > I forgot to say that we have another problem: coredumping. > > > > > > A thread which does do_coredump() send SIGKILL to ->mm users, and sleeps > > > on ->mm->core_startup_done. Now it can't be frozen if sub-thread goes to > > > refrigerator. I think this could be solved easily if we add a check to > > > refrigerator() as you suggested for ->vfork_donw. > > > > > > > I think the real solution would be to use an interruptible completion in the > > > > vfork code. It was discussed some time ago and, IIRC, Ingo had an experimental > > > > patch that implemented it. Still, for the suspend this really is not an issue > > > > in practice, so it wasn't merged. > > > > > > It is not (afaics) so trivial to do rightly, and with this change the parent > > > will be seen as TASK_INTERRUPTIBLE even without freezer in progress. > > > > > > A very vague idea: what if parent will do > > > > > > current->flags |= PF_PLEASE_CONSIDER_ME_AS_FROZEN_BUT_SET_TIF_FREEZE > > > wait_for_completion(&vfork); > > > try_to_freeze(); > > > > > > ? > > > > This should work, > > Good. So try_to_freeze_tasks() can forget about "if (!p->vfork_done)" check. > This needs more thinking, of course. For example, thaw_process() should be > carefull to clear TIF_FREEZE if we have the new flag set, but not PF_FROZEN. > frozen() should be changed to return true if PF_NEW_FLAG && TIF_FREEZE, but > it also called by refrigerator. > > But IF we really can do this, it will be a general solution. Appended is a patch that does something along these lines. The necessary thread_info flags are defined for i386 and x86_64, for now. Greetings, Rafael include/asm-i386/thread_info.h | 2 ++ include/asm-x86_64/thread_info.h | 2 ++ include/linux/freezer.h | 24 ++++++++++++++++++++++++ kernel/fork.c | 4 ++++ kernel/power/process.c | 24 +++++++++--------------- 5 files changed, 41 insertions(+), 15 deletions(-) Index: linux-2.6.20-mm2/include/asm-i386/thread_info.h =================================================================== --- linux-2.6.20-mm2.orig/include/asm-i386/thread_info.h 2007-02-18 19:49:34.000000000 +0100 +++ linux-2.6.20-mm2/include/asm-i386/thread_info.h 2007-02-18 19:50:37.000000000 +0100 @@ -135,6 +135,7 @@ static inline struct thread_info *curren #define TIF_IO_BITMAP 18 /* uses I/O bitmap */ #define TIF_FREEZE 19 /* is freezing for suspend */ #define TIF_FORCED_TF 20 /* true if TF in eflags artificially */ +#define TIF_FREEZER_SKIP 21 /* task freezer should not count us */ #define _TIF_SYSCALL_TRACE (1< #include #include +#include #include #include @@ -1393,7 +1394,10 @@ long do_fork(unsigned long clone_flags, tracehook_report_clone_complete(clone_flags, nr, p); if (clone_flags & CLONE_VFORK) { + freezer_do_not_count(current); wait_for_completion(&vfork); + try_to_freeze(); + freezer_count(current); tracehook_report_vfork_done(p, nr); } } else { Index: linux-2.6.20-mm2/kernel/power/process.c =================================================================== --- linux-2.6.20-mm2.orig/kernel/power/process.c 2007-02-18 19:49:34.000000000 +0100 +++ linux-2.6.20-mm2/kernel/power/process.c 2007-02-18 19:50:37.000000000 +0100 @@ -117,22 +117,12 @@ static unsigned int try_to_freeze_tasks( cancel_freezing(p); continue; } - if (is_user_space(p)) { - if (!freeze_user_space) - continue; - - /* Freeze the task unless there is a vfork - * completion pending - */ - if (!p->vfork_done) - freeze_process(p); - } else { - if (freeze_user_space) - continue; + if (is_user_space(p) == !freeze_user_space) + continue; - freeze_process(p); - } - todo++; + freeze_process(p); + if (!freezer_should_skip(p)) + todo++; } while_each_thread(g, p); read_unlock(&tasklist_lock); yield(); /* Yield is okay here */ @@ -199,6 +189,10 @@ static void thaw_tasks(int thaw_user_spa read_lock(&tasklist_lock); do_each_thread(g, p) { + if (freezer_should_skip(p)) + cancel_freezing(p); + } while_each_thread(g, p); + do_each_thread(g, p) { if (!freezeable(p)) continue;