Re: [PATCH] printk: modify console_unlock with printk-safe macros
hi Sergey and Andy: > On (07/15/17 18:36), Pierre Kuo wrote: > [..] >> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c >> index fc47863..21557cc 100644 >> --- a/kernel/printk/printk.c >> +++ b/kernel/printk/printk.c >> @@ -2194,8 +2194,7 @@ void console_unlock(void) >> size_t ext_len = 0; >> size_t len; >> >> - printk_safe_enter_irqsave(flags); >> - raw_spin_lock(_lock); >> + logbuf_lock_irqsave(flags); >> if (seen_seq != log_next_seq) { >> wake_klogd = true; >> seen_seq = log_next_seq; >> @@ -2267,8 +2266,7 @@ void console_unlock(void) >>*/ >> raw_spin_lock(_lock); >> retry = console_seq != log_next_seq; >> - raw_spin_unlock(_lock); >> - printk_safe_exit_irqrestore(flags); >> + logbuf_unlock_irqrestore(flags); >> >> if (retry && console_trylock()) >> goto again; > > I did it that particular way for a reason - console_unlock() does a > bunch of tricks: unlocking logbuf in the middle of printing loop, > breaking out of loop with local IRQs disabled, re-taking the logbuf > after the loop still will local IRQs disabled, etc. etc. I didn't > want to (and still don't) mix-in logbuf macros; we do things that > macros don't cover anyway. sorry, I don't agree that the patch > improves readability. Got ur points and appreciate for your illustration. ^^ Thanks a lot,
Re: [PATCH] printk: modify console_unlock with printk-safe macros
hi Sergey and Andy: > On (07/15/17 18:36), Pierre Kuo wrote: > [..] >> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c >> index fc47863..21557cc 100644 >> --- a/kernel/printk/printk.c >> +++ b/kernel/printk/printk.c >> @@ -2194,8 +2194,7 @@ void console_unlock(void) >> size_t ext_len = 0; >> size_t len; >> >> - printk_safe_enter_irqsave(flags); >> - raw_spin_lock(_lock); >> + logbuf_lock_irqsave(flags); >> if (seen_seq != log_next_seq) { >> wake_klogd = true; >> seen_seq = log_next_seq; >> @@ -2267,8 +2266,7 @@ void console_unlock(void) >>*/ >> raw_spin_lock(_lock); >> retry = console_seq != log_next_seq; >> - raw_spin_unlock(_lock); >> - printk_safe_exit_irqrestore(flags); >> + logbuf_unlock_irqrestore(flags); >> >> if (retry && console_trylock()) >> goto again; > > I did it that particular way for a reason - console_unlock() does a > bunch of tricks: unlocking logbuf in the middle of printing loop, > breaking out of loop with local IRQs disabled, re-taking the logbuf > after the loop still will local IRQs disabled, etc. etc. I didn't > want to (and still don't) mix-in logbuf macros; we do things that > macros don't cover anyway. sorry, I don't agree that the patch > improves readability. Got ur points and appreciate for your illustration. ^^ Thanks a lot,
Re: [PATCH] printk: modify console_unlock with printk-safe macros
On (07/15/17 18:36), Pierre Kuo wrote: [..] > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index fc47863..21557cc 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -2194,8 +2194,7 @@ void console_unlock(void) > size_t ext_len = 0; > size_t len; > > - printk_safe_enter_irqsave(flags); > - raw_spin_lock(_lock); > + logbuf_lock_irqsave(flags); > if (seen_seq != log_next_seq) { > wake_klogd = true; > seen_seq = log_next_seq; > @@ -2267,8 +2266,7 @@ void console_unlock(void) >*/ > raw_spin_lock(_lock); > retry = console_seq != log_next_seq; > - raw_spin_unlock(_lock); > - printk_safe_exit_irqrestore(flags); > + logbuf_unlock_irqrestore(flags); > > if (retry && console_trylock()) > goto again; I did it that particular way for a reason - console_unlock() does a bunch of tricks: unlocking logbuf in the middle of printing loop, breaking out of loop with local IRQs disabled, re-taking the logbuf after the loop still will local IRQs disabled, etc. etc. I didn't want to (and still don't) mix-in logbuf macros; we do things that macros don't cover anyway. sorry, I don't agree that the patch improves readability. -ss
Re: [PATCH] printk: modify console_unlock with printk-safe macros
On (07/15/17 18:36), Pierre Kuo wrote: [..] > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index fc47863..21557cc 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -2194,8 +2194,7 @@ void console_unlock(void) > size_t ext_len = 0; > size_t len; > > - printk_safe_enter_irqsave(flags); > - raw_spin_lock(_lock); > + logbuf_lock_irqsave(flags); > if (seen_seq != log_next_seq) { > wake_klogd = true; > seen_seq = log_next_seq; > @@ -2267,8 +2266,7 @@ void console_unlock(void) >*/ > raw_spin_lock(_lock); > retry = console_seq != log_next_seq; > - raw_spin_unlock(_lock); > - printk_safe_exit_irqrestore(flags); > + logbuf_unlock_irqrestore(flags); > > if (retry && console_trylock()) > goto again; I did it that particular way for a reason - console_unlock() does a bunch of tricks: unlocking logbuf in the middle of printing loop, breaking out of loop with local IRQs disabled, re-taking the logbuf after the loop still will local IRQs disabled, etc. etc. I didn't want to (and still don't) mix-in logbuf macros; we do things that macros don't cover anyway. sorry, I don't agree that the patch improves readability. -ss
Re: [PATCH] printk: modify console_unlock with printk-safe macros
On Sat, Jul 15, 2017 at 1:36 PM, Pierre Kuowrote: > In commit de6fcbdb68b2 ("printk: convert the rest to printk-safe"), we > create 4 helper macros to make printk-safe usage easier. > Here we modify some part of console_unlock with above marcros. > raw_spin_lock(_lock); ...and now we have "orphaned" call to lock. > - raw_spin_unlock(_lock); > + logbuf_unlock_irqrestore(flags); Personally I can't say this makes things clearer. -- With Best Regards, Andy Shevchenko
Re: [PATCH] printk: modify console_unlock with printk-safe macros
On Sat, Jul 15, 2017 at 1:36 PM, Pierre Kuo wrote: > In commit de6fcbdb68b2 ("printk: convert the rest to printk-safe"), we > create 4 helper macros to make printk-safe usage easier. > Here we modify some part of console_unlock with above marcros. > raw_spin_lock(_lock); ...and now we have "orphaned" call to lock. > - raw_spin_unlock(_lock); > + logbuf_unlock_irqrestore(flags); Personally I can't say this makes things clearer. -- With Best Regards, Andy Shevchenko
[PATCH] printk: modify console_unlock with printk-safe macros
From: pierre KuoIn commit de6fcbdb68b2 ("printk: convert the rest to printk-safe"), we create 4 helper macros to make printk-safe usage easier. Here we modify some part of console_unlock with above marcros. Signed-off-by: Pierre Kuo --- kernel/printk/printk.c |6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index fc47863..21557cc 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2194,8 +2194,7 @@ void console_unlock(void) size_t ext_len = 0; size_t len; - printk_safe_enter_irqsave(flags); - raw_spin_lock(_lock); + logbuf_lock_irqsave(flags); if (seen_seq != log_next_seq) { wake_klogd = true; seen_seq = log_next_seq; @@ -2267,8 +2266,7 @@ void console_unlock(void) */ raw_spin_lock(_lock); retry = console_seq != log_next_seq; - raw_spin_unlock(_lock); - printk_safe_exit_irqrestore(flags); + logbuf_unlock_irqrestore(flags); if (retry && console_trylock()) goto again; -- 1.7.9.5
[PATCH] printk: modify console_unlock with printk-safe macros
From: pierre Kuo In commit de6fcbdb68b2 ("printk: convert the rest to printk-safe"), we create 4 helper macros to make printk-safe usage easier. Here we modify some part of console_unlock with above marcros. Signed-off-by: Pierre Kuo --- kernel/printk/printk.c |6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index fc47863..21557cc 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2194,8 +2194,7 @@ void console_unlock(void) size_t ext_len = 0; size_t len; - printk_safe_enter_irqsave(flags); - raw_spin_lock(_lock); + logbuf_lock_irqsave(flags); if (seen_seq != log_next_seq) { wake_klogd = true; seen_seq = log_next_seq; @@ -2267,8 +2266,7 @@ void console_unlock(void) */ raw_spin_lock(_lock); retry = console_seq != log_next_seq; - raw_spin_unlock(_lock); - printk_safe_exit_irqrestore(flags); + logbuf_unlock_irqrestore(flags); if (retry && console_trylock()) goto again; -- 1.7.9.5