[PATCH 0/5] printk: Use whole ring buffer + some clean up

[PATCH 0/5] printk: Use whole ring buffer + some clean up

[Petr Mladek]

I have found few small problems when working on safe printk in NMI context.
The NMI stuff still needs some love. Here is the independent stuff that is
ready to go.

1st and 5th patch do small optimizations. They remove a duplicate computation.

2nd patch removes some unused code.

3rd patch adds a comment that would have saved me some time when trying to
understand the code. Well, maybe it is useful only for kernel newbies
like me ;-)

4th patch fixes two problems from the "by-one" department. They caused that
we have newer used the last 4 bytes of the ring buffer.

Petr Mladek (5):
  printk: Remove duplicated check for log level
  printk: Remove obsolete check for log level "c"
  printk: Add comment about tricky check for text buffer size
  printk: Use also the last bytes in the ring buffer
  printk: Do not compute the size of the message twice

 include/linux/printk.h | 10 +++-------
 kernel/printk/printk.c | 13 ++++++++-----
 2 files changed, 11 insertions(+), 12 deletions(-)

-- 
1.8.4

[PATCH 1/5] printk: Remove duplicated check for log level

[Petr Mladek]

The check for the exact log level is already done in printk_get_level.
We do not need to duplicate it in printk_skip_level.

Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
 include/linux/printk.h | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index fa47e2708c01..8860575899f2 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -24,13 +24,9 @@ static inline int printk_get_level(const char *buffer)
 
 static inline const char *printk_skip_level(const char *buffer)
 {
-	if (printk_get_level(buffer)) {
-		switch (buffer[1]) {
-		case '0' ... '7':
-		case 'd':	/* KERN_DEFAULT */
-			return buffer + 2;
-		}
-	}
+	if (printk_get_level(buffer))
+		return buffer + 2;
+
 	return buffer;
 }
 
-- 
1.8.4

[PATCH 2/5] printk: Remove obsolete check for log level "c"

[Petr Mladek]

The kernel log level "c" has been removed in the commit 61e99ab8e35a88b8
(printk: remove the now unnecessary "C" annotation for KERN_CONT). It is not
longer detected in printk_get_level. Hence we do not need to check it in
vprintk_emit.

Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
 kernel/printk/printk.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index b1d255f04135..54a4439d021c 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1561,8 +1561,6 @@ asmlinkage int vprintk_emit(int facility, int level,
 					level = kern_level - '0';
 			case 'd':	/* KERN_DEFAULT */
 				lflags |= LOG_PREFIX;
-			case 'c':	/* KERN_CONT */
-				break;
 			}
 			text_len -= end_of_header - text;
 			text = (char *)end_of_header;
-- 
1.8.4

[PATCH 3/5] printk: Add comment about tricky check for text buffer size

[Petr Mladek]

There is no check for potential "text_len" overflow. It is not needed
because only valid level is detected. It took me some time to understand why.
It would deserve a comment ;-)

Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
 kernel/printk/printk.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 54a4439d021c..bc6eed48a454 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1562,6 +1562,11 @@ asmlinkage int vprintk_emit(int facility, int level,
 			case 'd':	/* KERN_DEFAULT */
 				lflags |= LOG_PREFIX;
 			}
+			/*
+			 * No need to check length here because vscnprintf
+			 * put '\0' at the end of the string. Only valid and
+			 * newly printed level is detected.
+			 */
 			text_len -= end_of_header - text;
 			text = (char *)end_of_header;
 		}
-- 
1.8.4

[PATCH 4/5] printk: Use also the last bytes in the ring buffer

[Petr Mladek]

It seems that we have newer used the last byte in the ring buffer. In fact,
we have newer used the last 4 bytes because of padding.

First problem is in the check for free space. The exact number of free bytes
is enough to store the length of data.

Second problem is in the check where the ring buffer is rotated. The left side
counts the first unused index. It is unused, so it might be the same as the size
of the buffer.

Note that the first problem has to be fixed together with the second one.
Otherwise, the buffer is rotated even when there is enough space on the
end of the buffer. Then the beginning of the buffer is rewritten and
valid entries get corrupted.

Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
 kernel/printk/printk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index bc6eed48a454..a463707ca88f 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -319,7 +319,7 @@ static void log_store(int facility, int level,
 		else
 			free = log_first_idx - log_next_idx;
 
-		if (free > size + sizeof(struct printk_log))
+		if (free >= size + sizeof(struct printk_log))
 			break;
 
 		/* drop old messages until we have enough contiuous space */
@@ -327,7 +327,7 @@ static void log_store(int facility, int level,
 		log_first_seq++;
 	}
 
-	if (log_next_idx + size + sizeof(struct printk_log) >= log_buf_len) {
+	if (log_next_idx + size + sizeof(struct printk_log) > log_buf_len) {
 		/*
 		 * This message + an additional empty header does not fit
 		 * at the end of the buffer. Add an empty header with len == 0
-- 
1.8.4

[PATCH 5/5] printk: Do not compute the size of the message twice

[Petr Mladek]

This is just a tiny optimization. It removes duplicate computation of the
message size.

Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
 kernel/printk/printk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index a463707ca88f..9acbb9db1b43 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -351,7 +351,7 @@ static void log_store(int facility, int level,
 	else
 		msg->ts_nsec = local_clock();
 	memset(log_dict(msg) + dict_len, 0, pad_len);
-	msg->len = sizeof(struct printk_log) + text_len + dict_len + pad_len;
+	msg->len = size;
 
 	/* insert message */
 	log_next_idx += msg->len;
-- 
1.8.4