Den 11.07.2018 20.53, skrev Daniel Vetter:
On Wed, Jul 11, 2018 at 8:21 PM, Noralf Trønnes <nor...@tronnes.org> wrote:
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.
I think we can figure out what to do with kmscon once it's being
(re)submitted for upstream. No point having headaches over an issue
that doesn't yet exist.
My first reaction would be to handle it exactly as the fbdev stuff,
and not allow kmscon to be loaded later on.
-Daniel
Ok, I've fixed the commit message and pushed. Thanks for reviewing.
Icenowy, thanks for testing.
Noralf.
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
