[PATCH printk v4 0/8] printk: cleanup buffer handling

John Ogness posted 8 patches 2 years, 8 months ago
There is a newer version of this series
include/linux/console.h  | 100 +++++++++----
include/linux/printk.h   |   2 -
kernel/printk/internal.h |  45 ++++++
kernel/printk/printk.c   | 293 +++++++++++++++++++++++----------------
4 files changed, 288 insertions(+), 152 deletions(-)
[PATCH printk v4 0/8] printk: cleanup buffer handling
Posted by John Ogness 2 years, 8 months ago
Hi,

This is v4 of a series to cleanup console buffer handling and
prepare for code sharing with the upcoming threaded/atomic
consoles. v3 is here [0].

The main purpose of the series is to introduce two new lockless
functions to handle reading and formatting of printk messages. These
functions can then be used from any context, which is important for
the upcoming threaded/atomic consoles. The series also helps to
cleanup some of the internal printk interfaces and cleanly separate
formatting code from outputting code.

Some changes in this version were not part of the feedback. However,
from the discussions and examples I decided that some of the names
I had previously chosen were not appropriate. Particularly, structs,
variables, and functions should only use the "console_" prefix if it
is console-specific. Things that are used for general printk
processing should use a "printk_" prefix. This makes the diff to v3
rather large, even though the "real code" has only minor changes.

@Petr: Like with v3, this version uses a wrapper struct for the
message metadata to avoid clobbering. Making the message formatting
code robust enough to handle metadata clobbering was too complex
(particularly with the dropped tracking).

Changes since v3:

- Provide a detailed explanation in the commit message about why
  message metadata is put into a wrapper struct instead of the
  buffer struct.

- Reorder stack variable definitions so that static variables are at
  the top and separated from non-static variables. IMHO it is
  important to clearly see which of the variables are static.

- Drop a previous coding-style change from a line not related to
  this series.

- In console_prepend_dropped() make sure the buffer is large enough
  for the dropped message and at least PREFIX_MAX of the current
  message.

- Define the PREFIX_MAX macro for !CONFIG_PRINTK in internal.h
  because console_prepend_dropped() now needs it.

- Rename various functions, structs, fields, and macros:

    console_get_next_message()  ->  printk_get_next_message()

    struct console_buffers      ->  struct printk_buffers

    struct console_message      ->  struct printk_message

    console_message.cbufs       ->  printk_message.pbufs

    console_message.outbuf_seq  ->  printk_message.seq

    LOG_LINE_MAX                ->  PRINTKRB_RECORD_MAX

    PREFIX_MAX                  ->  PRINTK_PREFIX_MAX

    CONSOLE_LOG_MAX and
    CONSOLE_EXT_LOG_MAX         ->  PRINTK_MESSAGE_MAX

- Adjust the values of string limit macros. This is explained in
  detail in the commit message.

- Replace @buf and @text_buf in struct devkmsg_user with struct
  printk_buffers.

- Replace message formatting code in devkmsg_read() with
  printk_get_next_message().

- Define all printk_message structs on the stack.

John Ogness

[0] https://lore.kernel.org/lkml/20221221202704.857925-1-john.ogness@linutronix.de

John Ogness (6):
  printk: move size limit macros into internal.h
  printk: introduce struct printk_buffers
  printk: introduce printk_get_next_message() and printk_message
  printk: introduce console_prepend_dropped() for dropped messages
  printk: use printk_buffers for devkmsg
  printk: adjust string limit macros

Thomas Gleixner (2):
  console: Use BIT() macros for @flags values
  console: Document struct console

 include/linux/console.h  | 100 +++++++++----
 include/linux/printk.h   |   2 -
 kernel/printk/internal.h |  45 ++++++
 kernel/printk/printk.c   | 293 +++++++++++++++++++++++----------------
 4 files changed, 288 insertions(+), 152 deletions(-)


base-commit: 6b2b0d839acaa84f05a77184370f793752e786e9
-- 
2.30.2
Re: [PATCH printk v4 0/8] printk: cleanup buffer handling
Posted by Petr Mladek 2 years, 8 months ago
On Thu 2023-01-05 11:43:27, John Ogness wrote:
> Hi,
> 
> This is v4 of a series to cleanup console buffer handling and
> prepare for code sharing with the upcoming threaded/atomic
> consoles. v3 is here [0].
> 
> The main purpose of the series is to introduce two new lockless
> functions to handle reading and formatting of printk messages. These
> functions can then be used from any context, which is important for
> the upcoming threaded/atomic consoles. The series also helps to
> cleanup some of the internal printk interfaces and cleanly separate
> formatting code from outputting code.
> 
> John Ogness (6):
>   printk: move size limit macros into internal.h
>   printk: introduce struct printk_buffers
>   printk: introduce printk_get_next_message() and printk_message
>   printk: introduce console_prepend_dropped() for dropped messages

This patch would need a followup fix to prevent the compiler warning.

>   printk: use printk_buffers for devkmsg

This one would need respin to fix the problem with suppressed messages.

>   printk: adjust string limit macros
> 
> Thomas Gleixner (2):
>   console: Use BIT() macros for @flags values
>   console: Document struct console

The rest looks ready for linux-next.

I see three ways how to move forward:

1. Respin the entire patchset.
2. Respin only 7th patch + send the follow up fix seperately
3. Push first 6 patches and handle the rest separately.

What would be more convenient for you, please?
Honestly, I do not have any real preference.

Best Regards,
Petr
Re: [PATCH printk v4 0/8] printk: cleanup buffer handling
Posted by John Ogness 2 years, 8 months ago
On 2023-01-06, Petr Mladek <pmladek@suse.com> wrote:
> On Thu 2023-01-05 11:43:27, John Ogness wrote:
> I see three ways how to move forward:
>
> 1. Respin the entire patchset.
> 2. Respin only 7th patch + send the follow up fix seperately
> 3. Push first 6 patches and handle the rest separately.

I will respin the entire patchset. That is easist for me and makes
things quite straight forward for you.

I still am looking into and will to respond to your comments on patch
7/8 before I do the respin.

John