Received-SPF: pass (google.com: domain of linux-kernel+bounces-110034-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 21 Mar 2024 13:31:31 +0100 (CET)
From: Jiri Kosina <jikos@kernel.org>
To: Nam Cao <namcao@linutronix.de>
cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>, 
    Douglas Anderson <dianders@chromium.org>, 
    Hans de Goede <hdegoede@redhat.com>, Maxime Ripard <mripard@kernel.org>, 
    Kai-Heng Feng <kai.heng.feng@canonical.com>, 
    Johan Hovold <johan+linaro@kernel.org>, linux-input@vger.kernel.org, 
    linux-kernel@vger.kernel.org, Eva Kurchatova <nyandarknessgirl@gmail.com>, 
    stable@vger.kernel.org
Subject: Re: [PATCH] HID: i2c-hid: remove I2C_HID_READ_PENDING flag to prevent
 lock-up
In-Reply-To: <20240318105902.34789-1-namcao@linutronix.de>
Message-ID: <nycvar.YFH.7.76.2403211330380.20263@cbobk.fhfr.pm>
References: <20240318105902.34789-1-namcao@linutronix.de>
User-Agent: Alpine 2.21 (LSU 202 2017-01-01)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII

On Mon, 18 Mar 2024, Nam Cao wrote:

> The flag I2C_HID_READ_PENDING is used to serialize I2C operations.
> However, this is not necessary, because I2C core already has its own
> locking for that.
> 
> More importantly, this flag can cause a lock-up: if the flag is set in
> i2c_hid_xfer() and an interrupt happens, the interrupt handler
> (i2c_hid_irq) will check this flag and return immediately without doing
> anything, then the interrupt handler will be invoked again in an
> infinite loop.
> 
> Since interrupt handler is an RT task, it takes over the CPU and the
> flag-clearing task never gets scheduled, thus we have a lock-up.
> 
> Delete this unnecessary flag.

Hmm, right, good catch, I can't figure out what extra semantic 
I2C_HID_READ_PENDING would be adding (rather than deadlock :) ).
Why RT throttling didn't happen and let the system in a completely locked 
up state is something I don't understand, but that's separate.

I have now queued this in hid.git#for-6.9/upstream-fixes

Thanks,

-- 
Jiri Kosina
SUSE Labs