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]
Date:   Sat, 25 May 2019 19:19:28 +0200
From:   Sam Ravnborg <sam@...nborg.org>
To:     Daniel Vetter <daniel.vetter@...ll.ch>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Intel Graphics Development <intel-gfx@...ts.freedesktop.org>,
        DRI Development <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH 00/33] fbcon notifier begone!

Hi Daniel.

Good work, nice cleanup all over.

A few comments to a few patches - not something that warrant a
new series to be posted as long as it is fixed before the patches are
applied.


> btw for future plans: I think this is tricky enough (it's old code and all
> that) that we should let this soak for 2-3 kernel releases. I think the
> following would be nice subsequent cleanup/fixes:
> 
> - push the console lock completely from fbmem.c to fbcon.c. I think we're
>   mostly there with prep, but needs to pondering of corner cases.
I wonder - should this code consistently use __acquire() etc so we could
get a little static analysis help for the locking?

I have not played with this for several years and I do not know the
maturity of this today.

> - move fbcon.c from using indices for tracking fb_info (and accessing
>   registered_fbs without proper locking all the time) to real fb_info
>   pointers with the right amount of refcounting. Mostly motivated by the
>   fun I had trying to simplify fbcon_exit().
> 
> - make sure that fbcon call lock/unlock_fb when it calls fbmem.c
>   functions, and sprinkle assert_lockdep_held around in fbmem.c. This
>   needs the console_lock cleanups first.
> 
> But I think that's material for maybe next year or so.
Or maybe after next kernel release.
Could we put this nice plan into todo.rst or similar so we do not have
to hunt it down by asking google?

For the whole series you can add my:
Reviewed-by: Sam Ravnborg <sam@...nborg.org>

Some parts are reviewed as "this looks entirely correct", other parts
I would claim that I actually understood.
And after having spend some hours on this a r-b seems in order.

	Sam

Powered by blists - more mailing lists