Received: by 2002:a05:6902:102b:0:0:0:0 with SMTP id x11csp997154ybt; Sun, 14 Jun 2020 07:01:35 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyeBqLTNd8PdkygVbKnkx9vLoGVdhqYU2P9B/3lHFABVPa6NRmuhYVEc02uNoUHnVjxUkHF X-Received: by 2002:a05:6402:228d:: with SMTP id cw13mr20846389edb.150.1592143294903; Sun, 14 Jun 2020 07:01:34 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1592143294; cv=none; d=google.com; s=arc-20160816; b=UJHPk879ShWWE1BfyZEqRzRTY363GTM4B6AZJE+9vupUJjw6sUHZSlZjpnVoeIECph /XYa6Bs/5MXC3Wp+FOcAQ4e5s9cxAem1Pj3GQyPIP2v0vgM7JGW+xX3x+s209deyLvj6 3RbKG2B2rPlk54gfyJWwX0mCBx1XtkDiutY6qzg8nMs1octTHzM11H5Bw6x31fHiRF3K Rs8VdyVdj5oLaPdELmdQm0m0mMJjMxmtpgVOMqf5ZROJtP3q84CfERHIr2X1EAg7kFLj wW6wr5N/b+CvaA6Kty6xvDe1xYIzKKc5Q4f5Svbq2k3qgRsjP6TefGkVUeoPsIofcFv8 VL9w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version; bh=233qwlac7lV7KoqxYlqhSajK4Yb8g+fjpSk+cJ2lV3c=; b=uFpwRfE6EUnseyoIc0uFp8fk2GiubPCUKafgBhEAzVpZRFZhAWPfr5J6dWaMWKicFB Wu214yH23ELhc+siVC1KCWCvVjHnX0ooFU8d9zB6RfO2FBpvT9Pv9u2BUdkMELndkRp+ t+PQHMEdsOg7hgIIgQUg6mJGjULG5Se5cjLiy0a3SDHR2g/vkzL8VXaexbXXVCh5JEpI bDeyp3IFCIL3/Ig6xCHRQv6/0XWeMYTEVAFg1Qv/5jqJsfDQoag6gHHdBI0NEab0wu11 6LCezSW65ys2YT9RltZ0sT9z7dBcNrI2wpfwnXsYwDfQ11/vctLBX219fUfg/1O8Srss Jy+Q== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id r21si7413581ejx.647.2020.06.14.07.01.11; Sun, 14 Jun 2020 07:01:34 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726846AbgFNN73 (ORCPT + 99 others); Sun, 14 Jun 2020 09:59:29 -0400 Received: from mail-oi1-f193.google.com ([209.85.167.193]:43553 "EHLO mail-oi1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725949AbgFNN72 (ORCPT ); Sun, 14 Jun 2020 09:59:28 -0400 Received: by mail-oi1-f193.google.com with SMTP id j189so13424045oih.10; Sun, 14 Jun 2020 06:59:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=233qwlac7lV7KoqxYlqhSajK4Yb8g+fjpSk+cJ2lV3c=; b=bbesngi9tm9Nlv5RumNBfZ1BMetybLQYp/R5aJE8Cr0pDFcFmyVSdjqa+QajAmqxWg MneElEC/3pvgr4RXVE+hXbBLxCvS+bipkS0J45+WeCuyXu2lZmkDxUdG1j5fzAYfRDOJ vZ9K5r1CKVdup6g6UpxjJ2GDZv/P+8S6rSK0paGyDPpItBjFvis5GvqUBlsDeBcSsB4o 2K42bVvt4HyqXhwsUZjehu8JzpkgYPImOfdcYjjrh9JjxwOzPWmdIn90DucL6l42TIh8 LN4pVOO5kFSHpIrYQYsekLyXMZV3O6YULik5aHt6kuq1f/m4cDOMUVISqkN+GL9xy8aA KzgQ== X-Gm-Message-State: AOAM532pPPngTG6q6EhHE5ZLj0Y7Q21we8KavuzIUZNECZmxahQNzNF/ gZLQwDV1UGhJU+iGBvCVy2Smlp6t5ll7jhRAuVdmvA== X-Received: by 2002:aca:ad88:: with SMTP id w130mr6009881oie.103.1592143166177; Sun, 14 Jun 2020 06:59:26 -0700 (PDT) MIME-Version: 1.0 References: <20200614090751.GA2878@kunai> In-Reply-To: From: "Rafael J. Wysocki" Date: Sun, 14 Jun 2020 15:59:15 +0200 Message-ID: Subject: Re: RFC: a failing pm_runtime_get increases the refcnt? To: Geert Uytterhoeven Cc: Andy Shevchenko , Wolfram Sang , Linux PM , "Rafael J. Wysocki" , Linux-Renesas , Linux Kernel Mailing List , linux-i2c Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Jun 14, 2020 at 12:00 PM Geert Uytterhoeven wrote: > > Hi Andy, > > On Sun, Jun 14, 2020 at 11:43 AM Andy Shevchenko > wrote: > > On Sun, Jun 14, 2020 at 12:34 PM Andy Shevchenko > > wrote: > > > > > > On Sun, Jun 14, 2020 at 12:10 PM Wolfram Sang wrote: > > > > both in the I2C subsystem and also for Renesas drivers I maintain, I am > > > > starting to get boilerplate patches doing some pm_runtime_put_* variant > > > > because a failing pm_runtime_get is supposed to increase the ref > > > > counters? Really? This feels wrong and unintuitive to me. > > > > > > Yeah, that is a well known issue with PM (I even have for a long time > > > a coccinelle script, when I realized myself that there are a lot of > > > cases like this, but someone else discovered this recently, like > > > opening a can of worms). > > > > > > > I expect there > > > > has been a discussion around it but I couldn't find it. > > > > > > Rafael explained (again) recently this. I can't find it quickly, unfortunately. > > > > I _think_ this discussion, but may be it's simple another tentacle of > > the same octopus. > > https://patchwork.ozlabs.org/project/linux-tegra/patch/20200520095148.10995-1-dinghao.liu@zju.edu.cn/ > > Thanks, hadn't read that one! (so I was still at -1 from > http://sweng.the-davies.net/Home/rustys-api-design-manifesto ;-) > > So "pm_runtime_put_noidle()" is the (definitive?) one to pair with a > pm_runtime_get_sync() failure? If you bail out immediately on errors, then yes, it is. If you'd rather to something like ret = pm_runtime_get_sync(dev); if (ret < 0) goto fail; ... code depending on PM ... fail: pm_runtime_put_autosuspend(dev); then it will still work correctly. It actually doesn't matter which pm_runtime_put*() variant you call after a pm_runtime_get_sync() failure, but the _noidle() is the simplest one and it is sufficient. > > > > I wonder why we > > > > don't fix the code where the incremented refcount is expected for some > > > > reason. > > > > > > The main idea behind API that a lot of drivers do *not* check error > > > codes from runtime PM, so, we need to keep balance in case of > > > > > > pm_runtime_get(...); > > > ... > > > pm_runtime_put(...); > > I've always[*] considered a pm_runtime_get_sync() failure to be fatal > (or: cannot happen), and that there's nothing that can be done to > recover. Hence I never checked the function's return value. > Was that wrong? No, it wasn't. It is the right thing to do in the majority of cases. > [*] at least on Renesas SoCs with Clock and/or Power Domains. Cheers!