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@xxxxxxx>
>
> ---
> 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.
Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
