lkml.org 
[lkml]   [2015]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH] video/logo: introduce new system state for checking if logos are freed
Hello Tomi,

Am 25.05.2015 07:57, schrieb Tomi Valkeinen:
>
>
> On 06/05/15 10:09, Heiko Schocher wrote:
>> commit 92b004d1aa9f ("video/logo: prevent use of logos after they have been freed")
>>
>> added a late_initcall function to mark the logos as freed. In reality
>> the logos are freed later, and fbdev probe may be ran between this
>> late_initcall and the freeing of the logos. In that case the logos
>> would not be drawn. To prevent this introduced a new system_state
>> SYSTEM_FREEING_MEM and set this state before freeing memory. This
>> state could be checked now in fb_find_logo(). This system state
>> is maybe useful on other places too.
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>>
>> ---
>> Found this issue on an imx6 based board with a display which needs
>> a spi initialization. With 3.18.2 I see a perfect logo, but with
>> current ml, bootlogo is missing, because drm gets probed before
>> spi display, which leads in drm probing is deferred until the
>> spi display is probed. After that drm is probed again ... but
>> this is too late for showing the bootlogo.
>> With this patch, bootlogo is drawn again. I am not sure, if it
>> is so easy to add a new system state ... but we should have a
>> possibility to detect if initdata is freed or not. this is maybe
>> also for other modules interesting. Maybe we add a
>> kernel_initdata_freed()
>> function instead of a new system state?
>>
>> drivers/video/logo/logo.c | 15 ++++-----------
>> include/linux/kernel.h | 1 +
>> init/main.c | 1 +
>> 3 files changed, 6 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/video/logo/logo.c b/drivers/video/logo/logo.c
>> index 10fbfd8..d798a9f 100644
>> --- a/drivers/video/logo/logo.c
>> +++ b/drivers/video/logo/logo.c
>> @@ -26,16 +26,6 @@ MODULE_PARM_DESC(nologo, "Disables startup logo");
>> * Use late_init to mark the logos as freed to prevent any further use.
>> */
>>
>> -static bool logos_freed;
>> -
>> -static int __init fb_logo_late_init(void)
>> -{
>> - logos_freed = true;
>> - return 0;
>> -}
>> -
>> -late_initcall(fb_logo_late_init);
>> -
>> /* logo's are marked __initdata. Use __init_refok to tell
>> * modpost that it is intended that this function uses data
>> * marked __initdata.
>> @@ -44,7 +34,10 @@ const struct linux_logo * __init_refok fb_find_logo(int depth)
>> {
>> const struct linux_logo *logo = NULL;
>>
>> - if (nologo || logos_freed)
>> + if (system_state >= SYSTEM_FREEING_MEM)
>> + return NULL;
>> +
>> + if (nologo)
>> return NULL;
>>
>> if (depth >= 1) {
>> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
>> index 3a5b48e..e5875bf 100644
>> --- a/include/linux/kernel.h
>> +++ b/include/linux/kernel.h
>> @@ -462,6 +462,7 @@ extern bool early_boot_irqs_disabled;
>> /* Values used for system_state */
>> extern enum system_states {
>> SYSTEM_BOOTING,
>> + SYSTEM_FREEING_MEM,
>> SYSTEM_RUNNING,
>> SYSTEM_HALT,
>> SYSTEM_POWER_OFF,
>> diff --git a/init/main.c b/init/main.c
>> index 2115055..4965ed0 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -931,6 +931,7 @@ static int __ref kernel_init(void *unused)
>> kernel_init_freeable();
>> /* need to finish all async __init code before freeing the memory */
>> async_synchronize_full();
>> + system_state = SYSTEM_FREEING_MEM;
>> free_initmem();
>> mark_rodata_ro();
>> system_state = SYSTEM_RUNNING;
>
> Without locking, the initmem may be freed while fb_find_logo() is running.

Yes, you are right, that must be added ... but has such a change a
chance to go in mainline?

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?

bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


\
 
 \ /
  Last update: 2015-05-26 07:21    [W:0.056 / U:1.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site