From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C2764C433E1 for ; Mon, 24 Aug 2020 17:31:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A0CE0206F0 for ; Mon, 24 Aug 2020 17:31:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728437AbgHXRbN (ORCPT ); Mon, 24 Aug 2020 13:31:13 -0400 Received: from cloudserver094114.home.pl ([79.96.170.134]:52580 "EHLO cloudserver094114.home.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728466AbgHXRbF (ORCPT ); Mon, 24 Aug 2020 13:31:05 -0400 Received: from 89-64-88-199.dynamic.chello.pl (89.64.88.199) (HELO kreacher.localnet) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer 0.83.459) id 9706e1a0a736e6e0; Mon, 24 Aug 2020 19:31:02 +0200 From: "Rafael J. Wysocki" To: Alan Stern Cc: Linux PM , LKML , Linux ACPI , Greg Kroah-Hartman , "Rafael J. Wysocki" , Mika Westerberg Subject: Re: [PATCH] PM: sleep: core: Fix the handling of pending runtime resume requests Date: Mon, 24 Aug 2020 19:31:01 +0200 Message-ID: <3393548.q2lFjJrsnI@kreacher> In-Reply-To: <20200824150421.GD329866@rowland.harvard.edu> References: <7969920.MVx1BpXlEM@kreacher> <4922509.6NPD9QEisq@kreacher> <20200824150421.GD329866@rowland.harvard.edu> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org On Monday, August 24, 2020 5:04:21 PM CEST Alan Stern wrote: > On Mon, Aug 24, 2020 at 03:36:36PM +0200, Rafael J. Wysocki wrote: > > > Furthermore, by the logic used in this patch, the call to > > > pm_wakeup_event() in the original code is also redundant: Any required > > > wakeup event should have been generated when the runtime resume inside > > > pm_runtime_barrer() was carried out. > > > > It should be redundant in the real wakeup event cases, but it may cause > > spurious suspend aborts to occur when there are no real system wakeup > > events. > > > > Actually, the original code is racy with respect to system wakeup events, > > because it depends on the exact time when the runtime-resume starts. Namely, > > if it manages to start before the freezing of pm_wq, the wakeup will be lost > > unless the driver takes care of reporting it, which means that drivers really > > need to do that anyway. And if they do that (which hopefully is the case), the > > pm_wakeup_event() call in the core may be dropped. > > In other words, wakeup events are supposed to be reported at the time > the wakeup request is first noticed, right? That's correct. > We don't want to wait until > a resume or runtime_resume callback runs; thanks to this race the > callback might not run at all if the event isn't reported first. The callback will run, either through the wq or by the pm_runtime_barrier(), but if it runs through the wq, pm_runtime_barrier() will return 0 and pm_wakeup_event() will not called by the core, so it must be called from elsewhere anyway. > Therefore the reasoning behind the original code appears to have been > highly suspect. Indeed. > If there already was a queued runtime-resume request > for the device and the device was wakeup-enabled, the wakeup event > should _already_ have been reported at the time the request was queued. > And we shouldn't rely on it being reported by the runtime-resume > callback routine. Right. > > > This means that the code could be simplified to just: > > > > > > pm_runtime_barrier(dev); > > > > Yes, it could, so I'm going to re-spin the patch with this code simplification > > and updated changelog. > > > > > Will this fix the reported bug? > > > > I think so. > > Okay, we'll see! Fair enough!