Den 11.07.2018 20.00, skrev Daniel Vetter:
On Wed, Jul 11, 2018 at 05:56:32PM +0200, Noralf Trønnes wrote:
Commit 894a677f4b3e ("drm/cma-helper: Use the generic fbdev emulation")
broke almost all drivers that use the CMA helper.

The reason is that drm_client_new() requires that the DRM device has
been registered, but the drivers register fbdev before registering DRM.

Remove the requirement that DRM should be registered when creating a
new client.

This creates a theoretical race condition where drm_client_new() races
with drm_dev_unregister() resulting in the .unregister hook not being
called.

Case:
User loads module which calls drm_client_new().
Device is unplugged and drm_dev_unregister() is called.

If drm_dev_unregister() gets the clientlist_mutex first, the new client
will never get it's unregister hook called.
As long as we assume that drm_client_new() is serialized against
drm_dev_unregister (which it has to, or all the current fbdev code would
be busted) I'm not seeing a race here.

We can't allow drm_client_new concurrently with drm_dev_register, but that
feature got canned. So looks all good to me, and no races anywhere to be
seen. Assuming you agree and update the commit message:

There's no problem with an fbdev or bootsplash client, but the problem is
with say a kmscon client that is loaded as a module. kmscon can be loaded
any time when there is a DRM device present, but DRM can be unregistered
while registering this new client.

At least this is how I understand this.

Noralf.


Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>

Fixes: c76f0f7cb546 ("drm: Begin an API for in-kernel clients")
Cc: Maxime Ripard <maxime.rip...@bootlin.com>
Cc: Icenowy Zheng <icen...@aosc.io>
Cc: Chen-Yu Tsai <w...@csie.org>
Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
Signed-off-by: Noralf Trønnes <nor...@tronnes.org>
---
  drivers/gpu/drm/drm_client.c | 11 +----------
  1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 4039a4d103a8..9b142f58d489 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -78,7 +78,6 @@ EXPORT_SYMBOL(drm_client_close);
  int drm_client_new(struct drm_device *dev, struct drm_client_dev *client,
                   const char *name, const struct drm_client_funcs *funcs)
  {
-       bool registered;
        int ret;
if (!drm_core_check_feature(dev, DRIVER_MODESET) ||
@@ -97,21 +96,13 @@ int drm_client_new(struct drm_device *dev, struct 
drm_client_dev *client,
                goto err_put_module;
mutex_lock(&dev->clientlist_mutex);
-       registered = dev->registered;
-       if (registered)
-               list_add(&client->list, &dev->clientlist);
+       list_add(&client->list, &dev->clientlist);
        mutex_unlock(&dev->clientlist_mutex);
-       if (!registered) {
-               ret = -ENODEV;
-               goto err_close;
-       }
drm_dev_get(dev); return 0; -err_close:
-       drm_client_close(client);
  err_put_module:
        if (funcs)
                module_put(funcs->owner);
--
2.15.1


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to