[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250219163450.GJ19203@1wt.eu>
Date: Wed, 19 Feb 2025 17:34:51 +0100
From: Willy Tarreau <w@....eu>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: James Bottomley <James.Bottomley@...senPartnership.com>,
"Martin K. Petersen" <martin.petersen@...cle.com>,
Dan Carpenter <dan.carpenter@...aro.org>,
Christoph Hellwig <hch@...radead.org>,
Miguel Ojeda <miguel.ojeda.sandonis@...il.com>,
rust-for-linux <rust-for-linux@...r.kernel.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Greg KH <gregkh@...uxfoundation.org>, David Airlie <airlied@...il.com>,
linux-kernel@...r.kernel.org, ksummit@...ts.linux.dev
Subject: Re: Rust kernel policy
On Wed, Feb 19, 2025 at 06:32:11PM +0200, Laurent Pinchart wrote:
> > However I remember having faced code in the past where
> > developers had abused this "unlock on return" concept resulting in locks
> > lazily being kept way too long after an operation. I don't think this
> > will happen in the kernel thanks to reviews, but typically all the stuff
> > that's done after a locked retrieval was done normally is down outside
> > of the lock, while here for the sake of not dealing with unlocks, quite
> > a few lines were still covered by the lock for no purpose. Anyway
> > there's no perfect solution.
>
> There actually is in this case :-) You can reduce the scope with scoped
> guards:
>
> static int gpio_mockup_get_multiple(struct gpio_chip *gc,
> unsigned long *mask, unsigned long *bits)
> {
> struct gpio_mockup_chip *chip = gpiochip_get_data(gc);
> unsigned int bit, val;
>
> scoped_guard(mutex, &chip->lock) {
> for_each_set_bit(bit, mask, gc->ngpio) {
> val = __gpio_mockup_get(chip, bit);
> __assign_bit(bit, bits, val);
> }
> }
>
> return 0;
> }
>
> which is equivalent to
>
> static int gpio_mockup_get_multiple(struct gpio_chip *gc,
> unsigned long *mask, unsigned long *bits)
> {
> struct gpio_mockup_chip *chip = gpiochip_get_data(gc);
> unsigned int bit, val;
>
> {
> guard(mutex)(&chip->lock);
>
> for_each_set_bit(bit, mask, gc->ngpio) {
> val = __gpio_mockup_get(chip, bit);
> __assign_bit(bit, bits, val);
> }
> }
>
> return 0;
> }
>
> In this particular example there's nothing being done after the scope,
> but you could have more code there.
I see, excellent point!
Thanks,
Willy
Powered by blists - more mailing lists