linux-kernel - Re: [PATCH v5 5/6] drm/log: Implement suspend/resume

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: <0d478a44-ae01-46a8-b4e7-a28c480e23be@redhat.com>
Date: Mon, 4 Nov 2024 15:36:14 +0100
From: Jocelyn Falempe <jfalempe@...hat.com>
To: Petr Mladek <pmladek@...e.com>, John Ogness <john.ogness@...utronix.de>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
 Javier Martinez Canillas <javierm@...hat.com>,
 "Guilherme G . Piccoli" <gpiccoli@...lia.com>,
 bluescreen_avenger@...izon.net, Caleb Connolly <caleb.connolly@...aro.org>,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 5/6] drm/log: Implement suspend/resume

On 04/11/2024 15:15, Petr Mladek wrote:
> On Mon 2024-11-04 11:52:33, John Ogness wrote:
>> On 2024-11-04, Jocelyn Falempe <jfalempe@...hat.com> wrote:
>>> I looked at what serial drivers are doing, because they can also have
>>> their clock gated in suspend.
>>>
>>> Would calling console_stop() in the suspend and console_start() in
>>> resume work ?
>>
>> Yes. That is what it is for.
> 
> It seems that you are right. I have never really investigated the purpose
> of this API /o\
> 
Thanks, I will send a v6 with that change.

> One problem with this API is that it does not check whether the
> console is registered. I wonder whether it might cause problems.

At least for drm_log, register_console() will always be called before.
> 
> For example, we should not set the CON_ENABLE flag when the console is not
> registered. Doing so would cause register_console() to always enable
> the console, even when it is not preferred.
> 
> Additionally, nbcon_kthread_wake() uses con->rcuwait, which is initialized
> by nbcon_alloc() called from register_console(). Fortunately, nbcon_alloc()
> is always called even if the console is not enabled in the end, but this
> might change in the future and cause subtle errors.
> 
> [ After even more thinking ]
> 
> I wonder whether console_start()/console_stop() should really
> manipulate CON_ENABLE flag. It might be historical solution when
> @console_suspended was a global variable.
> 
> But it has changed with the commit 9e70a5e109a4a2336 ("printk: Add
> per-console suspended state").
> 
> It might make more sense when console_start()/console_stop()
> manipulates CON_SUSPENDED flag. Then it would make sense
> to rename them suspend_this_console()/resume_this_console().
> 
> What do you think?

Maybe when registering the console, having a flag to say "I want this 
console to be suspended with the console subsystem" or "I want to handle 
suspend/resume on my own, and call the relevant functions" would be better ?

That would avoid having the same console being suspended/resumed twice, 
and making clear what to expect.

Of course "no_console_suspend" won't really work for drivers handling 
suspend/resume themselves.

-- 

Jocelyn
> 
> Best Regards,
> Petr
>