Re: [PATCH 1/2] drm/vkms: Add drm_dev_enter/exit guards for hot-removable paths

[Oarora Etimis]

Whoops, May I ask about one path.

> 2. Conversely, if struct drm_device is still kept alive by an open
> /dev/dri/card<N> fd when crc/data is closed after drm_dev_unplug() sets
> dev->unplugged = true, drm_dev_enter(crtc->dev, &idx) in
> vkms_set_crc_source(crtc, NULL) will return false and exit early with
> -ENODEV. Doesn't that skip vkms_set_composer(out, false), leaving
> out->composer_enabled set to true and leaking the vblank reference from
> drm_crtc_vblank_put(&out->crtc)?

I followed it and think it really holds. And this issue was introduced by this patch.

drivers/gpu/drm/vkms/vkms_composer.c:vkms_set_crc_source() {
	if (!drm_dev_enter(crtc->dev, &idx))
		return -ENODEV;

	ret = vkms_crc_parse_source(src_name, &enabled);

	vkms_set_composer(out, enabled);
}

I understand the guard is meant to reject new operations after unplug. However, since closing the CRC file is a cleanup path, skipping vkms_set_composer(out, false) seems like it would leak the vblank reference.

Thank you,
Oarora Etimis