kms/connector: Don't query the kernel twice when updating

On hotplug, the events we receive from the kernel are async, and
connectors in the kernel come and go as they please. In practice, this
means that calling drmModeGetConnector() twice more or less directly
after each other, there is no guarantee that the latter call will return
anything if the former did.

When updating the connector in response to hotplugs, we'd first update
the list of existing connectors, and following that, query each and
every one again for their current state, to update our internal
representation; only the former handled drmModeGetConnector() returning
NULL, meaning if unlucky, we'd end up doing a null pointer dereference
when trying to update the state.

Handle this by querying the kernel for the current connector state only
once per connector, updating the list of connectors and their
corresponding state at the same time.

Fixes the following crash:

    #0 meta_kms_connector_read_state at ../src/backends/native/meta-kms-connector.c:684
    #1 meta_kms_connector_update_state at ../src/backends/native/meta-kms-connector.c:767
    #2 meta_kms_impl_device_update_states at ../src/backends/native/meta-kms-impl-device.c:916
    #3 meta_kms_device_update_states_in_impl at ../src/backends/native/meta-kms-device.c:267
    #4 meta_kms_update_states_in_impl at ../src/backends/native/meta-kms.c:604
    #5 update_states_in_impl at ../src/backends/native/meta-kms.c:620
    #6 meta_kms_run_impl_task_sync at ../src/backends/native/meta-kms.c:435
    #7 meta_kms_update_states_sync at ../src/backends/native/meta-kms.c:641
    #8 handle_hotplug_event at ../src/backends/native/meta-kms.c:651
    #9 on_udev_hotplug at ../src/backends/native/meta-kms.c:668

Related: https://bugzilla.redhat.com/show_bug.cgi?id=2131269
Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2709>
This commit is contained in:
Jonas Ådahl 2022-11-23 23:08:33 +01:00 committed by Marge Bot
parent 66dcef7ac1
commit 8bc375005c
3 changed files with 17 additions and 22 deletions

View File

@ -103,7 +103,8 @@ uint64_t meta_kms_connector_get_prop_drm_value (MetaKmsConnector *connector,
uint64_t value);
MetaKmsResourceChanges meta_kms_connector_update_state (MetaKmsConnector *connector,
drmModeRes *drm_resources);
drmModeRes *drm_resources,
drmModeConnector *drm_connector);
void meta_kms_connector_disable (MetaKmsConnector *connector);

View File

@ -754,20 +754,16 @@ out:
MetaKmsResourceChanges
meta_kms_connector_update_state (MetaKmsConnector *connector,
drmModeRes *drm_resources)
drmModeRes *drm_resources,
drmModeConnector *drm_connector)
{
MetaKmsImplDevice *impl_device;
drmModeConnector *drm_connector;
MetaKmsResourceChanges changes;
impl_device = meta_kms_device_get_impl_device (connector->device);
drm_connector = drmModeGetConnector (meta_kms_impl_device_get_fd (impl_device),
connector->id);
changes = meta_kms_connector_read_state (connector, impl_device,
drm_connector,
drm_resources);
g_clear_pointer (&drm_connector, drmModeFreeConnector);
return changes;
}

View File

@ -397,12 +397,14 @@ find_existing_connector (MetaKmsImplDevice *impl_device,
static MetaKmsResourceChanges
update_connectors (MetaKmsImplDevice *impl_device,
drmModeRes *drm_resources)
drmModeRes *drm_resources,
uint32_t updated_connector_id)
{
MetaKmsImplDevicePrivate *priv =
meta_kms_impl_device_get_instance_private (impl_device);
g_autolist (MetaKmsConnector) connectors = NULL;
gboolean added_connector = FALSE;
MetaKmsResourceChanges changes = META_KMS_RESOURCE_CHANGE_NONE;
unsigned int i;
int fd;
@ -421,6 +423,13 @@ update_connectors (MetaKmsImplDevice *impl_device,
if (connector)
{
connector = g_object_ref (connector);
if (updated_connector_id == 0 ||
meta_kms_connector_get_id (connector) == updated_connector_id)
{
changes |= meta_kms_connector_update_state (connector,
drm_resources,
drm_connector);
}
}
else
{
@ -436,7 +445,7 @@ update_connectors (MetaKmsImplDevice *impl_device,
if (!added_connector &&
g_list_length (connectors) == g_list_length (priv->connectors))
return META_KMS_RESOURCE_CHANGE_NONE;
return changes;
g_list_free_full (priv->connectors, g_object_unref);
priv->connectors = g_list_reverse (g_steal_pointer (&connectors));
@ -892,7 +901,7 @@ meta_kms_impl_device_update_states (MetaKmsImplDevice *impl_device,
goto err;
}
changes = update_connectors (impl_device, drm_resources);
changes = update_connectors (impl_device, drm_resources, connector_id);
for (l = priv->crtcs; l; l = l->next)
{
@ -905,17 +914,6 @@ meta_kms_impl_device_update_states (MetaKmsImplDevice *impl_device,
changes |= meta_kms_crtc_update_state (crtc);
}
for (l = priv->connectors; l; l = l->next)
{
MetaKmsConnector *connector = META_KMS_CONNECTOR (l->data);
if (connector_id > 0 &&
meta_kms_connector_get_id (connector) != connector_id)
continue;
changes |= meta_kms_connector_update_state (connector, drm_resources);
}
drmModeFreeResources (drm_resources);
return changes;
@ -1196,7 +1194,7 @@ meta_kms_impl_device_init_mode_setting (MetaKmsImplDevice *impl_device,
init_fallback_modes (impl_device);
update_connectors (impl_device, drm_resources);
update_connectors (impl_device, drm_resources, 0);
drmModeFreeResources (drm_resources);