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=-5.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 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 33487C433DF for ; Mon, 24 Aug 2020 15:04:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 16B5B206F0 for ; Mon, 24 Aug 2020 15:04:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725973AbgHXPE0 (ORCPT ); Mon, 24 Aug 2020 11:04:26 -0400 Received: from netrider.rowland.org ([192.131.102.5]:48593 "HELO netrider.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1725947AbgHXPEW (ORCPT ); Mon, 24 Aug 2020 11:04:22 -0400 Received: (qmail 332615 invoked by uid 1000); 24 Aug 2020 11:04:21 -0400 Date: Mon, 24 Aug 2020 11:04:21 -0400 From: Alan Stern To: "Rafael J. Wysocki" 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 Message-ID: <20200824150421.GD329866@rowland.harvard.edu> References: <7969920.MVx1BpXlEM@kreacher> <20200821193442.GA264863@rowland.harvard.edu> <4922509.6NPD9QEisq@kreacher> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4922509.6NPD9QEisq@kreacher> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org 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? 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. Therefore the reasoning behind the original code appears to have been highly suspect. 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. > > 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! Alan Stern