Re: [RFC PATCH 2/5] sysrq: use pr_cont_t for cont messages

2020-08-20 Thread Joe Perches
On Fri, 2020-08-21 at 00:17 +0206, John Ogness wrote:
> On 2020-08-20, Joe Perches  wrote:
> > And here it seems like the 'for (j =...)' loop is superfluous.
> 
> AFAICT it is skipping duplicate entries. In the case of a duplicate,
> only the first one is printed.

Ah, right.  thanks.




Re: [RFC PATCH 2/5] sysrq: use pr_cont_t for cont messages

2020-08-20 Thread John Ogness
On 2020-08-20, Joe Perches  wrote:
> And here it seems like the 'for (j =...)' loop is superfluous.

AFAICT it is skipping duplicate entries. In the case of a duplicate,
only the first one is printed.

> Maybe something like this would be reasonable:
> ---
>  drivers/tty/sysrq.c | 19 ++-
>  1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index a8e39b2cdd55..a145e4fc1a2a 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -572,21 +572,14 @@ void __handle_sysrq(int key, bool check_mask)
>   console_loglevel = orig_log_level;
>   }
>   } else {
> - pr_info("HELP : ");
> - /* Only print the help msg once per handler */
> + pr_context c;
> + pr_info_start(, "HELP :");
>   for (i = 0; i < ARRAY_SIZE(sysrq_key_table); i++) {
> - if (sysrq_key_table[i]) {
> - int j;
> -
> - for (j = 0; sysrq_key_table[i] !=
> - sysrq_key_table[j]; j++)
> - ;
> - if (j != i)
> - continue;
> - pr_cont("%s ", sysrq_key_table[i]->help_msg);
> - }
> + if (!sysrq_key_table[i])
> + continue;
> + pr_next(, " %s", sysrq_key_table[i]->help_msg);
>   }
> - pr_cont("\n");
> + pr_end(, "\n");
>   console_loglevel = orig_log_level;
>   }
>   rcu_read_unlock();


Re: [RFC PATCH 2/5] sysrq: use pr_cont_t for cont messages

2020-08-20 Thread Linus Torvalds
On Thu, Aug 20, 2020 at 10:48 AM Joe Perches  wrote:
>
> Maybe something like this would be reasonable:

Yes. At this point this improves the code, rather than making it less legible.

  Linus


Re: [RFC PATCH 2/5] sysrq: use pr_cont_t for cont messages

2020-08-20 Thread Joe Perches
On Wed, 2020-08-19 at 18:03 -0700, Linus Torvalds wrote:
> On Wed, Aug 19, 2020 at 4:26 PM John Ogness  wrote:
> > Use the new pr_cont_t mechanism.
> 
> This looks actively much worse than the old code.

Isn't this just a generic mechanism to simplify
the accumulation of
logging message chunks?

It does seem straightforward enough to me.

And here it seems like the 'for (j =...)' loop is superfluous.

Maybe something like this would be reasonable:
---
 drivers/tty/sysrq.c | 19 ++-
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index a8e39b2cdd55..a145e4fc1a2a 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -572,21 +572,14 @@ void __handle_sysrq(int key, bool check_mask)
console_loglevel = orig_log_level;
}
} else {
-   pr_info("HELP : ");
-   /* Only print the help msg once per handler */
+   pr_context c;
+   pr_info_start(, "HELP :");
for (i = 0; i < ARRAY_SIZE(sysrq_key_table); i++) {
-   if (sysrq_key_table[i]) {
-   int j;
-
-   for (j = 0; sysrq_key_table[i] !=
-   sysrq_key_table[j]; j++)
-   ;
-   if (j != i)
-   continue;
-   pr_cont("%s ", sysrq_key_table[i]->help_msg);
-   }
+   if (!sysrq_key_table[i])
+   continue;
+   pr_next(, " %s", sysrq_key_table[i]->help_msg);
}
-   pr_cont("\n");
+   pr_end(, "\n");
console_loglevel = orig_log_level;
}
rcu_read_unlock();




Re: [RFC PATCH 2/5] sysrq: use pr_cont_t for cont messages

2020-08-19 Thread Linus Torvalds
On Wed, Aug 19, 2020 at 4:26 PM John Ogness  wrote:
>
> Use the new pr_cont_t mechanism.

This looks actively much worse than the old code.

Don't do this.

Just make pr_cont() do what it used to do.

 Linus


[RFC PATCH 2/5] sysrq: use pr_cont_t for cont messages

2020-08-19 Thread John Ogness
Use the new pr_cont_t mechanism.

Signed-off-by: John Ogness 
---
 drivers/tty/sysrq.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index a8e39b2cdd55..be2aa816c444 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -572,7 +572,9 @@ void __handle_sysrq(int key, bool check_mask)
console_loglevel = orig_log_level;
}
} else {
-   pr_info("HELP : ");
+   pr_cont_t c;
+
+   pr_cont_begin(, KERN_INFO "HELP : ");
/* Only print the help msg once per handler */
for (i = 0; i < ARRAY_SIZE(sysrq_key_table); i++) {
if (sysrq_key_table[i]) {
@@ -583,10 +585,10 @@ void __handle_sysrq(int key, bool check_mask)
;
if (j != i)
continue;
-   pr_cont("%s ", sysrq_key_table[i]->help_msg);
+   pr_cont_append(, "%s ", 
sysrq_key_table[i]->help_msg);
}
}
-   pr_cont("\n");
+   pr_cont_end();
console_loglevel = orig_log_level;
}
rcu_read_unlock();
-- 
2.20.1