From: "Noralf Trønnes" <noralf@tronnes.org>
To: Oliver Neukum <oneukum@suse.de>, dri-devel@lists.freedesktop.org
Cc: hudson@trmm.net, markus@raatikainen.cc, sam@ravnborg.org,
linux-usb@vger.kernel.org, th020394@gmail.com, lkundrak@v3.sk,
pontus.fuchs@gmail.com, Daniel Vetter <daniel.vetter@ffwll.ch>,
peter@stuge.se
Subject: Re: [PATCH v5 3/3] drm: Add Generic USB Display driver
Date: Tue, 16 Feb 2021 17:43:33 +0100 [thread overview]
Message-ID: <d89c375e-3dea-89a3-2315-65ee91fd17f0@tronnes.org> (raw)
In-Reply-To: <316502da-218e-3837-d356-79a7b5dce7ae@tronnes.org>
Den 16.02.2021 17.40, skrev Noralf Trønnes:
>
>
> Den 16.02.2021 14.36, skrev Oliver Neukum:
>> Am Freitag, den 12.02.2021, 18:46 +0100 schrieb Noralf Trønnes:
>>> +static void gud_connector_early_unregister(struct drm_connector *connector)
>>> +{
>>> + struct gud_connector *gconn = to_gud_connector(connector);
>>> +
>>> + backlight_device_unregister(gconn->backlight);
>>> + cancel_work_sync(&gconn->backlight_work);
>>> +}
>>
>> Hi,
>>
>> this looks like you are creating a race condition where the queued work
>> may operate on an already unregistered backlight.
>>
>
> backlight_device_unregister() sets bd->ops = NULL. This means that
> userspace can't update brightness anymore, and thus won't call into this
> driver. After that it should be safe to flush/cancel the workqueue.
>
> Unless I'm missing something here.
>
And it's ok for the worker to operate on an unregistered backlight, the
memory is still available. It's freed when the connector goes away.
Noralf.
WARNING: multiple messages have this Message-ID (diff)
From: "Noralf Trønnes" <noralf@tronnes.org>
To: Oliver Neukum <oneukum@suse.de>, dri-devel@lists.freedesktop.org
Cc: hudson@trmm.net, markus@raatikainen.cc,
Daniel Vetter <daniel.vetter@ffwll.ch>,
linux-usb@vger.kernel.org, th020394@gmail.com, lkundrak@v3.sk,
pontus.fuchs@gmail.com, peter@stuge.se, sam@ravnborg.org
Subject: Re: [PATCH v5 3/3] drm: Add Generic USB Display driver
Date: Tue, 16 Feb 2021 17:43:33 +0100 [thread overview]
Message-ID: <d89c375e-3dea-89a3-2315-65ee91fd17f0@tronnes.org> (raw)
In-Reply-To: <316502da-218e-3837-d356-79a7b5dce7ae@tronnes.org>
Den 16.02.2021 17.40, skrev Noralf Trønnes:
>
>
> Den 16.02.2021 14.36, skrev Oliver Neukum:
>> Am Freitag, den 12.02.2021, 18:46 +0100 schrieb Noralf Trønnes:
>>> +static void gud_connector_early_unregister(struct drm_connector *connector)
>>> +{
>>> + struct gud_connector *gconn = to_gud_connector(connector);
>>> +
>>> + backlight_device_unregister(gconn->backlight);
>>> + cancel_work_sync(&gconn->backlight_work);
>>> +}
>>
>> Hi,
>>
>> this looks like you are creating a race condition where the queued work
>> may operate on an already unregistered backlight.
>>
>
> backlight_device_unregister() sets bd->ops = NULL. This means that
> userspace can't update brightness anymore, and thus won't call into this
> driver. After that it should be safe to flush/cancel the workqueue.
>
> Unless I'm missing something here.
>
And it's ok for the worker to operate on an unregistered backlight, the
memory is still available. It's freed when the connector goes away.
Noralf.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2021-02-16 16:44 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-12 17:46 [PATCH v5 0/3] GUD USB Display driver Noralf Trønnes
2021-02-12 17:46 ` Noralf Trønnes
2021-02-12 17:46 ` [PATCH v5 1/3] drm/uapi: Add USB connector type Noralf Trønnes
2021-02-12 17:46 ` Noralf Trønnes
2021-02-12 17:46 ` [PATCH v5 2/3] drm/probe-helper: Check epoch counter in output_poll_execute() Noralf Trønnes
2021-02-12 17:46 ` Noralf Trønnes
2021-02-12 17:46 ` [PATCH v5 3/3] drm: Add Generic USB Display driver Noralf Trønnes
2021-02-12 17:46 ` Noralf Trønnes
2021-02-15 11:58 ` Peter Stuge
2021-02-15 11:58 ` Peter Stuge
2021-02-16 13:19 ` Noralf Trønnes
2021-02-16 13:19 ` Noralf Trønnes
2021-02-16 13:36 ` Oliver Neukum
2021-02-16 13:36 ` Oliver Neukum
2021-02-16 16:40 ` Noralf Trønnes
2021-02-16 16:40 ` Noralf Trønnes
2021-02-16 16:43 ` Noralf Trønnes [this message]
2021-02-16 16:43 ` Noralf Trønnes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d89c375e-3dea-89a3-2315-65ee91fd17f0@tronnes.org \
--to=noralf@tronnes.org \
--cc=daniel.vetter@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=hudson@trmm.net \
--cc=linux-usb@vger.kernel.org \
--cc=lkundrak@v3.sk \
--cc=markus@raatikainen.cc \
--cc=oneukum@suse.de \
--cc=peter@stuge.se \
--cc=pontus.fuchs@gmail.com \
--cc=sam@ravnborg.org \
--cc=th020394@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.