From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from lgeamrelo03.lge.com (lgeamrelo03.lge.com [156.147.51.102]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8C72734572B for ; Fri, 20 Mar 2026 08:18:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=156.147.51.102 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773994719; cv=none; b=q6YfFU93RwPJhPg6e5qkTvcgh4qL5DcyW1BzO5aNG0Xmvvn91SMwijv+qUnPRhRSM/ix3k+rmKJeULAvBTKB6A7a+bHe0jjQQNPXRJL6EWDEXSNxrCVXvyaWLHrKwZZuL96eog71eiebJaxdKQXHcz3qcOMwwHp0bV2NeDPbLKo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773994719; c=relaxed/simple; bh=uysU7MtukPshm2NF1v3sTC8/8+j7ceRFUb1Ea+Bq3J8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=sfl3mmL5ZsoQ5/oSECt53n64LxaKoKGra8VewV2rGW7QSAQVb/oRliadlQEAcLwzvroe1fSdoF2sUBI0lrdhOKFs0gyIOjIZKSsc4uBIvvfo0LwWTk0omjYU0LWYOyIi6cCz7qzvGTrA2CmDXWjobj37aj9QKfKG35isyUTTiL8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=lge.com; spf=pass smtp.mailfrom=lge.com; arc=none smtp.client-ip=156.147.51.102 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=lge.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=lge.com Received: from unknown (HELO yjaykim-PowerEdge-T330) (10.177.112.156) by 156.147.51.102 with ESMTP; 20 Mar 2026 17:18:34 +0900 X-Original-SENDERIP: 10.177.112.156 X-Original-MAILFROM: youngjun.park@lge.com Date: Fri, 20 Mar 2026 17:18:34 +0900 From: YoungJun Park To: "Rafael J. Wysocki" Cc: akpm@linux-foundation.org, chrisl@kernel.org, kasong@tencent.com, pavel@kernel.org, shikemeng@huaweicloud.com, nphamcs@gmail.com, bhe@redhat.com, baohua@kernel.org, usama.arif@linux.dev, linux-mm@kvack.org, linux-pm@vger.kernel.org Subject: Re: [PATCH v5 3/3] PM: hibernate: fix spurious GFP mask WARNING in uswsusp path Message-ID: References: <20260319142404.3683019-1-youngjun.park@lge.com> <20260319142404.3683019-4-youngjun.park@lge.com> Precedence: bulk X-Mailing-List: linux-pm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Thu, Mar 19, 2026 at 08:55:43PM +0100, Rafael J. Wysocki wrote: > On Thu, Mar 19, 2026 at 3:24 PM Youngjun Park wrote: > > > > Commit 35e4a69b2003f ("PM: sleep: Allow pm_restrict_gfp_mask() > > stacking") introduced refcount-based GFP mask management that warns > > when pm_restore_gfp_mask() is called with saved_gfp_count == 0: > > > > WARNING: kernel/power/main.c:44 at pm_restore_gfp_mask+0xd7/0xf0 > > CPU: 0 UID: 0 PID: 373 Comm: s2disk > > Call Trace: > > snapshot_ioctl+0x964/0xbd0 > > __x64_sys_ioctl+0x724/0x1320 > > ... > > > > The uswsusp path calls pm_restore_gfp_mask() defensively in > > SNAPSHOT_CREATE_IMAGE and SNAPSHOT_UNFREEZE where the GFP mask may > > or may not be restricted depending on context (first call vs retry, > > hibernate vs resume). Before the stacking patch this was a silent > > no-op; now it triggers a WARNING. > > > > Introduce pm_restore_gfp_mask_safe() that skips the call when > > saved_gfp_count is 0. This is preferred over tracking the restrict > > state in snapshot_ioctl, as incorrect tracking risks leaving the > > GFP mask permanently restricted. > > > > Fixes: 35e4a69b2003f ("PM: sleep: Allow pm_restrict_gfp_mask() stacking") > > Signed-off-by: Youngjun Park > > --- > > include/linux/suspend.h | 1 + > > kernel/power/main.c | 7 +++++++ > > kernel/power/user.c | 4 ++-- > > 3 files changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/suspend.h b/include/linux/suspend.h > > index b02876f1ae38..7777931d88a5 100644 > > --- a/include/linux/suspend.h > > +++ b/include/linux/suspend.h > > @@ -454,6 +454,7 @@ extern void pm_report_hw_sleep_time(u64 t); > > extern void pm_report_max_hw_sleep(u64 t); > > void pm_restrict_gfp_mask(void); > > void pm_restore_gfp_mask(void); > > +void pm_restore_gfp_mask_safe(void); > > > > #define pm_notifier(fn, pri) { \ > > static struct notifier_block fn##_nb = \ > > diff --git a/kernel/power/main.c b/kernel/power/main.c > > index 5f8c9e12eaec..e610a8c8b7ff 100644 > > --- a/kernel/power/main.c > > +++ b/kernel/power/main.c > > @@ -36,6 +36,13 @@ > > static unsigned int saved_gfp_count; > > static gfp_t saved_gfp_mask; > > > > +void pm_restore_gfp_mask_safe(void) > > +{ > > + if (!saved_gfp_count) > > + return; > > + pm_restore_gfp_mask(); > > +} > > + > > void pm_restore_gfp_mask(void) > > { > > WARN_ON(!mutex_is_locked(&system_transition_mutex)); > > diff --git a/kernel/power/user.c b/kernel/power/user.c > > index 3e41544b99d5..41cff6a89a1c 100644 > > --- a/kernel/power/user.c > > +++ b/kernel/power/user.c > > @@ -306,7 +306,7 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd, > > case SNAPSHOT_UNFREEZE: > > if (!data->frozen || data->ready) > > break; > > - pm_restore_gfp_mask(); > > + pm_restore_gfp_mask_safe(); > > free_basic_memory_bitmaps(); > > data->free_bitmaps = false; > > thaw_processes(); > > @@ -318,7 +318,7 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd, > > error = -EPERM; > > break; > > } > > - pm_restore_gfp_mask(); > > + pm_restore_gfp_mask_safe(); > > error = hibernation_snapshot(data->platform_support); > > if (!error) { > > error = put_user(in_suspend, (int __user *)arg); > > -- > > AFAICS, this patch doesn't depend on the rest of the series, so I can > apply it separately unless there is a problem with that. > > However, for the other 2 patches in the series, I'd need some tags > (preferably Reviewed-by) from mm people. > > Thanks! Hi Rafael, While double-checking the code based on Andrew’s AI-assisted review, I noticed I missed one case. If userspace issues SNAPSHOT_FREEZE and then closes the device, snapshot_release() may call pm_restore_gfp_mask() without a matching restriction, which reproduces the same WARN. So we should switch snapshot_release() to pm_restore_gfp_mask_safe() as well. Also, since the safe wrapper may return early when saved_gfp_count == 0, the locking assertion would be skipped in that path. To preserve the invariant, it is better to keep: WARN_ON(!mutex_is_locked(&system_transition_mutex)); in the wrapper too. This modification is intentional, but after review I think this is better. I will update the patch and resend. Best regards, Youngjun park