Re: [PATCH v10 06/69] drm/probe-helper: Introduce .force_ctx() connector callback

[email protected]
Newsgroups org.freedesktop.lists.dri-devel,dev.linux.lists.linux-sunxi
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Modifying `connector->status` before `force_ctx()` can return `-EDEADLK` causes subsequent retries to incorrectly evaluate `old_status == connector->status`, silently skipping the hotplug event.
--

--- Patch [6]: [PATCH v10 06/69] drm/probe-helper: Introduce .force_ctx() connector callback ---
commit b08054d3073ea6707b0fe9e517dde416f7e5c8b6
Author: Cristian Ciocaltea <[email protected]>

drm/probe-helper: Introduce .force_ctx() connector callback

This commit introduces a force_ctx() variant to the connector helper
functions, taking a modeset acquire context and returning an error code
so that -EDEADLK can propagate. It takes precedence over the existing
force() callback when both are implemented.

> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index e26ead5b5e589..918255bcfa096 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c

[ ... ]

> @@ -592,8 +593,19 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>  			connector->status = connector_status_connected;
>  		else
>  			connector->status = connector_status_disconnected;
> -		if (connector->funcs->force)
> +
> +		if (funcs && funcs->force_ctx) {
> +			ret = funcs->force_ctx(connector, &ctx);
> +			if (ret == -EDEADLK) {
> +				drm_modeset_backoff(&ctx);
> +				goto retry;

[Severity: Medium]
Does modifying connector->status before force_ctx() cause hotplug events
to be dropped on lock contention?

If funcs->force_ctx() returns -EDEADLK, the code branches to the retry
label. During the retry loop, old_status is assigned the current
connector->status:

drm_helper_probe_single_connector_modes() {
retry:
        ...
        old_status = connector->status;

Since connector->status was already updated prior to the failed attempt,
old_status will equal the new status. When the lock is finally acquired
after the retry, the old_status != connector->status check later in the
function evaluates to false, causing the hotplug event detection logic
to be silently skipped.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.