In the past, `enum log_flags` was part of `struct log`, hence the name.
`struct log` has since been reworked and now this struct is stored
inside `struct printk_info`. However, the name was never updated, which
is somewhat confusing -- especially since these flags operate at the
record level rather than at the level of an abstract log.
printk_info_flags also joins its other metadata struct friends in
printk_ringbuffer.h.
Signed-off-by: Chris Down <[email protected]>
Cc: Petr Mladek <[email protected]>
---
kernel/printk/printk.c | 43 ++++++++++++++-----------------
kernel/printk/printk_ringbuffer.h | 6 +++++
2 files changed, 26 insertions(+), 23 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 114e9963f903..3201bb0c269c 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -350,11 +350,6 @@ static int console_msg_format = MSG_FORMAT_DEFAULT;
* non-prinatable characters are escaped in the "\xff" notation.
*/
-enum log_flags {
- LOG_NEWLINE = 2, /* text ended with a newline */
- LOG_CONT = 8, /* text is a fragment of a continuation line */
-};
-
/* syslog_lock protects syslog_* variables and write access to clear_seq. */
static DEFINE_RAW_SPINLOCK(syslog_lock);
@@ -1966,19 +1961,20 @@ static inline u32 printk_caller_id(void)
*
* @text: The terminated text message.
* @level: A pointer to the current level value, will be updated.
- * @lflags: A pointer to the current log flags, will be updated.
+ * @flags: A pointer to the current printk_info flags, will be updated.
*
* @level may be NULL if the caller is not interested in the parsed value.
* Otherwise the variable pointed to by @level must be set to
* LOGLEVEL_DEFAULT in order to be updated with the parsed value.
*
- * @lflags may be NULL if the caller is not interested in the parsed value.
- * Otherwise the variable pointed to by @lflags will be OR'd with the parsed
+ * @flags may be NULL if the caller is not interested in the parsed value.
+ * Otherwise the variable pointed to by @flags will be OR'd with the parsed
* value.
*
* Return: The length of the parsed level and control flags.
*/
-static u16 parse_prefix(char *text, int *level, enum log_flags *lflags)
+static u16 parse_prefix(char *text, int *level,
+ enum printk_info_flags *flags)
{
u16 prefix_len = 0;
int kern_level;
@@ -1994,8 +1990,8 @@ static u16 parse_prefix(char *text, int *level, enum log_flags *lflags)
*level = kern_level - '0';
break;
case 'c': /* KERN_CONT */
- if (lflags)
- *lflags |= LOG_CONT;
+ if (flags)
+ *flags |= LOG_CONT;
}
prefix_len += 2;
@@ -2005,8 +2001,9 @@ static u16 parse_prefix(char *text, int *level, enum log_flags *lflags)
return prefix_len;
}
-static u16 printk_sprint(char *text, u16 size, int facility, enum log_flags *lflags,
- const char *fmt, va_list args)
+static u16 printk_sprint(char *text, u16 size, int facility,
+ enum printk_info_flags *flags, const char *fmt,
+ va_list args)
{
u16 text_len;
@@ -2015,7 +2012,7 @@ static u16 printk_sprint(char *text, u16 size, int facility, enum log_flags *lfl
/* Mark and strip a trailing newline. */
if (text_len && text[text_len - 1] == '\n') {
text_len--;
- *lflags |= LOG_NEWLINE;
+ *flags |= LOG_NEWLINE;
}
/* Strip log level and control flags. */
@@ -2039,7 +2036,7 @@ int vprintk_store(int facility, int level,
{
const u32 caller_id = printk_caller_id();
struct prb_reserved_entry e;
- enum log_flags lflags = 0;
+ enum printk_info_flags flags = 0;
struct printk_record r;
u16 trunc_msg_len = 0;
char prefix_buf[8];
@@ -2071,22 +2068,22 @@ int vprintk_store(int facility, int level,
/* Extract log level or control flags. */
if (facility == 0)
- parse_prefix(&prefix_buf[0], &level, &lflags);
+ parse_prefix(&prefix_buf[0], &level, &flags);
if (level == LOGLEVEL_DEFAULT)
level = default_message_loglevel;
if (dev_info)
- lflags |= LOG_NEWLINE;
+ flags |= LOG_NEWLINE;
- if (lflags & LOG_CONT) {
+ if (flags & LOG_CONT) {
prb_rec_init_wr(&r, reserve_size);
if (prb_reserve_in_last(&e, prb, &r, caller_id, LOG_LINE_MAX)) {
text_len = printk_sprint(&r.text_buf[r.info->text_len], reserve_size,
- facility, &lflags, fmt, args);
+ facility, &flags, fmt, args);
r.info->text_len += text_len;
- if (lflags & LOG_NEWLINE) {
+ if (flags & LOG_NEWLINE) {
r.info->flags |= LOG_NEWLINE;
prb_final_commit(&e);
} else {
@@ -2113,20 +2110,20 @@ int vprintk_store(int facility, int level,
}
/* fill message */
- text_len = printk_sprint(&r.text_buf[0], reserve_size, facility, &lflags, fmt, args);
+ text_len = printk_sprint(&r.text_buf[0], reserve_size, facility, &flags, fmt, args);
if (trunc_msg_len)
memcpy(&r.text_buf[text_len], trunc_msg, trunc_msg_len);
r.info->text_len = text_len + trunc_msg_len;
r.info->facility = facility;
r.info->level = level & 7;
- r.info->flags = lflags & 0x1f;
+ r.info->flags = flags & 0x1f;
r.info->ts_nsec = ts_nsec;
r.info->caller_id = caller_id;
if (dev_info)
memcpy(&r.info->dev_info, dev_info, sizeof(r.info->dev_info));
/* A message without a trailing newline can be continued. */
- if (!(lflags & LOG_NEWLINE))
+ if (!(flags & LOG_NEWLINE))
prb_commit(&e);
else
prb_final_commit(&e);
diff --git a/kernel/printk/printk_ringbuffer.h b/kernel/printk/printk_ringbuffer.h
index 73cc80e01cef..71918d47ca95 100644
--- a/kernel/printk/printk_ringbuffer.h
+++ b/kernel/printk/printk_ringbuffer.h
@@ -50,6 +50,12 @@ struct prb_data_blk_lpos {
unsigned long next;
};
+/* Flags for a single printk record. */
+enum printk_info_flags {
+ LOG_NEWLINE = 2, /* text ended with a newline */
+ LOG_CONT = 8, /* text is a fragment of a continuation line */
+};
+
/*
* A descriptor: the complete meta-data for a record.
*
--
2.31.1
On 2021-05-25, Petr Mladek <[email protected]> wrote:
>> diff --git a/kernel/printk/printk_ringbuffer.h b/kernel/printk/printk_ringbuffer.h
>> index 73cc80e01cef..71918d47ca95 100644
>> --- a/kernel/printk/printk_ringbuffer.h
>> +++ b/kernel/printk/printk_ringbuffer.h
>> @@ -50,6 +50,12 @@ struct prb_data_blk_lpos {
>> unsigned long next;
>> };
>>
>> +/* Flags for a single printk record. */
>> +enum printk_info_flags {
>> + LOG_NEWLINE = 2, /* text ended with a newline */
>> + LOG_CONT = 8, /* text is a fragment of a continuation line */
>> +};
>
> Nit: Could you please move this after "enum desc_state" declaration?
Is there a reason that this enum is moved into printk_ringbuffer.h? The
ringbuffer does not use this enum.
John Ogness
On Tue 2021-05-18 13:00:38, Chris Down wrote:
> In the past, `enum log_flags` was part of `struct log`, hence the name.
> `struct log` has since been reworked and now this struct is stored
> inside `struct printk_info`. However, the name was never updated, which
> is somewhat confusing -- especially since these flags operate at the
> record level rather than at the level of an abstract log.
>
> printk_info_flags also joins its other metadata struct friends in
> printk_ringbuffer.h.
>
> Signed-off-by: Chris Down <[email protected]>
> Cc: Petr Mladek <[email protected]>
Reviewed-by: Petr Mladek <[email protected]>
Just one nit below.
> diff --git a/kernel/printk/printk_ringbuffer.h b/kernel/printk/printk_ringbuffer.h
> index 73cc80e01cef..71918d47ca95 100644
> --- a/kernel/printk/printk_ringbuffer.h
> +++ b/kernel/printk/printk_ringbuffer.h
> @@ -50,6 +50,12 @@ struct prb_data_blk_lpos {
> unsigned long next;
> };
>
> +/* Flags for a single printk record. */
> +enum printk_info_flags {
> + LOG_NEWLINE = 2, /* text ended with a newline */
> + LOG_CONT = 8, /* text is a fragment of a continuation line */
> +};
Nit: Could you please move this after "enum desc_state" declaration?
> /*
> * A descriptor: the complete meta-data for a record.
> *
> --
> 2.31.1
Best Regards,
Petr
On 2021-05-26, Petr Mladek <[email protected]> wrote:
>> >> diff --git a/kernel/printk/printk_ringbuffer.h b/kernel/printk/printk_ringbuffer.h
>> >> index 73cc80e01cef..71918d47ca95 100644
>> >> --- a/kernel/printk/printk_ringbuffer.h
>> >> +++ b/kernel/printk/printk_ringbuffer.h
>> >> @@ -50,6 +50,12 @@ struct prb_data_blk_lpos {
>> >> unsigned long next;
>> >> };
>> >>
>> >> +/* Flags for a single printk record. */
>> >> +enum printk_info_flags {
>> >> + LOG_NEWLINE = 2, /* text ended with a newline */
>> >> + LOG_CONT = 8, /* text is a fragment of a continuation line */
>> >> +};
>> >
>> > Nit: Could you please move this after "enum desc_state" declaration?
>>
>> Is there a reason that this enum is moved into printk_ringbuffer.h? The
>> ringbuffer does not use this enum.
>
> We want to use it in two source files: printk.c and index.c
> Alternative solution would be to move it to kernel/printk/internal.h.
>
> Well, will internal.h still be needed after we remove printk_safe?
We wouldn't be able to remove internal.h until deferred printing is
removed. And that cannot happen until after printing kthreads exist. So
it will still hang around for a while.
But even so, I do not like the idea of turning printk_ringbuffer.h into
the new internal.h. The ringbuffer is quite complex and IMHO we should
try to keep the printk_ringbuffer.* files as isolated as possible.
I would prefer to put this enum declaration in internal.h. Even if
eventually it is the only thing left in internal.h.
John Ogness
On Wed 2021-05-26 10:39:18, John Ogness wrote:
> On 2021-05-26, Petr Mladek <[email protected]> wrote:
> >> >> diff --git a/kernel/printk/printk_ringbuffer.h b/kernel/printk/printk_ringbuffer.h
> >> >> index 73cc80e01cef..71918d47ca95 100644
> >> >> --- a/kernel/printk/printk_ringbuffer.h
> >> >> +++ b/kernel/printk/printk_ringbuffer.h
> >> >> @@ -50,6 +50,12 @@ struct prb_data_blk_lpos {
> >> >> unsigned long next;
> >> >> };
> >> >>
> >> >> +/* Flags for a single printk record. */
> >> >> +enum printk_info_flags {
> >> >> + LOG_NEWLINE = 2, /* text ended with a newline */
> >> >> + LOG_CONT = 8, /* text is a fragment of a continuation line */
> >> >> +};
> >> >
> >> > Nit: Could you please move this after "enum desc_state" declaration?
> >>
> >> Is there a reason that this enum is moved into printk_ringbuffer.h? The
> >> ringbuffer does not use this enum.
> >
> > We want to use it in two source files: printk.c and index.c
> > Alternative solution would be to move it to kernel/printk/internal.h.
> >
> > Well, will internal.h still be needed after we remove printk_safe?
>
> We wouldn't be able to remove internal.h until deferred printing is
> removed. And that cannot happen until after printing kthreads exist. So
> it will still hang around for a while.
>
> But even so, I do not like the idea of turning printk_ringbuffer.h into
> the new internal.h. The ringbuffer is quite complex and IMHO we should
> try to keep the printk_ringbuffer.* files as isolated as possible.
>
> I would prefer to put this enum declaration in internal.h. Even if
> eventually it is the only thing left in internal.h.
Fair enough. Chris, please move enum printk_info_flags declaration
into kernel/printk/internal.h
Best Regards,
Petr
On Tue 2021-05-25 13:35:29, John Ogness wrote:
> On 2021-05-25, Petr Mladek <[email protected]> wrote:
> >> diff --git a/kernel/printk/printk_ringbuffer.h b/kernel/printk/printk_ringbuffer.h
> >> index 73cc80e01cef..71918d47ca95 100644
> >> --- a/kernel/printk/printk_ringbuffer.h
> >> +++ b/kernel/printk/printk_ringbuffer.h
> >> @@ -50,6 +50,12 @@ struct prb_data_blk_lpos {
> >> unsigned long next;
> >> };
> >>
> >> +/* Flags for a single printk record. */
> >> +enum printk_info_flags {
> >> + LOG_NEWLINE = 2, /* text ended with a newline */
> >> + LOG_CONT = 8, /* text is a fragment of a continuation line */
> >> +};
> >
> > Nit: Could you please move this after "enum desc_state" declaration?
>
> Is there a reason that this enum is moved into printk_ringbuffer.h? The
> ringbuffer does not use this enum.
We want to use it in two source files: printk.c and index.c
Alternative solution would be to move it to kernel/printk/internal.h.
Well, will internal.h still be needed after we remove printk_safe?
Anyway, I do not have strong opinion about it.
Best Regards,
Petr
Petr Mladek writes:
>Fair enough. Chris, please move enum printk_info_flags declaration
>into kernel/printk/internal.h
Sure thing, will do for v7 once the points in the other patch are agreed on.
Thanks! :-)