[printk,v2,6/7] printk: Use an output buffer descriptor struct for emit
Commit Message
From: Thomas Gleixner <tglx@linutronix.de>
To prepare for a new console infrastructure that is independent of
the console BKL, wrap the output mode into a descriptor struct so
the new infrastructure can share the emit code that dumps the
ringbuffer record into the output text buffers.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
include/linux/console.h | 15 ++++++
kernel/printk/printk.c | 107 ++++++++++++++++++++++++++++------------
2 files changed, 90 insertions(+), 32 deletions(-)
Comments
On Thu 2022-11-24 00:19:59, John Ogness wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> To prepare for a new console infrastructure that is independent of
> the console BKL, wrap the output mode into a descriptor struct so
> the new infrastructure can share the emit code that dumps the
> ringbuffer record into the output text buffers.
>
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2741,11 +2741,76 @@ static void __console_unlock(void)
> up_console_sem();
> }
>
> +/**
> + * console_get_next_message - Fill the output buffer with the next record
> + * @con: The console to print on
> + * @cmsg: Pointer to the output buffer descriptor
> + *
> + * Return: False if there is no pending record in the ringbuffer.
> + * True if there is a pending record in the ringbuffer.
> + *
> + * When the return value is true, then the caller must check
> + * @cmsg->outbuf. If not NULL it points to the first character to write
> + * to the device and @cmsg->outbuf_len contains the length of the message.
> + * If it is NULL then the record will be skipped.
> + */
> +static bool console_get_next_message(struct console *con, struct console_message *cmsg)
> +{
I wish, this change was done in two patches. 1st introducing and
using struct console_message. 2nd moving the code into separate
console_get_next_message().
I do not resist on it but it might help to see what exactly has changed.
> + struct console_buffers *cbufs = cmsg->cbufs;
> + static int panic_console_dropped;
> + struct printk_info info;
> + struct printk_record r;
> + size_t write_text_size;
> + char *write_text;
> + size_t len;
> +
> + cmsg->outbuf = NULL;
> + cmsg->outbuf_len = 0;
> +
> + prb_rec_init_rd(&r, &info, &cbufs->text[0], sizeof(cbufs->text));
> +
> + if (!prb_read_valid(prb, con->seq, &r))
> + return false;
> +
> + if (con->seq != r.info->seq) {
> + con->dropped += r.info->seq - con->seq;
> + con->seq = r.info->seq;
> + if (panic_in_progress() && panic_console_dropped++ > 10) {
> + suppress_panic_printk = 1;
> + pr_warn_once("Too many dropped messages. Suppress messages on non-panic CPUs to prevent livelock.\n");
> + }
> + }
> +
> + /*
> + * Skip record that has level above the console loglevel.
> + * Return true so the caller knows a record exists and
> + * leave cmsg->outbuf NULL so the caller knows the record
> + * is being skipped.
> + */
> + if (suppress_message_printing(r.info->level))
> + return true;
> +
> + if (cmsg->is_extmsg) {
> + write_text = &cbufs->ext_text[0];
> + write_text_size = sizeof(cbufs->ext_text);
> + len = info_print_ext_header(write_text, write_text_size, r.info);
> + len += msg_print_ext_body(write_text + len, write_text_size - len,
> + &r.text_buf[0], r.info->text_len, &r.info->dev_info);
> + } else {
> + write_text = &cbufs->text[0];
> + len = record_print_text(&r, console_msg_format & MSG_FORMAT_SYSLOG, printk_time);
> + }
> +
> + cmsg->outbuf = write_text;
> + cmsg->outbuf_len = len;
Please, remove "write_text" variable and use cmsg->outbuf directly.
It would safe one mental transition of buffer names:
cbufs->text -> write_text -> cmsg->outbuf
vs.
cbufs->text -> cmsg->outbuf
Best Regards,
Petr
PS: Please, wait a bit with updating the patches. I have got yet
another idea when seeing the code around dropped messages.
But I have to sleep over it.
My concern is that the message about dropped messages need not
fit into the smaller "cbufs->text" buffer. It might be better
to put it into the bigger one.
We might actually always use the bigger buffer as the output
buffer. The smaller buffer might be only temporary when formatting
the extended messages.
We could replace
struct console_buffers {
char ext_text[CONSOLE_EXT_LOG_MAX];
char text[CONSOLE_LOG_MAX];
};
with
struct console_buffers {
char outbuf[CONSOLE_EXT_LOG_MAX];
char readbuf[CONSOLE_LOG_MAX];
};
Normal consoles would use only @outbuf. Only the extended console
would need the @readbuf to read the messages before they are formatted.
I guess that struct console_message won't be needed then at all.
It might help to remove several twists in the code.
I am sorry that I have not got this idea when reviewing v1.
Well, the code was even more complicated at that time.
On Thu 2022-11-24 19:00:14, Petr Mladek wrote:
> PS: Please, wait a bit with updating the patches. I have got yet
> another idea when seeing the code around dropped messages.
> But I have to sleep over it.
>
> My concern is that the message about dropped messages need not
> fit into the smaller "cbufs->text" buffer. It might be better
> to put it into the bigger one.
>
> We might actually always use the bigger buffer as the output
> buffer. The smaller buffer might be only temporary when formatting
> the extended messages.
>
> We could replace
>
> struct console_buffers {
> char ext_text[CONSOLE_EXT_LOG_MAX];
> char text[CONSOLE_LOG_MAX];
> };
>
> with
>
> struct console_buffers {
> char outbuf[CONSOLE_EXT_LOG_MAX];
> char readbuf[CONSOLE_LOG_MAX];
> };
>
> Normal consoles would use only @outbuf. Only the extended console
> would need the @readbuf to read the messages before they are formatted.
>
> I guess that struct console_message won't be needed then at all.
>
> It might help to remove several twists in the code.
>
> I am sorry that I have not got this idea when reviewing v1.
> Well, the code was even more complicated at that time.
Honestly, I haven't looked if you extended struct console_messages in
later patches that added the atomic consoles. It is possible that
the structure will be needed in the end anyway.
This was just an idea. You know, I always try to simplify things.
And many layers, pointers to the same buffers with different names,
makes things complicated.
Well, there are always many ways how to design the code and I do not
want to delay it too much with trying them all. Please, tell me
when you think that some changes are not worth the effort.
Best Regards,
Petr
On 2022-11-24, Petr Mladek <pmladek@suse.com> wrote:
> I wish, this change was done in two patches. 1st introducing and
> using struct console_message. 2nd moving the code into separate
> console_get_next_message().
OK.
>> + if (cmsg->is_extmsg) {
>> + write_text = &cbufs->ext_text[0];
>> + write_text_size = sizeof(cbufs->ext_text);
>> + len = info_print_ext_header(write_text, write_text_size, r.info);
>> + len += msg_print_ext_body(write_text + len, write_text_size - len,
>> + &r.text_buf[0], r.info->text_len, &r.info->dev_info);
>> + } else {
>> + write_text = &cbufs->text[0];
>> + len = record_print_text(&r, console_msg_format & MSG_FORMAT_SYSLOG, printk_time);
>> + }
>> +
>> + cmsg->outbuf = write_text;
>> + cmsg->outbuf_len = len;
>
> Please, remove "write_text" variable and use cmsg->outbuf directly.
> It would safe one mental transition of buffer names:
>
> cbufs->text -> write_text -> cmsg->outbuf
>
> vs.
>
> cbufs->text -> cmsg->outbuf
I originally had the non-extended case without @write_text. I felt like
it was harder to follow what actually got set. Really the main objective
of the function is to set @outbuf and @outbuf_len. I felt like moving
that outside of the if/else block made it clearer what is going on. But
I can go back to having each if/else branch set those fields in their
own way.
> PS: Please, wait a bit with updating the patches. I have got yet
> another idea when seeing the code around dropped messages.
> But I have to sleep over it.
Don't worry. I always wait until you finish the full review before
touching anything. ;-)
> My concern is that the message about dropped messages need not
> fit into the smaller "cbufs->text" buffer. It might be better
> to put it into the bigger one.
This series _does_ put the dropped messages in the bigger one.
> We might actually always use the bigger buffer as the output
> buffer. The smaller buffer might be only temporary when formatting
> the extended messages.
>
> We could replace
>
> struct console_buffers {
> char ext_text[CONSOLE_EXT_LOG_MAX];
> char text[CONSOLE_LOG_MAX];
> };
>
> with
>
> struct console_buffers {
> char outbuf[CONSOLE_EXT_LOG_MAX];
> char readbuf[CONSOLE_LOG_MAX];
> };
>
> Normal consoles would use only @outbuf. Only the extended console
> would need the @readbuf to read the messages before they are
> formatted.
The "problem" with this idea is that record_print_text() creates the
normal output in-place within the readbuf. So for normal messages with
no dropped messages, we still end up writing out the readbuf.
> I guess that struct console_message won't be needed then at all.
Since we sometimes output the in-place readbuf and sometimes a newly
written buffer, it is nice that console_message can abstract that out.
Also, right now @is_extmsg is the only input variable. For thread/atomic
consoles, the input variables @seq and @dropped will be added.
console_message will then have its own copy of all the information
needed to let itself get filled and console_get_next_message() will no
longer require the console as an argument.
This is important for the thread/atomic consoles because it removes all
locking constraints from console_get_next_message(). For _this_ series,
console_get_next_message() still requires holding the console_lock
because it is accessing con->seq and con->dropped.
I could have added @seq and @dropped to console_message for this series,
but for the legacy consoles it looks like a lot of unnecessary
copying. Only with the thread/atomic consoles does the benefit become
obvious.
> It might help to remove several twists in the code.
A lot of this is preparation for the thread/atomic consoles. It is a
little bit twisty because it is primarily designed for the new
consoles. The trick is to get us from old to new without things getting
crazy in between.
I appreciate your comments. You see things from the perspective of the
"legacy consoles", which is helpful for us to keep things clean and
maintainable during the transition.
John
On Thu 2022-11-24 22:21:08, John Ogness wrote:
> On 2022-11-24, Petr Mladek <pmladek@suse.com> wrote:
> > I wish, this change was done in two patches. 1st introducing and
> > using struct console_message. 2nd moving the code into separate
> > console_get_next_message().
>
> OK.
>
> >> + if (cmsg->is_extmsg) {
> >> + write_text = &cbufs->ext_text[0];
> >> + write_text_size = sizeof(cbufs->ext_text);
> >> + len = info_print_ext_header(write_text, write_text_size, r.info);
> >> + len += msg_print_ext_body(write_text + len, write_text_size - len,
> >> + &r.text_buf[0], r.info->text_len, &r.info->dev_info);
> >> + } else {
> >> + write_text = &cbufs->text[0];
> >> + len = record_print_text(&r, console_msg_format & MSG_FORMAT_SYSLOG, printk_time);
> >> + }
> >> +
> >> + cmsg->outbuf = write_text;
> >> + cmsg->outbuf_len = len;
> >
> > Please, remove "write_text" variable and use cmsg->outbuf directly.
> > It would safe one mental transition of buffer names:
> >
> > cbufs->text -> write_text -> cmsg->outbuf
> >
> > vs.
> >
> > cbufs->text -> cmsg->outbuf
>
> I originally had the non-extended case without @write_text. I felt like
> it was harder to follow what actually got set. Really the main objective
> of the function is to set @outbuf and @outbuf_len. I felt like moving
> that outside of the if/else block made it clearer what is going on. But
> I can go back to having each if/else branch set those fields in their
> own way.
I am not sure if we are talking about the same thing. My idea was to do:
if (cmsg->is_extmsg) {
cmsg->outbuf = &cbufs->ext_text[0];
outbuf_size = sizeof(cbufs->ext_text);
len = info_print_ext_header(cmsg->outbuf, outbuf_size, r.info);
len += msg_print_ext_body(cmsg->outbuf + len, outbuf_size - len,
&r.text_buf[0], r.info->text_len, &r.info->dev_info);
} else {
cmsg->outbuf = &cbufs->text[0];
/* &r points to &cbufs->text[0], changes are done inline */
len = record_print_text(&r, console_msg_format & MSG_FORMAT_SYSLOG, printk_time);
}
> > PS: Please, wait a bit with updating the patches. I have got yet
> > another idea when seeing the code around dropped messages.
> > But I have to sleep over it.
>
> Don't worry. I always wait until you finish the full review before
> touching anything. ;-)
>
> > My concern is that the message about dropped messages need not
> > fit into the smaller "cbufs->text" buffer. It might be better
> > to put it into the bigger one.
>
> This series _does_ put the dropped messages in the bigger one.
Ah, I have overlooked this. It might actually be a motivation to avoid
all these shuffles and really use:
struct console_buffers {
char outbuf[CONSOLE_EXT_LOG_MAX];
char readbuf[CONSOLE_LOG_MAX];
};
> >
> > Normal consoles would use only @outbuf. Only the extended console
> > would need the @readbuf to read the messages before they are
> > formatted.
>
> The "problem" with this idea is that record_print_text() creates the
> normal output in-place within the readbuf. So for normal messages with
> no dropped messages, we still end up writing out the readbuf.
We handle this this in console_get_next_message() by reading the
messages into the right buffer:
boot is_extcon = console_srcu_read_flags(con) & CON_EXTENDED;
/*
* Normal consoles might read the message into the outbuf directly.
* Console headers are added inplace.
*/
if (is_extcon)
prb_rec_init_rd(&r, &info, &cbufs->readbuf[0], sizeof(cbufs->readbuf));
else
prb_rec_init_rd(&r, &info, &cbufs->outbuf[0], sizeof(cbufs->outbuf));
if (!prb_read_valid(prb, con->seq, &r))
return false;
...
if (is_extcon) {
len = info_print_ext_header(cbufs->outbuf, sizeof(cbufs->outbuf, r.info);
len += msg_print_ext_body(cbufs->outbuf + len, sizeof(cbufs->outbuf) - len,
&r.text_buf[0], r.info->text_len, &r.info->dev_info);
} else {
len = record_print_text(&r, console_msg_format & MSG_FORMAT_SYSLOG, printk_time);
}
> > I guess that struct console_message won't be needed then at all.
>
> Since we sometimes output the in-place readbuf and sometimes a newly
> written buffer, it is nice that console_message can abstract that out.
>
> Also, right now @is_extmsg is the only input variable. For thread/atomic
> consoles, the input variables @seq and @dropped will be added.
> console_message will then have its own copy of all the information
> needed to let itself get filled and console_get_next_message() will no
> longer require the console as an argument.
>
> This is important for the thread/atomic consoles because it removes all
> locking constraints from console_get_next_message(). For _this_ series,
> console_get_next_message() still requires holding the console_lock
> because it is accessing con->seq and con->dropped.
>
> I could have added @seq and @dropped to console_message for this series,
> but for the legacy consoles it looks like a lot of unnecessary
> copying. Only with the thread/atomic consoles does the benefit become
> obvious.
I could imagine adding these metadata into the struct console_buffers.
Or we could call it struct console_messages from the beginning.
We could even completely move con->seq, con->dropped into this new
structure. It would safe even more copies.
IMHO, the less structures and the less copying the better.
Especially when the values have different name in each structure
that makes it even more complicated.
Best Regards,
Petr
On 2022-11-25, Petr Mladek <pmladek@suse.com> wrote:
> I am not sure if we are talking about the same thing. My idea was to
> do:
We are and your idea is fine.
>> The "problem" with this idea is that record_print_text() creates the
>> normal output in-place within the readbuf. So for normal messages with
>> no dropped messages, we still end up writing out the readbuf.
>
> We handle this this in console_get_next_message() by reading the
> messages into the right buffer:
>
> struct console_buffers {
> char outbuf[CONSOLE_EXT_LOG_MAX];
> char readbuf[CONSOLE_LOG_MAX];
> };
>
> boot is_extcon = console_srcu_read_flags(con) & CON_EXTENDED;
>
> /*
> * Normal consoles might read the message into the outbuf directly.
> * Console headers are added inplace.
> */
> if (is_extcon)
> prb_rec_init_rd(&r, &info, &cbufs->readbuf[0], sizeof(cbufs->readbuf));
> else
> prb_rec_init_rd(&r, &info, &cbufs->outbuf[0], sizeof(cbufs->outbuf));
>
> if (!prb_read_valid(prb, con->seq, &r))
> return false;
We do not know if there will be dropped messages until _after_ we call
prb_read_valid().
> if (is_extcon) {
> len = info_print_ext_header(cbufs->outbuf, sizeof(cbufs->outbuf, r.info);
> len += msg_print_ext_body(cbufs->outbuf + len, sizeof(cbufs->outbuf) - len,
> &r.text_buf[0], r.info->text_len, &r.info->dev_info);
> } else {
> len = record_print_text(&r, console_msg_format & MSG_FORMAT_SYSLOG, printk_time);
> }
And the dropped messages should be inserted now somewhere too.
The fact is that we need two different sized buffers. Which one is used
for output is not known in advance. I think it is misleading to name the
variables based on their purpose because their purpose changes depending
on the situation.
I think the only usefulness we can derive from the names is their
size. How about naming them:
@buffer_console_log_max
@buffer_console_ext_log_max
>> Since we sometimes output the in-place readbuf and sometimes a newly
>> written buffer, it is nice that console_message can abstract that
>> out.
>>
>> Also, right now @is_extmsg is the only input variable. For
>> thread/atomic consoles, the input variables @seq and @dropped will be
>> added. console_message will then have its own copy of all the
>> information needed to let itself get filled and
>> console_get_next_message() will no longer require the console as an
>> argument.
>>
>> This is important for the thread/atomic consoles because it removes
>> all locking constraints from console_get_next_message(). For _this_
>> series, console_get_next_message() still requires holding the
>> console_lock because it is accessing con->seq and con->dropped.
>>
>> I could have added @seq and @dropped to console_message for this
>> series, but for the legacy consoles it looks like a lot of
>> unnecessary copying. Only with the thread/atomic consoles does the
>> benefit become obvious.
>
> I could imagine adding these metadata into the struct console_buffers.
> Or we could call it struct console_messages from the beginning.
>
> We could even completely move con->seq, con->dropped into this new
> structure. It would safe even more copies.
Well, consoles still need to have their own copy of @seq and @dropped
for proper per-console tracking. But the buffers are globally shared.
For this series I will drop @is_extmsg from struct console_message and
instead make it an argument of console_get_next_message() and
msg_print_dropped(). That makes more sense at this point. (It needs to
be a variable because it is not safe to re-read flags multiple times
when constructing the message.)
For v3 I will move the two buffers (with whatever name we decide is
best) into struct console_message and remove the struct console_buffers
definition. That will also remove the use of @cbufs everywhere.
For devkmsg I can replace @cbufs with separate @readbuf and @outbuf
buffers since that is always their correct purpose for devkmsg.
John
On Fri 2022-11-25 11:55:28, John Ogness wrote:
> On 2022-11-25, Petr Mladek <pmladek@suse.com> wrote:
> >> The "problem" with this idea is that record_print_text() creates the
> >> normal output in-place within the readbuf. So for normal messages with
> >> no dropped messages, we still end up writing out the readbuf.
> >
> > We handle this this in console_get_next_message() by reading the
> > messages into the right buffer:
> >
> > struct console_buffers {
> > char outbuf[CONSOLE_EXT_LOG_MAX];
> > char readbuf[CONSOLE_LOG_MAX];
> > };
> >
> > boot is_extcon = console_srcu_read_flags(con) & CON_EXTENDED;
> >
> > /*
> > * Normal consoles might read the message into the outbuf directly.
> > * Console headers are added inplace.
> > */
> > if (is_extcon)
> > prb_rec_init_rd(&r, &info, &cbufs->readbuf[0], sizeof(cbufs->readbuf));
> > else
> > prb_rec_init_rd(&r, &info, &cbufs->outbuf[0], sizeof(cbufs->outbuf));
> >
> > if (!prb_read_valid(prb, con->seq, &r))
> > return false;
>
> We do not know if there will be dropped messages until _after_ we call
> prb_read_valid().
Yes.
> > if (is_extcon) {
> > len = info_print_ext_header(cbufs->outbuf, sizeof(cbufs->outbuf, r.info);
> > len += msg_print_ext_body(cbufs->outbuf + len, sizeof(cbufs->outbuf) - len,
> > &r.text_buf[0], r.info->text_len, &r.info->dev_info);
> > } else {
> > len = record_print_text(&r, console_msg_format & MSG_FORMAT_SYSLOG, printk_time);
> > }
>
> And the dropped messages should be inserted now somewhere too.
>
> The fact is that we need two different sized buffers. Which one is used
> for output is not known in advance. I think it is misleading to name the
> variables based on their purpose because their purpose changes depending
> on the situation.
I still think that changing the purpose of the buffers complicates
the code[*] and is potential source of problems. It might be even
a sign of a bad design. IMHO, it would be a big win if the buffers
have a better defined meaning.
The message about dropped messages is rather short. What about
using a small buffer on stack. And adding it into outbuf
by shuffling the existing message. It is not that complicated.
IMHO, it would be worth it.
> > I could imagine adding these metadata into the struct console_buffers.
> > Or we could call it struct console_messages from the beginning.
> >
> > We could even completely move con->seq, con->dropped into this new
> > structure. It would safe even more copies.
>
> Well, consoles still need to have their own copy of @seq and @dropped
> for proper per-console tracking. But the buffers are globally shared.
Right. I have missed this. OK, what about the following?
struct console_buffers {
char outbuf[CONSOLE_EXT_LOG_MAX];
char readbuf[CONSOLE_LOG_MAX];
};
struct console_message {
struct console_buffers *bufs;
u64 seq;
unsigned long dropped;
unsigned int len; ???
};
struct console {
[...]
struct console_message *msg;
[...]
};
It will cause that the buffers will be on the 3rd level of
nesting. But I think that we would use the following everywhere
anyway:
void console_func(struct console *con)
{
char *outbuf = con->bufs->outbuf;
do_something(outbuf);
}
We could actually even use in console_get_next_message():
/*
* Normal console headers are added inplace. Extended console
* headers need to read the messages into a separate buffer.
*/
if (is_extcon) {
readbuf = con->bufs->readbuf;
redbuf_size = sizeof(con->bufs->readbuf);
} else {
readbuf = con->bufs->outbuf;
redbuf_size = sizeof(con->bufs->outbuf);
}
IMHO, this should be the only function where this "hack"
would be needed. All others would work just with outbuf.
IMHO, one big advantage is using the same names everywhere.
Another advantage is that it won't be necessary to copy
the values between different structures.
> For this series I will drop @is_extmsg from struct console_message and
> instead make it an argument of console_get_next_message() and
> msg_print_dropped(). That makes more sense at this point. (It needs to
> be a variable because it is not safe to re-read flags multiple times
> when constructing the message.)
>
> For v3 I will move the two buffers (with whatever name we decide is
> best) into struct console_message and remove the struct console_buffers
> definition. That will also remove the use of @cbufs everywhere.
This looks like a bad idea after all. I forgot that we wanted to share
the buffers between non-atomic consoles. It would be ugly to share
also the metadata, like @seq.
That said, I do not want to get stuck on this. If you still
do not like my proposal feel free to keep the text/text_ext
buffers and struct console_message abstraction. I think that
I could live with it.
Best Regards,
Petr
@@ -188,6 +188,21 @@ struct console_buffers {
char text[CONSOLE_LOG_MAX];
};
+/**
+ * struct console_message - console output buffer descriptor
+ * @cbufs: Pointer to console buffers storing the record text
+ * @outbuf: Pointer to the text buffer to be used for writing out
+ * to the device
+ * @outbuf_len: Length of text at @outbuf
+ * @is_extmsg: True if this is an extended format message
+ */
+struct console_message {
+ struct console_buffers *cbufs;
+ char *outbuf;
+ unsigned int outbuf_len;
+ bool is_extmsg;
+};
+
/**
* struct console - The console descriptor structure
* @name: The name of the console driver
@@ -2741,11 +2741,76 @@ static void __console_unlock(void)
up_console_sem();
}
+/**
+ * console_get_next_message - Fill the output buffer with the next record
+ * @con: The console to print on
+ * @cmsg: Pointer to the output buffer descriptor
+ *
+ * Return: False if there is no pending record in the ringbuffer.
+ * True if there is a pending record in the ringbuffer.
+ *
+ * When the return value is true, then the caller must check
+ * @cmsg->outbuf. If not NULL it points to the first character to write
+ * to the device and @cmsg->outbuf_len contains the length of the message.
+ * If it is NULL then the record will be skipped.
+ */
+static bool console_get_next_message(struct console *con, struct console_message *cmsg)
+{
+ struct console_buffers *cbufs = cmsg->cbufs;
+ static int panic_console_dropped;
+ struct printk_info info;
+ struct printk_record r;
+ size_t write_text_size;
+ char *write_text;
+ size_t len;
+
+ cmsg->outbuf = NULL;
+ cmsg->outbuf_len = 0;
+
+ prb_rec_init_rd(&r, &info, &cbufs->text[0], sizeof(cbufs->text));
+
+ if (!prb_read_valid(prb, con->seq, &r))
+ return false;
+
+ if (con->seq != r.info->seq) {
+ con->dropped += r.info->seq - con->seq;
+ con->seq = r.info->seq;
+ if (panic_in_progress() && panic_console_dropped++ > 10) {
+ suppress_panic_printk = 1;
+ pr_warn_once("Too many dropped messages. Suppress messages on non-panic CPUs to prevent livelock.\n");
+ }
+ }
+
+ /*
+ * Skip record that has level above the console loglevel.
+ * Return true so the caller knows a record exists and
+ * leave cmsg->outbuf NULL so the caller knows the record
+ * is being skipped.
+ */
+ if (suppress_message_printing(r.info->level))
+ return true;
+
+ if (cmsg->is_extmsg) {
+ write_text = &cbufs->ext_text[0];
+ write_text_size = sizeof(cbufs->ext_text);
+ len = info_print_ext_header(write_text, write_text_size, r.info);
+ len += msg_print_ext_body(write_text + len, write_text_size - len,
+ &r.text_buf[0], r.info->text_len, &r.info->dev_info);
+ } else {
+ write_text = &cbufs->text[0];
+ len = record_print_text(&r, console_msg_format & MSG_FORMAT_SYSLOG, printk_time);
+ }
+
+ cmsg->outbuf = write_text;
+ cmsg->outbuf_len = len;
+ return true;
+}
+
/*
* Print one record for the given console. The record printed is whatever
* record is the next available record for the given console.
*
- * @cbufs is the console buffers used to string-print the formatted messages.
+ * @cbufs is the console buffers to use for string-printing the message.
*
* @cookie is the cookie from entering the SRCU read-side critical section.
*
@@ -2761,52 +2826,30 @@ static void __console_unlock(void)
static bool console_emit_next_record(struct console *con, struct console_buffers *cbufs,
int cookie, bool *handover)
{
- static int panic_console_dropped;
- struct printk_info info;
- struct printk_record r;
- size_t write_text_size;
+ struct console_message cmsg = {
+ .cbufs = cbufs,
+ .is_extmsg = console_srcu_read_flags(con) & CON_EXTENDED,
+ };
unsigned long flags;
char *dropped_text;
- char *write_text;
- size_t len;
-
- prb_rec_init_rd(&r, &info, &cbufs->text[0], sizeof(cbufs->text));
*handover = false;
- if (!prb_read_valid(prb, con->seq, &r))
+ if (!console_get_next_message(con, &cmsg))
return false;
- if (con->seq != r.info->seq) {
- con->dropped += r.info->seq - con->seq;
- con->seq = r.info->seq;
- if (panic_in_progress() && panic_console_dropped++ > 10) {
- suppress_panic_printk = 1;
- pr_warn_once("Too many dropped messages. Suppress messages on non-panic CPUs to prevent livelock.\n");
- }
- }
-
- /* Skip record that has level above the console loglevel. */
- if (suppress_message_printing(r.info->level)) {
+ if (!cmsg.outbuf) {
+ /* Skipping this record. */
con->seq++;
goto skip;
}
- if (console_srcu_read_flags(con) & CON_EXTENDED) {
+ if (cmsg.is_extmsg) {
/* Extended consoles do not print "dropped messages". */
dropped_text = NULL;
-
- write_text = &cbufs->ext_text[0];
- write_text_size = sizeof(cbufs->ext_text);
- len = info_print_ext_header(write_text, write_text_size, r.info);
- len += msg_print_ext_body(write_text + len, write_text_size - len,
- &r.text_buf[0], r.info->text_len, &r.info->dev_info);
} else {
/* @ext_text is unused. Use it for "dropped messages". */
dropped_text = &cbufs->ext_text[0];
-
- write_text = &cbufs->text[0];
- len = record_print_text(&r, console_msg_format & MSG_FORMAT_SYSLOG, printk_time);
}
/*
@@ -2823,7 +2866,7 @@ static bool console_emit_next_record(struct console *con, struct console_buffers
console_lock_spinning_enable();
stop_critical_timings(); /* don't trace print latency */
- call_console_driver(con, write_text, len, dropped_text);
+ call_console_driver(con, cmsg.outbuf, cmsg.outbuf_len, dropped_text);
start_critical_timings();
con->seq++;