public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tzung-Bi Shih <tzungbi@kernel.org>
To: Chen-Yu Tsai <wenst@chromium.org>
Cc: Benson Leung <bleung@chromium.org>,
	Guenter Roeck <groeck@chromium.org>,
	AngeloGioacchino Del Regno 
	<angelogioacchino.delregno@collabora.com>,
	chrome-platform@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] platform/chromeos: cros_ec: Use per-device lockdep key
Date: Fri, 6 Jan 2023 17:08:27 +0800	[thread overview]
Message-ID: <Y7flCydDtLVwsXIJ@google.com> (raw)
In-Reply-To: <20230106045537.1243887-1-wenst@chromium.org>

On Fri, Jan 06, 2023 at 12:55:37PM +0800, Chen-Yu Tsai wrote:
> Lockdep reports a bogus possible deadlock on MT8192 Chromebooks due to
> the following lock sequences:
> 
> 1. lock(i2c_register_adapter) [1]; lock(&ec_dev->lock)
> 2. lock(&ec_dev->lock); lock(prepare_lock);
> 
> The actual dependency chains are much longer. The shortened version
> looks somewhat like:
> 
> 1. cros-ec-rpmsg on mtk-scp
>    ec_dev->lock -> prepare_lock
> 2. In rt5682_i2c_probe() on native I2C bus:
>    prepare_lock -> regmap->lock -> (possibly) i2c_adapter->bus_lock
> 3. In rt5682_i2c_probe() on native I2C bus:
>    regmap->lock -> i2c_adapter->bus_lock
> 4. In sbs_probe() on cros-ec-i2c (passthrough) I2C bus on cros-ec
>    i2c_adapter->bus_lock -> ec_dev->lock
> 
> While lockdep is correct that the shared lockdep classes have a circular
> dependency, it is bogus because
> 
>   a) 2+3 happen on a native I2C bus
>   b) 4 happens on the actual EC on ChromeOS devices
>   c) 1 happens on the SCP coprocessor on MediaTek Chromebooks that just
>      happen to expose a cros-ec interface, but do not have a passthrough
>      I2C bus
> 
> In short, the "dependencies" are actually on different devices.

Path of 4 looks weird to me.

Could you point out where sbs_probe() gets to acquire ec_dev->lock?

I may misunderstand: I thought there is no such I2C bus for passthrough
from kernel's point of view (as the bus and devices behind the EC).
See also [2].

[2]: https://elixir.bootlin.com/linux/v6.2-rc2/source/drivers/platform/chrome/cros_ec.c#L241


On a related note, for the commit title: s/chromeos/chrome/ if it gets
chance to have next version.

  reply	other threads:[~2023-01-06  9:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-06  4:55 [PATCH] platform/chromeos: cros_ec: Use per-device lockdep key Chen-Yu Tsai
2023-01-06  9:08 ` Tzung-Bi Shih [this message]
2023-01-07  5:43   ` Chen-Yu Tsai
2023-01-09  5:46     ` Tzung-Bi Shih
2023-01-09  6:19       ` Chen-Yu Tsai
2023-01-09  7:30         ` Tzung-Bi Shih
2023-01-09  7:35           ` Chen-Yu Tsai
2023-01-11  6:04             ` Tzung-Bi Shih

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y7flCydDtLVwsXIJ@google.com \
    --to=tzungbi@kernel.org \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=bleung@chromium.org \
    --cc=chrome-platform@lists.linux.dev \
    --cc=groeck@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=wenst@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox