[PATCH] commands: dmesg: clear logbuffer fully if not requested otherwise

[PATCH] commands: dmesg: clear logbuffer fully if not requested otherwise

[Jonas Rebmann]

Previously, `dmesg -c` would clear the logbuffer except for the last 10
lines. This comes with a fair amount of surprise given linux `dmesg -c`
clears the whole logbuffer.

Clear the complete logbuffer given `dmesg -c` but with an optional
argument `dmesg -c [<num>]` to keep <num> lines.

While at it, move the deleteion of log lines to the log_print function
to prevent a corner case of lost log message if they arrive between the
log_print() and log_clean() in dmesg.

If loglevels are selected using -l or -p in addition to cleaning being
selected using -c, only clean those messages shown by the loglevel
selection.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 commands/dmesg.c        | 17 ++++++++++-------
 common/console_common.c | 20 +++++++++++++++++---
 include/linux/printk.h  |  2 +-
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/commands/dmesg.c b/commands/dmesg.c
index a93ad5b359..6853f3afc9 100644
--- a/commands/dmesg.c
+++ b/commands/dmesg.c
@@ -77,14 +77,20 @@ static unsigned dmesg_get_levels(const char *__args)
 static int do_dmesg(int argc, char *argv[])
 {
 	int opt, ret, i;
-	int delete_buf = 0, emit = 0;
+	int delete_buf = -1, emit = 0;
 	unsigned flags = 0, levels = 0;
 	char *set = NULL;
 
-	while ((opt = getopt(argc, argv, "ctderl:p:n:")) > 0) {
+	while ((opt = getopt(argc, argv, "c::tderl:p:n:")) > 0) {
 		switch (opt) {
 		case 'c':
-			delete_buf = 1;
+			if (optarg) {
+				ret = kstrtoint(optarg, 10, &delete_buf);
+				if (ret || delete_buf < 0)
+					return COMMAND_ERROR_USAGE;
+			} else {
+				delete_buf = 0;
+			}
 			break;
 		case 't':
 			flags |= BAREBOX_LOG_PRINT_TIME;
@@ -155,13 +161,10 @@ static int do_dmesg(int argc, char *argv[])
 		return 0;
 	}
 
-	ret = log_print(flags, levels);
+	ret = log_print(flags, levels, delete_buf);
 	if (ret)
 		return 1;
 
-	if (delete_buf)
-		log_clean(10);
-
 	return 0;
 }
 
diff --git a/common/console_common.c b/common/console_common.c
index 5b7a64c99c..23646d8f81 100644
--- a/common/console_common.c
+++ b/common/console_common.c
@@ -210,12 +210,23 @@ int log_writefile(const char *filepath)
 	return ret < 0 ? ret : nbytes;
 }
 
-int log_print(unsigned flags, unsigned levels)
+/**
+ * log_print - print and optionally clear the log buffer
+ *
+ * @flags:	Flags selecting output formatting
+ * @levels:	bitmask of loglevels to print, 0 for all
+ * @delete_buf:	Number of printed lines to keep in the buffer,
+ *		-1 to keep all.
+ *
+ * This function deletes all messages in the logbuf exceeding
+ * the limit.
+ */
+int log_print(unsigned flags, unsigned levels, int delete_buf)
 {
-	struct log_entry *log;
+	struct log_entry *log, *tmp;
 	unsigned long last = 0;
 
-	list_for_each_entry(log, &barebox_logbuf, list) {
+	list_for_each_entry_safe(log, tmp, &barebox_logbuf, list) {
 		uint64_t time_ns = log->timestamp;
 		unsigned long time;
 
@@ -250,6 +261,9 @@ int log_print(unsigned flags, unsigned levels)
 			printf("] ");
 
 		printf("%s", log->msg);
+
+		if (delete_buf >= 0 && barebox_logbuf_num_messages > delete_buf)
+			log_del(log);
 	}
 
 	return 0;
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 0d7180f0eb..78ab3e6d16 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -192,7 +192,7 @@ extern void log_clean(unsigned int limit);
 #define BAREBOX_LOG_PRINT_TIME		BIT(0)
 
 int log_writefile(const char *filepath);
-int log_print(unsigned flags, unsigned levels);
+int log_print(unsigned flags, unsigned levels, int delete_buf);
 
 struct va_format {
 	const char *fmt;

---
base-commit: 8defba1d0ab1aef9dd5d57710e18d0d02e2c48e2
change-id: 20251017-dmesg-05b9e462e8a1

Best regards,
--  
Jonas Rebmann <jre@pengutronix.de>

Re: [PATCH] commands: dmesg: clear logbuffer fully if not requested otherwise

[Ahmad Fatoum]

Hi,

On 10/17/25 1:04 PM, Jonas Rebmann wrote:
> Previously, `dmesg -c` would clear the logbuffer except for the last 10
> lines. This comes with a fair amount of surprise given linux `dmesg -c`
> clears the whole logbuffer.
> 
> Clear the complete logbuffer given `dmesg -c` but with an optional
> argument `dmesg -c [<num>]` to keep <num> lines.
> 
> While at it, move the deleteion of log lines to the log_print function

deletion

> to prevent a corner case of lost log message if they arrive between the
> log_print() and log_clean() in dmesg.
> 
> If loglevels are selected using -l or -p in addition to cleaning being
> selected using -c, only clean those messages shown by the loglevel
> selection.

Given that this might break test suites, I think it warrants a short
addition to migration-2025.11.0.rst (create if needed).

> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>  commands/dmesg.c        | 17 ++++++++++-------
>  common/console_common.c | 20 +++++++++++++++++---
>  include/linux/printk.h  |  2 +-
>  3 files changed, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/commands/dmesg.c b/commands/dmesg.c
> index a93ad5b359..6853f3afc9 100644
> --- a/commands/dmesg.c
> +++ b/commands/dmesg.c
> @@ -77,14 +77,20 @@ static unsigned dmesg_get_levels(const char *__args)
>  static int do_dmesg(int argc, char *argv[])
>  {
>  	int opt, ret, i;
> -	int delete_buf = 0, emit = 0;
> +	int delete_buf = -1, emit = 0;

just make it unsigned and use UINT_MAX?

delete_buf is no longer a good name for the variable IMO.

>  	unsigned flags = 0, levels = 0;
>  	char *set = NULL;
>  
> -	while ((opt = getopt(argc, argv, "ctderl:p:n:")) > 0) {
> +	while ((opt = getopt(argc, argv, "c::tderl:p:n:")) > 0) {
>  		switch (opt) {
>  		case 'c':
> -			delete_buf = 1;
> +			if (optarg) {
> +				ret = kstrtoint(optarg, 10, &delete_buf);

With type changed above, kstrtouint

> +				if (ret || delete_buf < 0)

... which simplifies this condition.

> +					return COMMAND_ERROR_USAGE;
> +			} else {
> +				delete_buf = 0;

You can initialize this right after case '0': and drop the else.

> +			}
>  			break;
>  		case 't':
>  			flags |= BAREBOX_LOG_PRINT_TIME;
> @@ -155,13 +161,10 @@ static int do_dmesg(int argc, char *argv[])
>  		return 0;
>  	}
>  
> -	ret = log_print(flags, levels);
> +	ret = log_print(flags, levels, delete_buf);

Not sure, we want to overload log_print with this.
How about we switch the ctrlc() to ctrlc_non_interruptible() and then we
are certain that there will be no messages coming in between as there
will be no resched() points (barebox multitasking is completely
cooperative).

> -int log_print(unsigned flags, unsigned levels)
> +/**

Thanks for adding docs :)

> +		if (delete_buf >= 0 && barebox_logbuf_num_messages > delete_buf)

If delete_buf were unsigned, it would simplify the condition.
The variable name is suboptimal though, something with keep in it would
be better.


Cheers,
Ahmad

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH] commands: dmesg: clear logbuffer fully if not requested otherwise

[Sascha Hauer]

On Fri, Oct 17, 2025 at 01:04:36PM +0200, Jonas Rebmann wrote:
> Previously, `dmesg -c` would clear the logbuffer except for the last 10
> lines. This comes with a fair amount of surprise given linux `dmesg -c`
> clears the whole logbuffer.
> 
> Clear the complete logbuffer given `dmesg -c` but with an optional
> argument `dmesg -c [<num>]` to keep <num> lines.
> 
> While at it, move the deleteion of log lines to the log_print function
> to prevent a corner case of lost log message if they arrive between the
> log_print() and log_clean() in dmesg.
> 
> If loglevels are selected using -l or -p in addition to cleaning being
> selected using -c, only clean those messages shown by the loglevel
> selection.

I find this behaviour rather surprising. The only useful thing I could
think of we can do with this is "Discard unimportant messages", but only
by printing them. It would be more useful if we had a -C (uppercase)
option to discard messages without printing them. But even with that,
there are many things we can do with this behaviour that are not useful
and only a few that are actually useful. We could delete for example all
"info" messages and keep the more and less important messages, but why
would we want to do this?

I think you should rather use the filter options -l and -p to limit the
output to the messages you are interested in and finally use -c to clear
the buffer.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] commands: dmesg: clear logbuffer fully if not requested otherwise

[Jonas Rebmann]

Hi Sascha,

On 2025-10-20 11:36, Sascha Hauer wrote:
>> If loglevels are selected using -l or -p in addition to cleaning being
>> selected using -c, only clean those messages shown by the loglevel
>> selection.
> 
> I find this behaviour rather surprising. The only useful thing I could
> think of we can do with this is "Discard unimportant messages", but only
> by printing them. It would be more useful if we had a -C (uppercase)
> option to discard messages without printing them. But even with that,
> there are many things we can do with this behaviour that are not useful
> and only a few that are actually useful. We could delete for example all
> "info" messages and keep the more and less important messages, but why
> would we want to do this?
> > I think you should rather use the filter options -l and -p to limit the
> output to the messages you are interested in and finally use -c to clear
> the buffer.

I think both behaviors could come as a surprise depending on how you
look at it but if no one raises another position, I can make sure that
in v2, -c will always clear all messages of all loglevels which is also
much closer to the current behavior. I'd then add a warning that in
these cases, filtered-out messages are cleared without being shown.

However I disagree about the -C option and the idea of clearing in a
separate step.

My concept of -c is that it 'consumes' the ringbuffer, a 'dequeue'
operation, if you will. This needs to be a single step to ensure that no
messages get lost or duplicated.

For example in some long-running test, you might run `dmesg -c`
periodically and store the output. It should be guaranteed that when
doing this, you never loose a single log message (if you clear often
enough to never fill/overflow the buffer). I think the prior
implementation couldn't guarantee that, but my patch changes that. From
that perspective, the option to clear the buffer in a separate step
would not help.

In a 'consume and filter' scenario, one should just use dmesg -c -r and
perform the loglevel filtering elsewhere instead of 'consuming' per
loglevel by mixing -c with -l/-p which is why I'm ok with the change.

Regards,
Jonas

-- 
Pengutronix e.K.                           | Jonas Rebmann               |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH] commands: dmesg: clear logbuffer fully if not requested otherwise

[Sascha Hauer]

On Mon, Oct 20, 2025 at 02:36:11PM +0200, Jonas Rebmann wrote:
> Hi Sascha,
> 
> On 2025-10-20 11:36, Sascha Hauer wrote:
> > > If loglevels are selected using -l or -p in addition to cleaning being
> > > selected using -c, only clean those messages shown by the loglevel
> > > selection.
> > 
> > I find this behaviour rather surprising. The only useful thing I could
> > think of we can do with this is "Discard unimportant messages", but only
> > by printing them. It would be more useful if we had a -C (uppercase)
> > option to discard messages without printing them. But even with that,
> > there are many things we can do with this behaviour that are not useful
> > and only a few that are actually useful. We could delete for example all
> > "info" messages and keep the more and less important messages, but why
> > would we want to do this?
> > > I think you should rather use the filter options -l and -p to limit the
> > output to the messages you are interested in and finally use -c to clear
> > the buffer.
> 
> I think both behaviors could come as a surprise depending on how you
> look at it but if no one raises another position, I can make sure that
> in v2, -c will always clear all messages of all loglevels which is also
> much closer to the current behavior. I'd then add a warning that in
> these cases, filtered-out messages are cleared without being shown.

At least clearing all messages would be in line with dmesg -c in Linux.

> 
> However I disagree about the -C option and the idea of clearing in a
> separate step.
> 
> My concept of -c is that it 'consumes' the ringbuffer, a 'dequeue'
> operation, if you will. This needs to be a single step to ensure that no
> messages get lost or duplicated.
> 
> For example in some long-running test, you might run `dmesg -c`
> periodically and store the output. It should be guaranteed that when
> doing this, you never loose a single log message (if you clear often
> enough to never fill/overflow the buffer). I think the prior
> implementation couldn't guarantee that, but my patch changes that. From
> that perspective, the option to clear the buffer in a separate step
> would not help.

I rather meant while using dmesg interactively. For the long-running
test I agree, we shouldn't lose log messages in this case, but Ahmad
provided a way to archieve this.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |