2018-05-31 10:22:13

by Maninder Singh

[permalink] [raw]
Subject: [PATCH 1/2] printk: remove unused flag LOG_NOCONS

This patch removes unused flag LOG_NOCONS for printk.
usage of this flag is removed long back with below commit.

"5c2992ee7fd8a29d04125dc0aa3522784c5fa5eb"
printk: remove console flushing special cases for
partial buffered lines

Signed-off-by: Maninder Singh <[email protected]>
Signed-off-by: Vaneet Narang <[email protected]>
---
kernel/printk/printk.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 2f4af21..ab15903 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -349,10 +349,9 @@ enum con_msg_format_flags {
*/

enum log_flags {
- LOG_NOCONS = 1, /* already flushed, do not print to console */
- LOG_NEWLINE = 2, /* text ended with a newline */
- LOG_PREFIX = 4, /* text started with a prefix */
- LOG_CONT = 8, /* text is a fragment of a continuation line */
+ LOG_NEWLINE = 1, /* text ended with a newline */
+ LOG_PREFIX = 2, /* text started with a prefix */
+ LOG_CONT = 4, /* text is a fragment of a continuation line */
};

struct printk_log {
--
1.9.1



2018-05-31 10:55:49

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: [PATCH 1/2] printk: remove unused flag LOG_NOCONS

On (05/31/18 15:47), Maninder Singh wrote:
>
> This patch removes unused flag LOG_NOCONS for printk.
> usage of this flag is removed long back with below commit.
>
> "5c2992ee7fd8a29d04125dc0aa3522784c5fa5eb"
> printk: remove console flushing special cases for
> partial buffered lines
>
> Signed-off-by: Maninder Singh <[email protected]>
> Signed-off-by: Vaneet Narang <[email protected]>

Makes sense.

Reviewed-by: Sergey Senozhatsky <[email protected]>

-ss

2018-05-31 12:17:29

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH 1/2] printk: remove unused flag LOG_NOCONS

On Thu 2018-05-31 15:47:51, Maninder Singh wrote:
> This patch removes unused flag LOG_NOCONS for printk.
> usage of this flag is removed long back with below commit.

Make sense.

> "5c2992ee7fd8a29d04125dc0aa3522784c5fa5eb"
> printk: remove console flushing special cases for
> partial buffered lines
>
> Signed-off-by: Maninder Singh <[email protected]>
> Signed-off-by: Vaneet Narang <[email protected]>
> ---
> kernel/printk/printk.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 2f4af21..ab15903 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -349,10 +349,9 @@ enum con_msg_format_flags {
> */
>
> enum log_flags {
> - LOG_NOCONS = 1, /* already flushed, do not print to console */
> - LOG_NEWLINE = 2, /* text ended with a newline */
> - LOG_PREFIX = 4, /* text started with a prefix */
> - LOG_CONT = 8, /* text is a fragment of a continuation line */
> + LOG_NEWLINE = 1, /* text ended with a newline */
> + LOG_PREFIX = 2, /* text started with a prefix */
> + LOG_CONT = 4, /* text is a fragment of a continuation line */
> };

Please, do not renumber the bits if there is no real need for it.
The format of the log buffer is read also by external tool like
"crash". It seems that "crash" ignores these flags but...

Best Regards,
Petr

2018-05-31 15:14:29

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/2] printk: remove unused flag LOG_NOCONS

On Thu, 2018-05-31 at 14:16 +0200, Petr Mladek wrote:
> On Thu 2018-05-31 15:47:51, Maninder Singh wrote:
> > This patch removes unused flag LOG_NOCONS for printk.
> > usage of this flag is removed long back with below commit.
>
> Make sense.
>
> > "5c2992ee7fd8a29d04125dc0aa3522784c5fa5eb"
> > printk: remove console flushing special cases for
> > partial buffered lines
[]
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
[]
> > @@ -349,10 +349,9 @@ enum con_msg_format_flags {
> > */
> >
> > enum log_flags {
> > - LOG_NOCONS = 1, /* already flushed, do not print to console */
> > - LOG_NEWLINE = 2, /* text ended with a newline */
> > - LOG_PREFIX = 4, /* text started with a prefix */
> > - LOG_CONT = 8, /* text is a fragment of a continuation line */
> > + LOG_NEWLINE = 1, /* text ended with a newline */
> > + LOG_PREFIX = 2, /* text started with a prefix */
> > + LOG_CONT = 4, /* text is a fragment of a continuation line */
> > };
>
> Please, do not renumber the bits if there is no real need for it.
> The format of the log buffer is read also by external tool like
> "crash". It seems that "crash" ignores these flags but...

Hmm, if it's not an internal interface, then these
definitions should probably be removed from this file
and exposed in a uapi file.


2018-06-04 21:34:53

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 1/2] printk: remove unused flag LOG_NOCONS

On Thu, 31 May 2018 14:16:33 +0200
Petr Mladek <[email protected]> wrote:

> > enum log_flags {
> > - LOG_NOCONS = 1, /* already flushed, do not print to console */
> > - LOG_NEWLINE = 2, /* text ended with a newline */
> > - LOG_PREFIX = 4, /* text started with a prefix */
> > - LOG_CONT = 8, /* text is a fragment of a continuation line */
> > + LOG_NEWLINE = 1, /* text ended with a newline */
> > + LOG_PREFIX = 2, /* text started with a prefix */
> > + LOG_CONT = 4, /* text is a fragment of a continuation line */
> > };
>
> Please, do not renumber the bits if there is no real need for it.
> The format of the log buffer is read also by external tool like
> "crash". It seems that "crash" ignores these flags but...

Then what's the problem for renumbering? I've renumbered internal flags
before. No one complained about it.

-- Steve

2018-06-05 12:56:57

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH 1/2] printk: remove unused flag LOG_NOCONS

