Re: consoles: was: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-08 Thread Sergey Senozhatsky
On (20/12/07 10:50), Petr Mladek wrote: [..] > > and then some call into the scheduler > > (or other kernel core functions) under semaphore's spin_lock. > > For instance > > > > up() > > raw_spin_lock_irqsave(&sem->lock) > >__up() > > wake_up_process() > >try_

Re: vprintk_store: was: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-07 Thread Petr Mladek
On Sun 2020-12-06 23:36:53, John Ogness wrote: > On 2020-12-04, Petr Mladek wrote: > >> + if (facility == 0) { > >> + while (text_len >= 2 && printk_get_level(text)) { > >> + text_len -= 2; > >> + text += 2; > >> + } > > > > We should avoid two

Re: recursion handling: Re: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-07 Thread Petr Mladek
On Sun 2020-12-06 22:50:54, John Ogness wrote: > On 2020-12-04, Petr Mladek wrote: > > On Tue 2020-12-01 21:59:41, John Ogness wrote: > >> Since the ringbuffer is lockless, there is no need for it to be > >> protected by @logbuf_lock. Remove @logbuf_lock. > >> > >> --- a/kernel/printk/printk.c >

Re: syslog: was: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-07 Thread Petr Mladek
On Sun 2020-12-06 22:12:21, John Ogness wrote: > On 2020-12-04, Petr Mladek wrote: > > On Tue 2020-12-01 21:59:41, John Ogness wrote: > >> Since the ringbuffer is lockless, there is no need for it to be > >> protected by @logbuf_lock. Remove @logbuf_lock. > >> > >> --- a/kernel/printk/printk.c >

Re: devkmsg: was [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-07 Thread Petr Mladek
On Sun 2020-12-06 21:57:46, John Ogness wrote: > On 2020-12-04, Petr Mladek wrote: > >> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > >> index e9018c4e1b66..7385101210be 100644 > >> --- a/kernel/printk/printk.c > >> +++ b/kernel/printk/printk.c > >> @@ -785,7 +749,6 @@ static loff

Re: consoles: was: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-07 Thread Petr Mladek
On Sat 2020-12-05 13:39:53, Sergey Senozhatsky wrote: > On (20/12/04 17:19), Petr Mladek wrote: > [..] > > > --- a/kernel/printk/printk.c > > > +++ b/kernel/printk/printk.c > > > @@ -2432,7 +2490,6 @@ void console_unlock(void) > > > size_t len; > > > > > > printk_safe_enter_ir

Re: vprintk_store: was: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-06 Thread John Ogness
On 2020-12-04, Petr Mladek wrote: >> +/* Strip kernel syslog prefix. */ > > Syslog actually uses: format. We are skipping log level > and control flags here. OK. >> +if (facility == 0) { >> +while (text_len >= 2 && printk_get_level(text)) { >> +text_len -

Re: recursion handling: Re: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-06 Thread John Ogness
On 2020-12-05, Sergey Senozhatsky wrote: >> One reason is the use of per-cpu variables. Alternative solution would >> be to store printk_context into task_struct. > > We can keep per-CPU, disable preemption and have counters for > every context (task, soft/hard irq, NMI). Shouldn't be a problem T

Re: recursion handling: Re: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-06 Thread John Ogness
On 2020-12-05, Sergey Senozhatsky wrote: >> We should allow at least some level of recursion. Otherwise, we would >> not see warnings printed from vsprintf code. > > One level of recursion seems reasonable on one hand, but on the other > hand I also wonder if we can get useful info from recursion

Re: recursion handling: Re: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-06 Thread John Ogness
On 2020-12-04, Petr Mladek wrote: > On Tue 2020-12-01 21:59:41, John Ogness wrote: >> Since the ringbuffer is lockless, there is no need for it to be >> protected by @logbuf_lock. Remove @logbuf_lock. >> >> --- a/kernel/printk/printk.c >> +++ b/kernel/printk/printk.c >> @@ -1847,6 +1811,65 @@ sta

Re: syslog: was: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-06 Thread John Ogness
On 2020-12-04, Petr Mladek wrote: > On Tue 2020-12-01 21:59:41, John Ogness wrote: >> Since the ringbuffer is lockless, there is no need for it to be >> protected by @logbuf_lock. Remove @logbuf_lock. >> >> --- a/kernel/printk/printk.c >> +++ b/kernel/printk/printk.c >> @@ -1490,19 +1444,30 @@ st

Re: devkmsg: was [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-06 Thread John Ogness
On 2020-12-04, Petr Mladek wrote: >> Since the ringbuffer is lockless, there is no need for it to be >> protected by @logbuf_lock. Remove @logbuf_lock. > > It might make sense to split also this patch into few more pieces that > would remove the lock from a particular interface. OK. >> diff --gi

Re: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-06 Thread John Ogness
On 2020-12-04, Sergey Senozhatsky wrote: >> +static char *get_printk_count(void) > > A nit: I think get_foo() has some sort of special meaning and one > would expect that there should be put_foo() as well, and that those > have something to do with the ref counting. OK. How about: static char *p

Re: recursion handling: Re: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-05 Thread Sergey Senozhatsky
On (20/12/04 17:10), Petr Mladek wrote: > > One reason is the use of per-cpu variables. Alternative solution would > be to store printk_context into task_struct. We can keep per-CPU, disable preemption and have counters for every context (task, soft/hard irq, NMI). Shouldn't be a problem

Re: consoles: was: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-04 Thread Sergey Senozhatsky
On (20/12/04 17:19), Petr Mladek wrote: [..] > > --- a/kernel/printk/printk.c > > +++ b/kernel/printk/printk.c > > @@ -2432,7 +2490,6 @@ void console_unlock(void) > > size_t len; > > > > printk_safe_enter_irqsave(flags); > > Why do we actually need to use the printk_safe

Re: recursion handling: Re: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-04 Thread Sergey Senozhatsky
On (20/12/04 17:10), Petr Mladek wrote: [..] > char *get_printk_counter_by_ctx() > { > int ctx = 0; > > if (in_nmi) > ctx = 1; > > if (!printk_percpu_data_ready()) > return &printk_count_early[ctx]; > > return this_cpu_ptr(printk_count[ctx]); >

consoles: was: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-04 Thread Petr Mladek
On Tue 2020-12-01 21:59:41, John Ogness wrote: > Since the ringbuffer is lockless, there is no need for it to be > protected by @logbuf_lock. Remove @logbuf_lock. > > --- a/kernel/printk/internal.h > +++ b/kernel/printk/internal.h > @@ -59,7 +57,7 @@ void defer_console_output(void); > __printf(1,

vprintk_store: was: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-04 Thread Petr Mladek
On Tue 2020-12-01 21:59:41, John Ogness wrote: > Since the ringbuffer is lockless, there is no need for it to be > protected by @logbuf_lock. Remove @logbuf_lock. > > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -1867,40 +1890,75 @@ static inline u32 printk_caller_id(void) >

recursion handling: Re: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-04 Thread Petr Mladek
On Tue 2020-12-01 21:59:41, John Ogness wrote: > Since the ringbuffer is lockless, there is no need for it to be > protected by @logbuf_lock. Remove @logbuf_lock. > > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -1847,6 +1811,65 @@ static void call_console_drivers(const char *e

syslog: was: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-04 Thread Petr Mladek
On Tue 2020-12-01 21:59:41, John Ogness wrote: > Since the ringbuffer is lockless, there is no need for it to be > protected by @logbuf_lock. Remove @logbuf_lock. > > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -1490,19 +1444,30 @@ static int syslog_print_all(char __user *buf,

devkmsg: was [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-04 Thread Petr Mladek
On Tue 2020-12-01 21:59:41, John Ogness wrote: > Since the ringbuffer is lockless, there is no need for it to be > protected by @logbuf_lock. Remove @logbuf_lock. I am going to split the feedback into few mails. It might make sense to split also this patch into few more pieces that would remove th

Re: [PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-03 Thread Sergey Senozhatsky
On (20/12/01 21:59), John Ogness wrote: > > +#ifdef CONFIG_PRINTK_NMI > +#define NUM_RECURSION_CTX 2 > +#else > +#define NUM_RECURSION_CTX 1 > +#endif > + > +struct printk_recursion { > + charcount[NUM_RECURSION_CTX]; > +}; > + > +static DEFINE_PER_CPU(struct printk_recursion, percpu_prin

[PATCH next v2 3/3] printk: remove logbuf_lock, add syslog_lock

2020-12-01 Thread John Ogness
Since the ringbuffer is lockless, there is no need for it to be protected by @logbuf_lock. Remove @logbuf_lock. This means that printk_nmi_direct and printk_safe_flush_on_panic() no longer need to acquire any lock to run. The global variables @syslog_seq, @syslog_partial, @syslog_time were also p