From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932353Ab2ARRao (ORCPT ); Wed, 18 Jan 2012 12:30:44 -0500 Received: from mail-gx0-f174.google.com ([209.85.161.174]:62676 "EHLO mail-gx0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932200Ab2ARRam (ORCPT ); Wed, 18 Jan 2012 12:30:42 -0500 Date: Wed, 18 Jan 2012 09:30:37 -0800 From: Tejun Heo To: "Srivatsa S. Bhat" Cc: "Rafael J. Wysocki" , Linux PM list , LKML , horms@verge.net.au, "pavel@ucw.cz" , Len Brown Subject: Re: [Update][PATCH] PM / Hibernate: Fix s2disk regression related to unlock_system_sleep() Message-ID: <20120118173037.GE30664@google.com> References: <201201172345.15010.rjw@sisk.pl> <201201180015.56510.rjw@sisk.pl> <4F16C24A.4050007@linux.vnet.ibm.com> <4F16F94C.4020000@linux.vnet.ibm.com> <4F16FF0D.1030606@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4F16FF0D.1030606@linux.vnet.ibm.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, Srivatsa. On Wed, Jan 18, 2012 at 10:49:09PM +0530, Srivatsa S. Bhat wrote: > I agree, but I was trying to keep the comment from growing too long ;) It doesn't have to be long. It just has to give some meaning to the decision. AFAICS, it is correct to call try_to_freeze() on unlock_system_sleep() regardless of 20sec window. There's no guarantee the unlocking task is gonna hit try_to_freeze() elsewhere and not calling it actually makes the interface buggy. That said, it causes a problem because unlock_system_sleep() is called in a special context during later stage of hibernation where the usual expectation - that a freezable task which sees a freezing condition should freeze - doesn't hold. The correct solution would be somehow marking that condition so that either try_to_freeze() doesn't get invoked or gets nullified - e.g. making the SKIP thing a counter and ensure the hibernating task has it elevated throughout the whole process. Alternatively, if the code path is limited enough, using a different version of the unlock function, unlock_system_sleep_nofreeze() or whatever, would work too - this is a popular approach for synchronization functions which interacts with scheduler and preemption. For now, as a quick fix, maybe not calling try_to_freeze() unconditionally is okay, I don't know, but it's a hack. Thanks. -- tejun