On Mon 2018-06-04 17:33:42, Steven Rostedt wrote:
> On Thu, 31 May 2018 14:16:33 +0200
> Petr Mladek <[email protected]> wrote:
>
> > > enum log_flags {
> > > - LOG_NOCONS = 1, /* already flushed, do not print to console */
> > > - LOG_NEWLINE = 2, /* text ended with a newline */
> > > - LOG_PREFIX = 4, /* text started with a prefix */
> > > - LOG_CONT = 8, /* text is a fragment of a continuation line */
> > > + LOG_NEWLINE = 1, /* text ended with a newline */
> > > + LOG_PREFIX = 2, /* text started with a prefix */
> > > + LOG_CONT = 4, /* text is a fragment of a continuation line */
> > > };
> >
> > Please, do not renumber the bits if there is no real need for it.
> > The format of the log buffer is read also by external tool like
> > "crash". It seems that "crash" ignores these flags but...
>
> Then what's the problem for renumbering? I've renumbered internal flags
> before. No one complained about it.

Steven, did you renumber enum log_flags or flags in a different
subsystem?

Note that struct printk_log is a bit special because it is used by
the "crash" tool to implement the dmesg/log command. While "crash"
tool does not have special handling for most other internal
structures.

I have double checked "crash" sources and it ignores these flags
at the moment but it might change in the future => I suggest to
do not renumber them if there is not a real need.

Best Regards,
Petr

2018-06-05 13:00:29

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH 1/2] printk: remove unused flag LOG_NOCONS

On Thu 2018-05-31 08:12:23, Joe Perches wrote:
> On Thu, 2018-05-31 at 14:16 +0200, Petr Mladek wrote:
> > On Thu 2018-05-31 15:47:51, Maninder Singh wrote:
> > > This patch removes unused flag LOG_NOCONS for printk.
> > > usage of this flag is removed long back with below commit.
> >
> > Make sense.
> >
> > > "5c2992ee7fd8a29d04125dc0aa3522784c5fa5eb"
> > > printk: remove console flushing special cases for
> > > partial buffered lines
> []
> > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> []
> > > @@ -349,10 +349,9 @@ enum con_msg_format_flags {
> > > */
> > >
> > > enum log_flags {
> > > - LOG_NOCONS = 1, /* already flushed, do not print to console */
> > > - LOG_NEWLINE = 2, /* text ended with a newline */
> > > - LOG_PREFIX = 4, /* text started with a prefix */
> > > - LOG_CONT = 8, /* text is a fragment of a continuation line */
> > > + LOG_NEWLINE = 1, /* text ended with a newline */
> > > + LOG_PREFIX = 2, /* text started with a prefix */
> > > + LOG_CONT = 4, /* text is a fragment of a continuation line */
> > > };
> >
> > Please, do not renumber the bits if there is no real need for it.
> > The format of the log buffer is read also by external tool like
> > "crash". It seems that "crash" ignores these flags but...
>
> Hmm, if it's not an internal interface, then these
> definitions should probably be removed from this file
> and exposed in a uapi file.

"crash" is not a typical userspace tool. We break it regularly.
We do not need to put the API into the stone because of "crash".
On the other hand, we also need not break it if it can be avoided
easily.

Best Regards,
Petr