linux-kernel - Re: [PATCH v2 3/8] iio: accel: kx022a: Use cleanup.h helpers

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] [day] [month] [year] [list]

Message-ID: <20241126175550.4a8bedf3@jic23-huawei>
Date: Tue, 26 Nov 2024 17:55:50 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>, Lars-Peter Clausen
 <lars@...afoo.de>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/8] iio: accel: kx022a: Use cleanup.h helpers

On Mon, 25 Nov 2024 11:34:36 +0200
Matti Vaittinen <mazziesaccount@...il.com> wrote:

> Hello Jonathan,
> 
> Thanks again!
> 
> On 23/11/2024 18:42, Jonathan Cameron wrote:
> > On Thu, 21 Nov 2024 10:20:23 +0200
> > Matti Vaittinen <mazziesaccount@...il.com> wrote:
> >   
> >> A few functions in KX022A need to use mutex for protecting the
> >> enabling/disabling of the measurement while configurations are being
> >> made. Some of the functions can be slightly simplified by using the
> >> __cleanup based scoped mutexes, which allows dropping the goto based
> >> unlocking at error path.
> >>
> >> Simplify error paths using guard(mutex).
> >>
> >> Signed-off-by: Matti Vaittinen <mazziesaccount@...il.com>  
> > Now we have guard(), the main reason (I think) for the
> > combined on + lock and off + unlock paths is gone. So can
> > we just flatten those and do the locking at caller.  
> 
> I did consider this too :)
> 
> Why I decided to keep it as it is, (even though we need the extra 
> mutex_unlock() at certain error path) is because I kind of like the 
> lock+off and unlock+on functions. This locking does not protect data, 
> but really a sequence of operations that needs to be done while sensor 
> is OFF state. It's almost like a doc saying that "please, ensure the 
> sensor is OFF for the following operations" :)

hmm.  I really don't like them because they are 'unusual'  :)

I'd argue they just ensure a sequence of writes go in as an atomic thing.
Two of those writes happen to be turn it off and turn it on.

So the data the are protecting is the device internal state data.

> 
> (Another thing is that we do claim the direct mode in write_raw, and 
> goto is still handy for releasing it. Scoped guards won't play nicely 
> with goto. Yes, we could probably use the __cleanup for direct mode, but 
> I still like the lock+off, unlock+on for the reason above)
There is a nice new cleanup that David did to make the direct mode
handling much cleaner.

	if_not_cond_guard(iio_claim_direct_try, indio_dev)
		return -EBUSY;


> 
> Yours,
> 	-- Matti
>