Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 27 Jun 2023 19:57:44 +0800
From:   Kent Gibson <warthog618@gmail.com>
To:     Bartosz Golaszewski <brgl@bgdev.pl>
Cc:     YE Chengfeng <cyeaa@connect.ust.hk>,
        "linus.walleij@linaro.org" <linus.walleij@linaro.org>,
        "andy@kernel.org" <andy@kernel.org>,
        "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] gpio: gpiolib-cdev: Fix potential &lr->wait.lock
 deadlock issue
Message-ID: <ZJrOuLXmnxuRlbLL@sol>
References: <TYCP286MB1188B6A0379F7928C63288DF8A21A@TYCP286MB1188.JPNP286.PROD.OUTLOOK.COM>
 <ZJk87rWsDj7pWJIP@sol>
 <CAMRc=McGOV+A8ROXRinUkWG0POTe37RWphRHDW6sy7UMU-e2UA@mail.gmail.com>
 <ZJo+wBMsyHvu/7Mj@sol>
 <CAMRc=MeyEuZpDXsZHVGdrivZbYjHr1YDiYAbu4A0--Xy9rKVUw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAMRc=MeyEuZpDXsZHVGdrivZbYjHr1YDiYAbu4A0--Xy9rKVUw@mail.gmail.com>
Precedence: bulk

On Tue, Jun 27, 2023 at 01:47:16PM +0200, Bartosz Golaszewski wrote:
> On Tue, Jun 27, 2023 at 3:43 AM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Mon, Jun 26, 2023 at 05:50:47PM +0200, Bartosz Golaszewski wrote:
> > > On Mon, Jun 26, 2023 at 9:23 AM Kent Gibson <warthog618@gmail.com> wrote:
> > > >
> > > >
> >
> > While we are on the subject of spin_locks, why does
> > gpio_desc_to_lineinfo() use spin_lock_irqsave()?
> > I assume the _irq is necessary as the desc could be updated at interrupt
> > level, but AFAICT gpio_desc_to_lineinfo() is only ever called from process
> > context, so why not just spin_lock_irq()?
> >
> > Cheers,
> > Kent.
> 
> Didn't we use an atomic notifier before for some reason? Then it got
> changed to blocking but the lock stayed like this? It does look like
> spin_lock_irq() would be fine here. On the other hand - if something
> isn't broken... :)
> 

Yeah, it was atomic before blocking, but that doesn't explain the need
for the save - interrupts are always enabled in that function.
Not a big difference either way, and irqsave is always safe, so no
problem with leaving it as is - I just thought it odd when I noticed it,
while spin locks and context were front of mind.

Cheers,
Kent.