All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joshua Crofts <joshua.crofts1@gmail.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: "Fabio Cesari" <fabio.cesari@gmail.com>,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Brian Masney" <bmasney@redhat.com>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iio: light: isl29028: fix runtime PM reference leak on error paths
Date: Mon, 7 Sep 2026 09:29:24 +0200	[thread overview]
Message-ID: <20260907092924.00000e80@gmail.com> (raw)
In-Reply-To: <20260907030751.3da628d4@jic23-huawei>

On Mon, 7 Sep 2026 03:07:51 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon,  7 Sep 2026 00:15:17 +0200
> Fabio Cesari <fabio.cesari@gmail.com> wrote:
> 
> > Hi Jonathan,
> > 
> > On Sun, 6 Sep 2026 18:43:58 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >   
> > > Whilst perhaps not best practice as such, it is pretty
> > > common to just not bother checking the return of pm_runtime_put_autosuspend()
> > > at least partly because of that annoying -ENOSYS result if runtime pm isn't
> > > enabled.  That is what happens with the ACQUIRE macros for instance.
> > >
> > > Do we have any particular reason to thing it is more likely to fail i this
> > > case than any other?    
> > 
> > No reason: both functions already returned that value to userspace before the
> > patch, and I left that unchanged only because the patch was aimed at the leak.
> > It makes sense to fix it as well.
> > 
> > v2 will take the reference with PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND() in
> > both functions, as Joshua Crofts suggested, so the value goes away along with
> > the manual put.  
> 
> Why the if enabled variant?  I think that only makes sense in places that can
> be hit prior to runtime pm being up and running. Maybe that's true here.

Yeah, my mistake, I suggested using the IF_ENABLED variant. All reads would fail
if PM is disabled since the macro would throw an error...

The regular PM_RUNTIME_ACQUIRE_AUTOSUSPEND() is adequate.

-- 
Kind regards,
Joshua Crofts

  reply	other threads:[~2026-09-07  7:29 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-06 13:11 [PATCH] iio: light: isl29028: fix runtime PM reference leak on error paths Fabio Cesari
2026-09-06 13:55 ` Joshua Crofts
2026-09-06 22:09   ` Fabio Cesari
2026-09-06 17:43 ` Jonathan Cameron
2026-09-06 22:15   ` Fabio Cesari
2026-09-07  2:07     ` Jonathan Cameron
2026-09-07  7:29       ` Joshua Crofts [this message]
2026-09-09 16:21         ` Fabio Cesari
2026-09-06 22:37 ` [PATCH v2] " Fabio Cesari
2026-09-07  7:19   ` Joshua Crofts
2026-09-07 11:02     ` Fabio Cesari

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260907092924.00000e80@gmail.com \
    --to=joshua.crofts1@gmail.com \
    --cc=andy@kernel.org \
    --cc=bmasney@redhat.com \
    --cc=dlechner@baylibre.com \
    --cc=fabio.cesari@gmail.com \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.