Received-SPF: pass (google.com: domain of linux-kernel+bounces-174725-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <SN7PR11MB660461A6E5F3FB4546B5BDB9D9E62@SN7PR11MB6604.namprd11.prod.outlook.com>
In-Reply-To: <SN7PR11MB660461A6E5F3FB4546B5BDB9D9E62@SN7PR11MB6604.namprd11.prod.outlook.com>
From: Muni Sekhar <munisekharrms@gmail.com>
Date: Thu, 9 May 2024 22:50:33 +0530
Message-ID: <CAHhAz+hvb+8sT3BYS_pT3Lmi5X4PjYWRJR=hvmX1e4Ays512FA@mail.gmail.com>
Subject: Re: Seeking Assistance with Spin Lock Usage and Resolving Hard LOCKUP Error
To: "Billie Alsup (balsup)" <balsup@cisco.com>
Cc: kernelnewbies <kernelnewbies@kernelnewbies.org>, LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, May 9, 2024 at 8:56=E2=80=AFPM Billie Alsup (balsup) <balsup@cisco.=
com> wrote:
>
> >From: Muni Sekhar <munisekharrms@gmail.com>
>
> >Here is a brief overview of how I have implemented spin locks in my modu=
le:
> >
> >spinlock_t my_spinlock; // Declare a spin lock
> >
> >// In ISR context (interrupt handler):
> >spin_lock_irqsave(&my_spinlock, flags);
> >// ... Critical section ...
> >spin_unlock_irqrestore(&my_spinlock, flags);
> >
> >
> >// In process context: (struct file_operations.read)
> >spin_lock(&my_spinlock);
> >// ... Critical section ...
> >spin_unlock(&my_spinlock);
>
> from my understanding, you have the usage backwards.  It is the irqsave/i=
rqrestore versions that should be used within process context to prevent th=
e interrupt from being handled on the same cpu while executing in your crit=
ical section.
>
> The use of irqsave/irqrestore within the isr itself is ok, although perha=
ps unnecessary.  It depends on whether the interrupt can occur again while =
you are servicing the interrupt (whether on this cpu or another).  Usually =
(?) the same interrupt does not nest, unless you have explicitly coded to a=
llow it (for example, by acknowledging and re-enabling the interrupt early =
in your ISR). Certainly the spinlock is necessary to protect the critical s=
ection from running in an isr on one cpu and process space on another cpu.
>
In the scenario where an interrupt occurs while we are servicing the
interrupt, and in the scenario where it doesn't occur while we are
servicing the interrupt, when should we use the
spin_lock_irqsave/spin_unlock_irqrestore APIs?
> From a lockup perspective, not doing the irqsave/irqrestore from process =
context could explain your problem. Also look for code (anywhere!) that bli=
ndly enables interrupts, rather than doing irqrestore from a prior irqsave.


--=20
Thanks,
Sekhar