linux-kernel - Re: [PATCH 8/8] drm/udl: Use drm_fb_helper deferred

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKMK7uGR_rC9K2TxRVJ_3_EMNZHD_WDbShXDsTrMX6kA7euj3Q@mail.gmail.com>
Date:	Wed, 20 Apr 2016 23:22:52 +0200
From:	Daniel Vetter <daniel@...ll.ch>
To:	Noralf Trønnes <noralf@...nnes.org>
Cc:	dri-devel <dri-devel@...ts.freedesktop.org>,
	Linux Fbdev development list <linux-fbdev@...r.kernel.org>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Tomi Valkeinen <tomi.valkeinen@...com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 8/8] drm/udl: Use drm_fb_helper deferred_io support

On Wed, Apr 20, 2016 at 9:20 PM, Noralf Trønnes <noralf@...nnes.org> wrote:
>>> @@ -330,20 +203,20 @@ static int udl_fb_open(struct fb_info *info, int
>>> user)
>>>         ufbdev->fb_count++;
>>>   -     if (fb_defio && (info->fbdefio == NULL)) {
>>> -               /* enable defio at last moment if not disabled by client
>>> */
>>> +       if (!info->fbdefio) {
>>> +               /* enable defio at last moment */
>>>                 struct fb_deferred_io *fbdefio;
>>>                 fbdefio = kmalloc(sizeof(struct fb_deferred_io),
>>> GFP_KERNEL);
>>> -
>>>                 if (fbdefio) {
>>>                         fbdefio->delay = DL_DEFIO_WRITE_DELAY;
>>> -                       fbdefio->deferred_io = udlfb_dpy_deferred_io;
>>> +                       fbdefio->deferred_io = drm_fb_helper_deferred_io;
>>
>> Why all these changes here? I figured just exchanging the deferred_io
>> pointer should be all that's really needed in the setup code?
>
>
> Because we always need to initialize deferred_io since we use it's worker
> to handle fb_{fillrect,copyarea,imageblit} damage in drm_fb_helper.
>
> The previous code didn't use deferred_io to handle these, it just handled
> the damage directly unless it was running in atomic context, in which case
> it just recorded the damage and returned, leaving it to the next call to
> push the changes.

That kind of explanation needs to be added to the commit message. I
completely missed that udl doesn't have an async work item for defio
from atomic.

> And in the following code I fixed a null pointer problem as well, maybe I
> shouldn't have packed it in here. If fbdefio allocation fails == NULL,
> fb_deferred_io_init() will trigger a BUG().

Yeah, better to split that out into a separate bugfix I think.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch