Messages in this thread | | | Date | Tue, 15 Feb 2022 10:29:19 +0100 | From | Petr Mladek <> | Subject | Re: [PATCH printk v1 01/13] printk: rename cpulock functions |
| |
On Mon 2022-02-14 15:49:08, Sergey Senozhatsky wrote: > On (22/02/11 13:44), Petr Mladek wrote: > > On Mon 2022-02-07 20:49:11, John Ogness wrote: > > > Since the printk cpulock is CPU-reentrant and since it is used > > > in all contexts, its usage must be carefully considered and > > > most likely will require programming locklessly. To avoid > > > mistaking the printk cpulock as a typical lock, rename it to > > > cpu_sync. The main functions then become: > > > > > > printk_cpu_sync_get_irqsave(flags); > > > printk_cpu_sync_put_irqrestore(flags); > > > > It is possible that I will understand the motivation later when > > reading the entire patchset. But my initial reaction is confusion ;-) > > > > From mo POV, it is a lock. It tries to get exclusive access and > > has to wait until the current owner releases it. > > printk has been using enter/exit naming for a while now (starting with > nmi enter/exit, then printk_safe enter/exit and soon direct enter/exit); > so may be we can follow suit here and use printk_cpu_sync_enter() and > printk_cpu_sync_exit()?
Interesting idea.
Honestly, I do not like it much. The existing enter/exit API is not blocking but "cpu_sync" API is.
This patch is about how to make it more obvious that this API has to be used carefully. I see the following main risks when using this API:
+ it might cause deadlocks, especially in panic
+ it is supposed to be tail lock (no other locks allowed in this context)
+ it is re-entrant
+ it does not prevent parallel (nested) access on the same CPU
"get/put" are more acceptable for me. They create at lest some feeling that it tries to get something and it might take some time. Even thought many "get/put" APIs are not blocking.
BTW: The API disables IRQ. So the nested access is limited to two levels: normal/IRQ and nested NMI contexts.
Best Regards, Petr
| |