linux-kernel - Re: [PATCH RFC 3/6] iio: core: Add cleanup.h support for iio_device_claim

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DES3YOKEMZ16.2OIHKXA8ESBUX@gmail.com>
Date: Sun, 07 Dec 2025 11:00:28 -0500
From: "Kurt Borja" <kuurtb@...il.com>
To: "Jonathan Cameron" <jic23@...nel.org>, "Andy Shevchenko"
 <andy.shevchenko@...il.com>
Cc: "Kurt Borja" <kuurtb@...il.com>, "David Lechner"
 <dlechner@...libre.com>, "Andy Shevchenko" <andriy.shevchenko@...el.com>,
 "Lars-Peter Clausen" <lars@...afoo.de>, "Michael Hennerich"
 <Michael.Hennerich@...log.com>, "Benson Leung" <bleung@...omium.org>,
 "Antoniu Miclaus" <antoniu.miclaus@...log.com>, "Gwendal Grignou"
 <gwendal@...omium.org>, "Shrikant Raskar" <raskar.shree97@...il.com>,
 "Per-Daniel Olsson" <perdaniel.olsson@...s.com>, Nuno Sá
 <nuno.sa@...log.com>, "Andy Shevchenko" <andy@...nel.org>, "Guenter Roeck"
 <groeck@...omium.org>, "Jonathan Cameron" <Jonathan.Cameron@...wei.com>,
 <linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <chrome-platform@...ts.linux.dev>
Subject: Re: [PATCH RFC 3/6] iio: core: Add cleanup.h support for
 iio_device_claim_*()

On Sat Dec 6, 2025 at 1:43 PM -05, Jonathan Cameron wrote:
> On Thu, 4 Dec 2025 19:36:28 +0200
> Andy Shevchenko <andy.shevchenko@...il.com> wrote:
>
>> On Thu, Dec 4, 2025 at 7:18 PM Kurt Borja <kuurtb@...il.com> wrote:
>> > On Wed Dec 3, 2025 at 5:34 PM -05, David Lechner wrote:  
>> > > On 12/3/25 3:50 PM, David Lechner wrote:  
>> > >> On 12/3/25 1:18 PM, Kurt Borja wrote:  
>> 
>> ...
>> 
>> > > When I made the comments about keeping "mode" in the name, I forgot
>> > > that DEFINE_GUARD_COND() only extends a DEFINE_GUARD(). So I understand
>> > > if we need to make names that fit a certain pattern rather than what
>> > > I suggested.
>> > >
>> > > Still would be nice to have:
>> > >
>> > > iio_device_claim_mode()
>> > > iio_device_claim_mode_direct()
>> > > iio_device_claim_mode_buffer()
>> > > iio_device_release_mode()
>> > >
>> > > Just really annoying to rename iio_device_{claim,release}_direct()
>> > > everywhere since we just did that. We could keep both names around
>> > > for a while though to avoid the churn.  
>
> Definitely. Possibly indefinitely. I don't want a rename just to make
> this facility easier to use as people won't see what is under the
> ACQUIRE() anyway if we end up doing something like Rafael has done for
> runtime PM where you don't call ACQUIRE() directly but use a runtime pm
> specific macro (not sure if that will make this cycle or not, was
> still being discussed when I went on holiday).
>
> https://lore.kernel.org/all/3400866.aeNJFYEL58@rafael.j.wysocki/

That looks nice.

>
>
>> >
>> > If we rename iio_device_claim_direct() (which is huge), maybe we can
>> > pick shorter names and more descriptive names while at it? I was
>> > thinking something like:
>> >
>> >         iio_mode_lock()
>> >         iio_mode_lock_direct()
>> >         iio_mode_lock_buffer()
>> >         iio_mode_unlock()  
>> 
>> The device context is important, so at least iio_dev_mode_lock() (and so on).
>
> If we are bringing lock into the name do we need to make it explicit which can fail?
> Given you can't use them in the wrong place, maybe not.
>
> iio_mode_lock_try_direct() or maybe iio_mode_lock_direct_try()?

As Andy mentioned, maybe iio_mode_trylock_{direct,buffer}()?

>
> This was less relevant when they all could fail.  Maybe we don't need to
> bother given how rarely used the unconditional ones are.
>
> I did like the claiming of mode terminology because it made it a little
> more clear that we were taking a lock that was there for a purpose rather than
> a normal lock.  Also the fact it's a lock is an implementation detail I'd
> rather not back into the ABI.

Even if it's an implementation detail, from what I've seen, a lot of
drivers might depend on this being a lock.

I my other series (ti-ads1x18), I dropped my private lock in favor of
iio_device_claim_direct() per David's suggestion too.

>
> Maybe it's worth something inspired by Rafael's patch linked above?
>
> #define IIO_DEV_ACQUIRE_DIRECT_MODE(_dev, _var) \
> 	ACQUIRE(iio_device_claim_direct, _var)(_dev);
> #define IIO_DEV_ACQUIRE_BUFFER_MODE(_dev, _var) \
> 	ACQUIRE(iio_device_claim_buffer, _var)(_dev);

I like this a lot, I'll add it here.

>
> For the two more complex ones and fine using guard() for the rare
> any mode variant.
>
> Then we can have whatever naming we like for the helpers under
> the hood as no one will ever use them directly.

Yes, I do think a rename would be nice, but maybe we can leave that for
a (near) future patch.

On that note, should I really rename iio_device_claim_buffer_mode()?
With this macros I don't think is necessary anymore.

>
> Hohum. Hardest problems in computer science etc, coherency and naming. :)

Indeed :)


-- 
 ~ Kurt