Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1683518 > unrolled thread

[PATCH] printk: Modify operators of printed_len

Back to article view | Back to linux.kernel

Contents

  [PATCH] printk: Modify operators of printed_len Pierre Kuo <vichy.kuo@gmail.com> - 2017-07-08 05:00 +0200
    Re: [PATCH] printk: Modify operators of printed_len Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-07-08 07:20 +0200
    Re: [PATCH] printk: Modify operators of printed_len Petr Mladek <pmladek@suse.com> - 2017-07-10 16:00 +0200
      Re: [PATCH] printk: Modify operators of printed_len pierre kuo <vichy.kuo@gmail.com> - 2017-07-11 06:10 +0200

#1683518 — [PATCH] printk: Modify operators of printed_len

In 8b1742c9c207, we remove printk-recursion detection code in
vprintk_emit(), where it is the first place that printed_len calculated.
After removing above detection, it seems we can directly assign the
result of log_output to printed_len.

Signed-off-by: Pierre Kuo <vichy.kuo@gmail.com>
---
 kernel/printk/printk.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index fc47863..a2a8cac 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1701,7 +1701,7 @@ asmlinkage int vprintk_emit(int facility, int level,
 	size_t text_len = 0;
 	enum log_flags lflags = 0;
 	unsigned long flags;
-	int printed_len = 0;
+	int printed_len;
 	bool in_sched = false;
 
 	if (level == LOGLEVEL_SCHED) {
@@ -1754,7 +1754,7 @@ asmlinkage int vprintk_emit(int facility, int level,
 	if (dict)
 		lflags |= LOG_PREFIX|LOG_NEWLINE;
 
-	printed_len += log_output(facility, level, lflags, dict, dictlen, text, text_len);
+	printed_len = log_output(facility, level, lflags, dict, dictlen, text, text_len);
 
 	logbuf_unlock_irqrestore(flags);
 
-- 
1.7.9.5

[toc] | [next] | [standalone]

#1683535

On (07/08/17 10:51), Pierre Kuo wrote:
> In 8b1742c9c207, we remove printk-recursion detection code in
> vprintk_emit(), where it is the first place that printed_len calculated.
> After removing above detection, it seems we can directly assign the
> result of log_output to printed_len.
> 
> Signed-off-by: Pierre Kuo <vichy.kuo@gmail.com>

Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

	-ss

> ---
>  kernel/printk/printk.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index fc47863..a2a8cac 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1701,7 +1701,7 @@ asmlinkage int vprintk_emit(int facility, int level,
>  	size_t text_len = 0;
>  	enum log_flags lflags = 0;
>  	unsigned long flags;
> -	int printed_len = 0;
> +	int printed_len;
>  	bool in_sched = false;
>  
>  	if (level == LOGLEVEL_SCHED) {
> @@ -1754,7 +1754,7 @@ asmlinkage int vprintk_emit(int facility, int level,
>  	if (dict)
>  		lflags |= LOG_PREFIX|LOG_NEWLINE;
>  
> -	printed_len += log_output(facility, level, lflags, dict, dictlen, text, text_len);
> +	printed_len = log_output(facility, level, lflags, dict, dictlen, text, text_len);
>  
>  	logbuf_unlock_irqrestore(flags);
>  
> -- 
> 1.7.9.5
> 

[toc] | [prev] | [next] | [standalone]

#1684269

On Sat 2017-07-08 10:51:13, Pierre Kuo wrote:
> In 8b1742c9c207, we remove printk-recursion detection code in
> vprintk_emit(), where it is the first place that printed_len calculated.
> After removing above detection, it seems we can directly assign the
> result of log_output to printed_len.
> 
> Signed-off-by: Pierre Kuo <vichy.kuo@gmail.com>

Great catch!

I just noticed that the same applies also to text_len
variable. Well, it was caused by another commit ddb9baa822265b55
("printk: report lost messages in printk safe/nmi contexts").
Could you please send a patch for this as well?

I would personally fix both variables in a single patch. But
I do not have a strong opinion about it.


This seems to be your first patch sent to the kernel mailing list.
Let me share some hints that might help you to handle more complex
patchsets ;-)

There is a standard format how to reference older commits. It is
'commit <12+ chars of sha1> ("<title line>")', see my comment above
for an example.

A good practice is to run ./scripts/checkpatch.pl <patch> before
you send the patch. Well, you need to use a common sense and ignore
false positives or hints that make a particular patch less readable
in the end.

Also it is handy to bump the version of the patch when it is
updated, e.g. use [PATCH v2] in the subject. People also
summarize changes against the previous version(s) below
the --- line. Well, this is more useful when there is a longer
delay between the versions and the changes are more complicated.

Best Regards,
Petr

[toc] | [prev] | [next] | [standalone]

#1684764

hi Petr
> I just noticed that the same applies also to text_len
> variable. Well, it was caused by another commit ddb9baa822265b55
> ("printk: report lost messages in printk safe/nmi contexts").
> Could you please send a patch for this as well?
sure and it is my pleasure.

>
> This seems to be your first patch sent to the kernel mailing list.
Yes :-)

> There is a standard format how to reference older commits. It is
> 'commit <12+ chars of sha1> ("<title line>")', see my comment above
> for an example.
>
> A good practice is to run ./scripts/checkpatch.pl <patch> before
> you send the patch. Well, you need to use a common sense and ignore
> false positives or hints that make a particular patch less readable
> in the end.
>
> Also it is handy to bump the version of the patch when it is
> updated, e.g. use [PATCH v2] in the subject. People also
> summarize changes against the previous version(s) below
> the --- line. Well, this is more useful when there is a longer
> delay between the versions and the changes are more complicated.
Really appreciate hints you provided and I will send the v2 patch soon.

Best Regards.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web