Re: [RFC PATCH] video/logo: introduce new system state for checking if logos are freed

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2832 bytes --]



On 26/05/15 10:08, Geert Uytterhoeven wrote:
> On Tue, May 26, 2015 at 8:54 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On 26/05/15 06:56, Heiko Schocher wrote:
>>>> Without locking, the initmem may be freed while fb_find_logo() is
>>>> running.
> 
> Or afterwards. Drivers may keep the pointer around indefinitely.
> 
>>> Yes, you are right, that must be added ... but has such a change a
>>> chance to go in mainline?
>>
>> I don't know. To be honest, this whole thing feels a bit like hackery. I
>> think initdata should only be accessed from initcalls, never asynchronously.
>>
>>> BTW: Could this not be currently a problem on multicore systems?
>>> If lets say core 2 just draws the logo, another core 1 calls
>>> fb_logo_late_init() and later core 1 free_initmem(), while the core 2
>>> still draws it?
>>
>> Yes, I think so...
> 
> I don't think that can happen. All initcalls should complete before initmem
> is freed.

Ah, true, the question was only about the initcalls. I was answering to
what actually can happen with the logo code as a whole.

The whole problem started when I fixed an issue where the logos were
accessed from a workqueue. I don't remember the details, but I think drm
always (?) sets up some console stuff via workthread. In that case we
could have the workthread accessing the logos, while initmem is being freed.

>> So, maybe it would be better to not even try to go forward with the
>> current approach. Two approaches come to my mind:
>>
>> 1) Keep the logos in the memory, and don't even try to free them. I
>> don't know many bytes they are in total, though.
> 
> m68k/allmodconfig:
> 
> $ size drivers/video/logo/logo*o
>    text   data    bss    dec    hex filename
>      24   6961      0   6985   1b49 drivers/video/logo/logo_linux_clut224.o
>      24    800      0    824    338 drivers/video/logo/logo_linux_mono.o
>      24   3200      0   3224    c98 drivers/video/logo/logo_linux_vga16.o
>      24   6955      0   6979   1b43 drivers/video/logo/logo_mac_clut224.o
>     161      4      2    167     a7 drivers/video/logo/logo.o
> 
> Not that bad... Custom logos may be larger, though.

I wonder how much a simple RLE would cut down the sizes...

>> 2) Make a copy of the logos to a kmalloced area at some early boot
>> stage. Then manually free the logos at some point (after the first
>> access to the logos? after a certain time (urgh...)?).
> 
> 3) Draw the logos from an initcall on all frame buffers that exist at that
>    point in time. Yes, this will destroy (part of) the content that's
>    currently shown.

Isn't that almost the same as now? The problem is that the fb probes are
deferred to a very late stage, so we would not have the fbs when the
suggested initcall would be called.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]