[v3] reduce console_lock scope

[PATCH printk v3 09/40] um: kmsg_dump: use console_is_enabled()

Posted by John Ogness 3 years, 5 months ago

Replace (console->flags & CON_ENABLED) usage with console_is_enabled().

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 arch/um/kernel/kmsg_dump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
index 40abf1e9ccb1..f0233e2f8de0 100644
--- a/arch/um/kernel/kmsg_dump.c
+++ b/arch/um/kernel/kmsg_dump.c
@@ -32,7 +32,7 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
 		 * expected to output the crash information.
 		 */
 		if (strcmp(con->name, "ttynull") != 0 &&
-		    (con->flags & CON_ENABLED)) {
+		    console_is_enabled(con)) {
 			break;
 		}
 	}
-- 
2.30.2

Re: [PATCH printk v3 09/40] um: kmsg_dump: use console_is_enabled()

Posted by Petr Mladek 3 years, 5 months ago

On Mon 2022-11-07 15:22:07, John Ogness wrote:
> Replace (console->flags & CON_ENABLED) usage with console_is_enabled().
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
>  arch/um/kernel/kmsg_dump.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
> index 40abf1e9ccb1..f0233e2f8de0 100644
> --- a/arch/um/kernel/kmsg_dump.c
> +++ b/arch/um/kernel/kmsg_dump.c
> @@ -32,7 +32,7 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
>  		 * expected to output the crash information.
>  		 */
>  		if (strcmp(con->name, "ttynull") != 0 &&
> -		    (con->flags & CON_ENABLED)) {
> +		    console_is_enabled(con)) {

Same as with the 9th patch. I would merge this with the patch
switching to the srcu console list iterator. It will explain
why the racy check is needed here. This change does not make
sense without the other.

Best Regards,
Petr

Re: [PATCH printk v3 09/40] um: kmsg_dump: use console_is_enabled()

Posted by Petr Mladek 3 years, 5 months ago

On Wed 2022-11-09 15:56:11, Petr Mladek wrote:
> On Mon 2022-11-07 15:22:07, John Ogness wrote:
> > Replace (console->flags & CON_ENABLED) usage with console_is_enabled().
> > 
> > Signed-off-by: John Ogness <john.ogness@linutronix.de>
> > ---
> >  arch/um/kernel/kmsg_dump.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
> > index 40abf1e9ccb1..f0233e2f8de0 100644
> > --- a/arch/um/kernel/kmsg_dump.c
> > +++ b/arch/um/kernel/kmsg_dump.c
> > @@ -32,7 +32,7 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
> >  		 * expected to output the crash information.
> >  		 */
> >  		if (strcmp(con->name, "ttynull") != 0 &&
> > -		    (con->flags & CON_ENABLED)) {
> > +		    console_is_enabled(con)) {
> 
> Same as with the 9th patch. I would merge this with the patch
> switching to the srcu console list iterator. It will explain
> why the racy check is needed here. This change does not make
> sense without the other.

Ah, this was supposed to be for the 10th patch.

I am sorry for confusion.

Best Regards,
Petr

Re: [PATCH printk v3 09/40] um: kmsg_dump: use console_is_enabled()

Posted by Petr Mladek 3 years, 5 months ago

On Mon 2022-11-07 15:22:07, John Ogness wrote:
> Replace (console->flags & CON_ENABLED) usage with console_is_enabled().
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
>  arch/um/kernel/kmsg_dump.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
> index 40abf1e9ccb1..f0233e2f8de0 100644
> --- a/arch/um/kernel/kmsg_dump.c
> +++ b/arch/um/kernel/kmsg_dump.c
> @@ -32,7 +32,7 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
>  		 * expected to output the crash information.
>  		 */
>  		if (strcmp(con->name, "ttynull") != 0 &&
> -		    (con->flags & CON_ENABLED)) {
> +		    console_is_enabled(con)) {

The patch does not explain why the racy check is needed here.

I would prefer to merge this patch with the patch switching to the srcu
console list iterator. It will become more or less self-explaining.

Anyway, I agree that the racy check is acceptable here.

Best Regards,
Petr