Messages in this thread | | | Date | Mon, 25 Jun 2018 10:37:04 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH] printk: remove unnecessary kmalloc() from syslog during clear |
| |
On Mon, 25 Jun 2018 15:37:05 +0200 Petr Mladek <pmladek@suse.com> wrote:
> On Wed 2018-06-20 19:26:19, Namit Gupta wrote: > > When the request is only for clearing logs, there is no need for > > allocation/deallocation. Only the indexes need to be reset and returned. > > Rest of the patch is mostly made up of changes because of indention. > > > > Signed-off-by: Namit Gupta <gupta.namit@samsung.com> > > Signed-off-by: Himanshu Maithani <himanshu.m@samsung.com> > > > --- > > kernel/printk/printk.c | 111 ++++++++++++++++++++++++++----------------------- > > 1 file changed, 60 insertions(+), 51 deletions(-) > > > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > > index 512f7c2..53952ce 100644 > > --- a/kernel/printk/printk.c > > +++ b/kernel/printk/printk.c > > @@ -1348,71 +1348,80 @@ static int syslog_print_all(char __user *buf, int size, bool clear) > > { > > char *text; > > int len = 0; > > + u64 next_seq; > > + u64 seq; > > + u32 idx; > > + > > + if (!buf) { > > + if (clear) { > > + logbuf_lock_irq(); > > + clear_seq = log_next_seq; > > + clear_idx = log_next_idx; > > + logbuf_unlock_irq(); > > I pushed a bit different version into printk.git, branch for-4.19, > see below. It removes the code duplication. Also it keeps the original > indentation. IMHO, it helped to better distinguish the code for printing > and clearing. > > It is rather a cosmetic change, so I do not want you to resend > Reviewed-by tags. But feel free to disagree and ask me to use > the original variant. >
I actually prefer the original version. It's not really duplicating much text, and it's cleaner, because it lets you know exactly what is happening when buf == NULL.
if (buf) text = kmalloc();
logbuf_lock_irq();
if (buf) { [...] }
Is IMHO rather ugly.
And the original patch has one more advantage. If buf and clear are both NULL/zero, we don't take any locks.
-- Steve
| |