Messages in this thread | | | From | John Ogness <> | Subject | Re: [PATCH printk v1 07/13] printk: move buffer definitions into console_emit_next_record() caller | Date | Wed, 02 Mar 2022 17:31:58 +0106 |
| |
On 2022-02-16, Petr Mladek <pmladek@suse.com> wrote: >> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c >> index 822b7b6ad6d1..02bde45c1149 100644 >> --- a/kernel/printk/printk.c >> +++ b/kernel/printk/printk.c >> @@ -2597,13 +2611,13 @@ static bool console_emit_next_record(struct console *con, bool *handover) >> goto skip; >> } >> >> - if (con->flags & CON_EXTENDED) { >> - write_text = &ext_text[0]; >> - len = info_print_ext_header(ext_text, sizeof(ext_text), r.info); >> - len += msg_print_ext_body(ext_text + len, sizeof(ext_text) - len, >> + if (ext_text) { >> + write_text = ext_text; >> + len = info_print_ext_header(ext_text, CONSOLE_EXT_LOG_MAX, r.info); >> + len += msg_print_ext_body(ext_text + len, CONSOLE_EXT_LOG_MAX - len, >> &r.text_buf[0], r.info->text_len, &r.info->dev_info); >> } else { >> - write_text = &text[0]; >> + write_text = text; >> len = record_print_text(&r, console_msg_format & MSG_FORMAT_SYSLOG, printk_time); > > @text and @ext_text buffers are never used at the same time. It might > be enough to use a single text[CONSOLE_EXT_LOG_MAX] buffer. It would > even slightly simplify the code.
No, they _are_ used at the same time.
r.text_buf is @text. msg_print_ext_body() takes @ext_text and &r.text_buf[0]. Unfortunately msg_print_ext_body() does not work "in place" like record_print_text() does.
>> @@ -2650,6 +2664,9 @@ static bool console_emit_next_record(struct console *con, bool *handover) >> */ >> static bool console_flush_all(bool do_cond_resched, u64 *next_seq, bool *handover) >> { >> + static char dropped_text[DROPPED_TEXT_MAX]; >> + static char ext_text[CONSOLE_EXT_LOG_MAX]; >> + static char text[CONSOLE_LOG_MAX]; > > These buffers are for printing from console_unlock(). The same buffers > will need to be allocated for each console in the kthreads. > > It might make sense to allocate these buffers in register_console() > and store the pointers in struct console. > > Well, we might need extra buffers for atomic console drivers and > diffent contexts that would be used during panic. But maybe > they can be allocated in register_console() as well.
register_console() happens quite early. But my plan for v2 is to make them global static variables and allocate them on the first register_console().
>> bool any_usable = false; >> struct console *con; >> bool any_progress; >> @@ -2667,7 +2684,16 @@ static bool console_flush_all(bool do_cond_resched, u64 *next_seq, bool *handove >> continue; >> any_usable = true; >> >> - progress = console_emit_next_record(con, handover); >> + if (con->flags & CON_EXTENDED) { >> + /* Extended consoles do not print "dropped messages". */ >> + progress = console_emit_next_record(con, &text[0], > > IMHO, &text[0] buffer is not used for extended consoles.
Yes. msg_print_ext_body() needs it.
>> + &ext_text[0], NULL, >> + handover); >> + } else { >> + progress = console_emit_next_record(con, &text[0], >> + NULL, &dropped_text[0], >> + handover); >> + } >> if (*handover) >> return true; > > I do not resist on allocating the buffers in register_console(). I am > not sure if it would really makes things easier.
I'll give it a try for v2.
John
| |