On Tue 2018-06-26 10:39:59, Steven Rostedt wrote: > On Tue, 26 Jun 2018 14:43:32 +0200 > Petr Mladek <[email protected]> wrote: > > > On Mon 2018-06-25 23:44:07, Sergey Senozhatsky wrote: > > > On (06/25/18 10:37), Steven Rostedt wrote: > > > > > > > > Is IMHO rather ugly. > > > > > > Either way works for me. So I'll leave it to you and Petr to decide :) > > > > > > > And the original patch has one more advantage. If buf and clear are > > > > both NULL/zero, we don't take any locks. > > > > > > But we never use syslog_print_all(buf = NULL, clear = false). It's either > > > NULL/true [move forward clear idx, do not copy to user], or !NULL/use > > > defined > > > value [copy to user, move or don't move clear idx forward] > > > > Yup, I suggest the following version as a compromise. It has the code > > duplication but I agree that it is negligible. Otherwise, it looks > > cleaner. > > > > This looks fine. For proper history though, what I do in this case, > would be to make this into two patches. One with Namit's original > patch, and then a second that removes the code duplication (your patch).
Makes sense. I have just pushed the original patch (with Sergey's formatting fixes) into printk.git, for-4.19 branch. And I propose the clean up patch, see below. I am sorry for the mess. It seems that nothing can be considered a simple obvious change. And my attempt to safe people time had the opposite effect. Proposed patch: From: Petr Mladek <[email protected]> Date: Wed, 27 Jun 2018 16:37:42 +0200 Subject: [PATCH] printk: Clean up syslog_print_all() syslog_print_all() is called twice. Once with a valid buffer and once just to set the indexes. Both variants are already handled separately. This patch just makes it more obvious. It does not change the existing behavior. Signed-off-by: Petr Mladek <[email protected]> --- kernel/printk/printk.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 16b02cc51a14..fcc1992c040a 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -1356,16 +1356,6 @@ static int syslog_print_all(char __user *buf, int size, bool clear) u64 seq; u32 idx; - if (!buf) { - if (clear) { - logbuf_lock_irq(); - clear_seq = log_next_seq; - clear_idx = log_next_idx; - logbuf_unlock_irq(); - } - return 0; - } - text = kmalloc(LOG_LINE_MAX + PREFIX_MAX, GFP_KERNEL); if (!text) return -ENOMEM; @@ -1437,6 +1427,14 @@ static int syslog_print_all(char __user *buf, int size, bool clear) return len; } +static void syslog_clear(void) +{ + logbuf_lock_irq(); + clear_seq = log_next_seq; + clear_idx = log_next_idx; + logbuf_unlock_irq(); +} + int do_syslog(int type, char __user *buf, int len, int source) { bool clear = false; @@ -1481,7 +1479,7 @@ int do_syslog(int type, char __user *buf, int len, int source) break; /* Clear ring buffer */ case SYSLOG_ACTION_CLEAR: - syslog_print_all(NULL, 0, true); + syslog_clear(); break; /* Disable logging to console */ case SYSLOG_ACTION_CONSOLE_OFF: -- 2.13.7
