From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Leun Subject: Re: 3.11-rc regression bisected: s2disk does not work (was Re: [PATCH v3 13/16] futex: use freezable blocking call) Date: Tue, 23 Jul 2013 23:58:27 +0200 Message-ID: <20130723235827.014f7899@xenia.leun.net> References: <1367884221-20462-1-git-send-email-ccross@android.com> <15305281.aClQ8XUG9t@vostro.rjw.lan> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: "Rafael J. Wysocki" , Linus Torvalds , lkml , Pavel Machek , Peter Zijlstra , Ingo Molnar , Andrew Morton , Mandeep Singh Baines , Oleg Nesterov , linux-nfs , Linux PM list , netdev , Tejun Heo , Darren Hart , Thomas Gleixner , Randy Dunlap , Al Viro To: Colin Cross Return-path: In-Reply-To: Sender: linux-pm-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Tue, 23 Jul 2013 13:31:49 -0700 Colin Cross wrote: > On Mon, Jul 22, 2013 at 11:28 PM, Colin Cross > wrote: > > On Mon, Jul 22, 2013 at 6:41 PM, Rafael J. Wysocki > > wrote: > >> On Monday, July 22, 2013 05:42:49 PM Colin Cross wrote: > >>> On Mon, Jul 22, 2013 at 5:32 PM, Linus Torvalds > >>> wrote: > >>> > On Mon, Jul 22, 2013 at 4:55 PM, Colin Cross > >>> > wrote: > >>> >> > >>> >> I think the right solution is to add a flag to the freezing > >>> >> task that marks it unfreezable. I think PF_NOFREEZE would > >>> >> work, although it is normally used on kernel threads, can you > >>> >> see if the attached patch helps? > >>> > > >>> > Hmm. That does seem to be the right thing to do, but I wonder > >>> > about the *other* callers of freeze_processes() IOW, kexec and > >>> > friends. > >>> > > >>> > So maybe we should do this in {freeze|thaw}_processes() itself, > >>> > and just make the rule be that the caller of freeze_processes() > >>> > itself is obviously not frozen, and has to be the same one that > >>> > then thaws things? > >>> > > >>> > Colin? Rafael? Comments? > >>> > > >>> > Linus > >>> > >>> I was worried about clearing the flag in thaw_processes(). If a > >>> kernel thread with PF_NOFREEZE set ever called thaw_processes(), > >>> which autosleep might do, it would clear the flag. Or if a > >>> different thread called freeze_processes() and thaw_processes(). > >> > >> Is that legitimate? > > > > Nothing precludes it today, but I don't see any need for it. I'll > > add a comment when I add the flag. > > > >>> All the other callers besides the SNAPSHOT_FREEZE ioctl stay in > >>> the kernel between freeze_processes() and thaw_processes(), which > >>> makes the fanout of places that could call try_to_freeze() much > >>> more controllable. > >>> > >>> Using a new flag that operates like PF_NOFREEZE but doesn't > >>> conflict with it, or a nofreeze_depth counter, would also work. > >> > >> Well, that would be robust enough. At least if the purpose of > >> that new flag is clearly specified, people hopefully won't be > >> tempted to optimize it away in the future. > >> > >> Thanks, > >> Rafael > > > > OK, I'll add a new flag. > > > Michael, can you see if this patch works and doesn't throw any > warnings during suspend or resume? Tried several times with and without threads = y in suspend.conf, tried also to produce high load / much processes / high memory usage. Worked every time, no WARN seen. > If the extra process flag is considered too precious for this > (there are only 2 left after this patch) I could get the > same functionality by having freeze_processes() reject calls > from a PF_KTHREAD|PF_NOFREEZE thread, and use PF_KTHREAD to > determine if PF_NOFREEZE should be cleared in thaw_processes(). If another solution is considered please do not hesitate to send me the patch for another round of check. -- MfG, Michael Leun