Re: [PATCH v11 29/36] phy: core: add notifier infrastructure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Potential self-deadlock or AB-BA deadlock between `phy->mutex` and `phy->notifier.rwsem` when consumers handle reset notifications.
--

--- Patch [29]: [PATCH v11 29/36] phy: core: add notifier infrastructure ---
commit 205cc0767ab713005453a3401f261c9b0a0289ce
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

phy: core: add notifier infrastructure

This commit introduces a new PHY notifier API to allow consumer drivers to
handle reset events coming from a PHY device. It adds functions to register
and unregister notifiers, and to trigger reset notifications.

> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
[ ... ]
> +int phy_notify_reset(struct phy *phy, enum phy_notification event)
> +{
> +	int ret;
> +
> +	if (!phy)
> +		return 0;
> +
> +	ret = blocking_notifier_call_chain(&phy->notifier, event, phy);

[Severity: Medium]
Could this lead to a self-deadlock or an AB-BA deadlock between phy->mutex
and the notifier rwsem?

If a PHY provider calls phy_notify_reset() from within a PHY operation
callback that executes with phy->mutex held, the notifier chain takes a
read lock on phy->notifier.rwsem. If a consumer callback then attempts to
quiesce its hardware by calling a PHY framework API like phy_power_off(),
it will try to acquire phy->mutex again, resulting in a self-deadlock:

phy_set_mode()
  mutex_lock(&phy->mutex);
  provider_ops->set_mode()
    phy_notify_reset()
      blocking_notifier_call_chain()  // takes rwsem read lock
        consumer_callback()
          phy_power_off()
            mutex_lock(&phy->mutex);  // self-deadlock

Conversely, if phy_notify_reset() is called outside of phy->mutex, but the
consumer calls back into the PHY framework, it establishes a lock dependency
from the notifier rwsem to phy->mutex. A concurrent PHY operation that holds
phy->mutex and registers a notifier could create a classic AB-BA deadlock if
a writer is pending on the rwsem.

Should the documentation warn consumers to defer PHY API calls to a
workqueue when handling these notifications, or should the lock hierarchy
be explicitly defined?

> +	return notifier_to_errno(ret);
> +}
> +EXPORT_SYMBOL_GPL(phy_notify_reset);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=29

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy