Re: [PATCH v4 3/4] Revert "printk: lock/unlock console only for new logbuf entries"

2020-07-08 Thread Sergey Senozhatsky
On (20/07/07 17:05), John Ogness wrote:
> This reverts commit 3ac37a93fa9217e576bebfd4ba3e80edaaeb2289.
> 
> This optimization will not apply once the transition to a lockless
> printk is complete. Rather than porting this optimization through
> the transition only to remove it anyway, just revert it now to
> simplify the transition.
> 
> Signed-off-by: John Ogness 

Acked-by: Sergey Senozhatsky 

-ss

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH v4 0/4] printk: replace ringbuffer

2020-07-08 Thread Petr Mladek
On Tue 2020-07-07 17:05:28, John Ogness wrote:
> Hello,
> 
> Here is a v4 for the first series to rework the printk
> subsystem. The v3 is here [0]. This first series
> only replaces the existing ringbuffer implementation. No locking
> is removed. The semantics/behavior of printk are kept the same
> except for a minor optimization that is reverted (patch 3).
> 
> Despite minor changes to the ringbuffer code since v3 (comments,
> function names, very minor refactoring), the ringbuffer logic
> itself has not changed. And, in particular, the memory barriers
> have been exactly preserved from v3. For this reason I deem it
> appropriate to keep Paul's reviewed by tag (patch 2).
> 
> RFC patches for various userspace tools to dump the kernel log
> are available: crash [1], makedumpfile [2], kexec-tools [3].
> 
> Finally, I would like to thank some people/organizations that
> helped with performing ringbuffer stress tests on big or rare
> hardware that I do not have access to:
> 
> - Prarit Bhargava of Red Hat (x86_64, ppc64le power8)
> - Michael Cree of Debian (alpha)
> - Jeff Scheel of OSU Open Source Lab (ppc64le power8 kvm)

OK, I think that we are ready to try this in linux-next.
I am going to push it there via printk/linux.git.

I have a good feeling about the patchset. The great thing is that
the access is still synchronized using logbuf_lock so that we do not
have to deal with races for the moment.

Of course, there are still many potential problems. The following comes
to my mind:

   + Bugs in the algorithm logic or implementation might prevent
 showing any messages on consoles or via syslog or /dev/kmsg.
 We did our best to avoid it.

   + Debugging tools accessing the buffer directly would need to
 understand the new structure. Fortunately John provided
 patches for the most prominent ones.

   + Small devices might complain about less effective use of memory.
 Part of descriptors and dictionaries ring buffers might stay
 unused. But it hopefully could get tuned.


This is basically just a start of the journey. I hope that it will be a
good one.

Best Regards,
Petr

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH v4 4/4] printk: use the lockless ringbuffer

2020-07-08 Thread Petr Mladek
On Tue 2020-07-07 17:05:32, John Ogness wrote:
> Replace the existing ringbuffer usage and implementation with
> lockless ringbuffer usage. Even though the new ringbuffer does not
> require locking, all existing locking is left in place. Therefore,
> this change is purely replacing the underlining ringbuffer.
> 
> Changes that exist due to the ringbuffer replacement:
> 
> - The VMCOREINFO has been updated for the new structures.
> 
> - Dictionary data is now stored in a separate data buffer from the
>   human-readable messages. The dictionary data buffer is set to the
>   same size as the message buffer. Therefore, the total required
>   memory for both dictionary and message data is
>   2 * (2 ^ CONFIG_LOG_BUF_SHIFT) for the initial static buffers and
>   2 * log_buf_len (the kernel parameter) for the dynamic buffers.
> 
> - Record meta-data is now stored in a separate array of descriptors.
>   This is an additional 72 * (2 ^ (CONFIG_LOG_BUF_SHIFT - 5)) bytes
>   for the static array and 72 * (log_buf_len >> 5) bytes for the
>   dynamic array.
> 
> Signed-off-by: John Ogness 

Reviewed-by: Petr Mladek 

Best Regards,
Petr

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH v4 3/4] Revert "printk: lock/unlock console only for new logbuf entries"

2020-07-08 Thread Petr Mladek
On Tue 2020-07-07 17:05:31, John Ogness wrote:
> This reverts commit 3ac37a93fa9217e576bebfd4ba3e80edaaeb2289.
> 
> This optimization will not apply once the transition to a lockless
> printk is complete. Rather than porting this optimization through
> the transition only to remove it anyway, just revert it now to
> simplify the transition.
> 
> Signed-off-by: John Ogness 

Reviewed-by: Petr Mladek 

Best Regards,
Petr

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH v4 4/4] printk: use the lockless ringbuffer

2020-07-08 Thread John Ogness
Fix prepared for for v5.

On 2020-07-08, kernel test robot  wrote:
>>> kernel/printk/printk.c:1146:10: warning: format specifies type 'unsigned 
>>> long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
>   new_descs_size);
>   ^~
>include/linux/printk.h:338:33: note: expanded from macro 'pr_err'
>printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
>   ~~~ ^~~
>1 warning generated.

Reported-by: kernel test robot 

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index de8d54be4115..0b1337f4188c 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1142,7 +1142,7 @@ void __init setup_log_buf(int early)
new_descs_size = new_descs_count * sizeof(struct prb_desc);
new_descs = memblock_alloc(new_descs_size, LOG_ALIGN);
if (unlikely(!new_descs)) {
-   pr_err("log_buf_len: %lu desc bytes not available\n",
+   pr_err("log_buf_len: %zu desc bytes not available\n",
   new_descs_size);
memblock_free(__pa(new_dict_buf), new_log_buf_len);
memblock_free(__pa(new_log_buf), new_log_buf_len);

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec