Received-SPF: pass (google.com: domain of linux-kernel+bounces-129897-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Wed, 3 Apr 2024 14:58:49 +0100
From: Daniel Thompson <daniel.thompson@linaro.org>
To: liu.yec@h3c.com
Cc: dianders@chromium.org, gregkh@linuxfoundation.org,
	jason.wessel@windriver.com, jirislaby@kernel.org,
	kgdb-bugreport@lists.sourceforge.net, linux-kernel@vger.kernel.org,
	linux-serial@vger.kernel.org
Subject: Re: [PATCH V8] kdb: Fix the deadlock issue in KDB debugging.
Message-ID: <20240403135849.GE25200@aspen.lan>
References: <20240402125802.GC25200@aspen.lan>
 <20240403061109.3142580-1-liu.yec@h3c.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240403061109.3142580-1-liu.yec@h3c.com>

On Wed, Apr 03, 2024 at 02:11:09PM +0800, liu.yec@h3c.com wrote:
> From: LiuYe <liu.yeC@h3c.com>
>
> Currently, if CONFIG_KDB_KEYBOARD is enabled, then kgdboc will
> attempt to use schedule_work() to provoke a keyboard reset when
> transitioning out of the debugger and back to normal operation.
> This can cause deadlock because schedule_work() is not NMI-safe.
>
> <snip>
>
> We fix the problem by using irq_work to call schedule_work()
> instead of calling it directly. This is because we cannot
> resynchronize the keyboard state from the hardirq context
> provided by irq_work. This must be done from the task context
> in order to call the input subsystem.
>
> Therefore, we have to defer the work twice. First, safely
> switch from the debug trap context (similar to NMI) to the
> hardirq, and then switch from the hardirq to the system work queue.
>
> Signed-off-by: LiuYe <liu.yeC@h3c.com>
> Co-authored-by: Daniel Thompson <daniel.thompson@linaro.org>
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>

I'm happy with how this is looking. In the long term it might be good to
move the keyboard resync code so it is with the rest of the kdb keyboard
code rather than in tty/serial. However I certainly don't want to tangle
that kind of clean up along with a bug fix so I think this is ready to
go now.

@Greg: I assume you want to take this via the tty/serial tree? I
contributed a fair bit to the eventual patch so a Reviewed-by from me
probably isn't appropriate but if you want to take the code it is
certainly:
Acked-by: Daniel Thompson <daniel.thompson@linaro.org>


Daniel.