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

Groups > linux.kernel > #1636730 > unrolled thread

[PATCH 0/7] kernel-trace: Fine-tuning for seven function implementations

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] kernel-trace: Fine-tuning for seven function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2017-05-05 23:10 +0200
    [PATCH 6/7] kernel-trace: Adjust two checks for null pointers in  uprobe_buffer_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-05-05 23:10 +0200
    [PATCH 4/7] kernel-trace: Improve a size determination in  create_hist_field() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-05-05 23:10 +0200
    [PATCH 5/7] kernel-trace: Replace two seq_printf() calls by  seq_puts() in probes_seq_show() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-05-05 23:10 +0200
    Re: [PATCH 0/7] kernel-trace: Fine-tuning for seven function  implementations Steven Rostedt <rostedt@goodmis.org> - 2017-05-05 23:10 +0200
      Re: kernel-trace: Fine-tuning for seven function implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2017-05-05 23:30 +0200
    [PATCH 2/7] kernel-trace: Replace five seq_puts() calls by seq_putc() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-05-05 23:10 +0200
    [PATCH 6/7] kernel-trace: Adjust two checks for null pointers in  uprobe_buffer_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-05-05 23:10 +0200
    [PATCH 1/7] kernel-trace: Combine two function calls into one in  hist_trigger_entry_print() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-05-05 23:10 +0200
    [PATCH 3/7] kernel-trace: Adjust two checks for null pointers in  compatible_field() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-05-05 23:10 +0200
    [PATCH 7/7] kernel-trace: Delete an error message for a failed memory  allocation in create_trace_uprobe() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-05-05 23:20 +0200

#1636730 — [PATCH 0/7] kernel-trace: Fine-tuning for seven function implementations

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 5 May 2017 22:50:05 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (7):
  Combine two function calls into one in hist_trigger_entry_print()
  Replace five seq_puts() calls by seq_putc()
  Adjust two checks for null pointers in compatible_field()
  Improve a size determination in create_hist_field()
  Replace two seq_printf() calls by seq_puts() in probes_seq_show()
  Adjust two checks for null pointers in uprobe_buffer_init()
  Delete an error message for a failed memory allocation in create_trace_uprobe()

 kernel/trace/trace_events_hist.c | 18 ++++++++----------
 kernel/trace/trace_uprobe.c      | 10 ++++------
 2 files changed, 12 insertions(+), 16 deletions(-)

-- 
2.12.2

[toc] | [next] | [standalone]

#1636732 — [PATCH 6/7] kernel-trace: Adjust two checks for null pointers in uprobe_buffer_init()

Am 05.05.2017 um 23:00 schrieb SF Markus Elfring:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 5 May 2017 22:50:05 +0200
> 
> A few update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (7):
>   Combine two function calls into one in hist_trigger_entry_print()
>   Replace five seq_puts() calls by seq_putc()
>   Adjust two checks for null pointers in compatible_field()
>   Improve a size determination in create_hist_field()
>   Replace two seq_printf() calls by seq_puts() in probes_seq_show()
>   Adjust two checks for null pointers in uprobe_buffer_init()
>   Delete an error message for a failed memory allocation in create_trace_uprobe()
> 
>  kernel/trace/trace_events_hist.c | 18 ++++++++----------
>  kernel/trace/trace_uprobe.c      | 10 ++++------
>  2 files changed, 12 insertions(+), 16 deletions(-)
> 

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

#1636733 — [PATCH 4/7] kernel-trace: Improve a size determination in create_hist_field()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 5 May 2017 20:15:46 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 kernel/trace/trace_events_hist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 36412deac24c..a75223572374 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -353,7 +353,7 @@ static struct hist_field *create_hist_field(struct ftrace_event_field *field,
 	if (field && is_function_field(field))
 		return NULL;
 
-	hist_field = kzalloc(sizeof(struct hist_field), GFP_KERNEL);
+	hist_field = kzalloc(sizeof(*hist_field), GFP_KERNEL);
 	if (!hist_field)
 		return NULL;
 
-- 
2.12.2

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

#1636734 — [PATCH 5/7] kernel-trace: Replace two seq_printf() calls by seq_puts() in probes_seq_show()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 5 May 2017 20:30:05 +0200

Two strings which did not contain data format specifications should be put
into a sequence. Thus use the corresponding function "seq_puts".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 kernel/trace/trace_uprobe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index a7581fec9681..3e662d7ea6a4 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -612,11 +612,11 @@ static int probes_seq_show(struct seq_file *m, void *v)
 	} else {
 		switch (sizeof(void *)) {
 		case 4:
-			seq_printf(m, "0x00000000");
+			seq_puts(m, "0x00000000");
 			break;
 		case 8:
 		default:
-			seq_printf(m, "0x0000000000000000");
+			seq_puts(m, "0x0000000000000000");
 			break;
 		}
 	}
-- 
2.12.2

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

#1636736

On Fri, 5 May 2017 23:00:41 +0200
SF Markus Elfring <elfring@users.sourceforge.net> wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 5 May 2017 22:50:05 +0200
> 
> A few update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (7):

Hi Markus,

Just to let you know, it's never a good idea to send new patches out
during the merge window. They are most likely to be missed and
forgotten during this time. Unless they are critical bug fixes, it's
best to wait till after a merge window.

I wont be touching or even looking at these until after 4.12-rc1 is
released. Feel free to reply to this email with a ping in a week or two.

Thanks,

-- Steve

>   Combine two function calls into one in hist_trigger_entry_print()
>   Replace five seq_puts() calls by seq_putc()
>   Adjust two checks for null pointers in compatible_field()
>   Improve a size determination in create_hist_field()
>   Replace two seq_printf() calls by seq_puts() in probes_seq_show()
>   Adjust two checks for null pointers in uprobe_buffer_init()
>   Delete an error message for a failed memory allocation in create_trace_uprobe()
> 
>  kernel/trace/trace_events_hist.c | 18 ++++++++----------
>  kernel/trace/trace_uprobe.c      | 10 ++++------
>  2 files changed, 12 insertions(+), 16 deletions(-)
> 

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

#1636745 — Re: kernel-trace: Fine-tuning for seven function implementations

> Just to let you know, it's never a good idea to send new patches out
> during the merge window.

Thanks for such information.


> They are most likely to be missed and forgotten during this time.

This can happen then occasionally.


> Unless they are critical bug fixes, it's best to wait till after a merge window.

I am curious if the involved software developers will be interested at all
to consider update suggestions from this small patch series.


> I wont be touching or even looking at these until after 4.12-rc1 is released.

This is also fine.

How are the chances to pick related software development challenges up
when you find that the time will be better?


> Feel free to reply to this email with a ping in a week or two.

Would any source code reviewers like to send also a reminder then
besides looking into any waiting queues in other information systems?

Regards,
Markus

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

#1636739 — [PATCH 2/7] kernel-trace: Replace five seq_puts() calls by seq_putc()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 5 May 2017 19:45:12 +0200

Five single characters should be put into a sequence.
Thus use the corresponding function "seq_putc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 kernel/trace/trace_events_hist.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 94999934ffd5..df566e21344d 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1013,7 +1013,7 @@ hist_trigger_entry_print(struct seq_file *m,
 	}
 
 	if (!multiline)
-		seq_puts(m, " ");
+		seq_putc(m, ' ');
 
 	seq_printf(m, "} hitcount: %10llu",
 		   tracing_map_read_sum(elt, HITCOUNT_IDX));
@@ -1030,7 +1030,7 @@ hist_trigger_entry_print(struct seq_file *m,
 		}
 	}
 
-	seq_puts(m, "\n");
+	seq_putc(m, '\n');
 }
 
 static int print_entries(struct seq_file *m,
@@ -1168,7 +1168,7 @@ static int event_hist_trigger_print(struct seq_file *m,
 		key_field = hist_data->fields[i];
 
 		if (i > hist_data->n_vals)
-			seq_puts(m, ",");
+			seq_putc(m, ',');
 
 		if (key_field->flags & HIST_FIELD_FL_STACKTRACE)
 			seq_puts(m, "stacktrace");
@@ -1182,7 +1182,7 @@ static int event_hist_trigger_print(struct seq_file *m,
 		if (i == HITCOUNT_IDX)
 			seq_puts(m, "hitcount");
 		else {
-			seq_puts(m, ",");
+			seq_putc(m, ',');
 			hist_field_print(m, hist_data->fields[i]);
 		}
 	}
@@ -1195,7 +1195,7 @@ static int event_hist_trigger_print(struct seq_file *m,
 		sort_key = &hist_data->sort_keys[i];
 
 		if (i > 0)
-			seq_puts(m, ",");
+			seq_putc(m, ',');
 
 		if (sort_key->field_idx == HITCOUNT_IDX)
 			seq_puts(m, "hitcount");
-- 
2.12.2

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

#1636740 — [PATCH 6/7] kernel-trace: Adjust two checks for null pointers in uprobe_buffer_init()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 5 May 2017 22:30:16 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written !…

Thus fix the affected source code place.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 kernel/trace/trace_uprobe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 3e662d7ea6a4..e9c1865f7397 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -705,13 +705,13 @@ static int uprobe_buffer_init(void)
 	int cpu, err_cpu;
 
 	uprobe_cpu_buffer = alloc_percpu(struct uprobe_cpu_buffer);
-	if (uprobe_cpu_buffer == NULL)
+	if (!uprobe_cpu_buffer)
 		return -ENOMEM;
 
 	for_each_possible_cpu(cpu) {
 		struct page *p = alloc_pages_node(cpu_to_node(cpu),
 						  GFP_KERNEL, 0);
-		if (p == NULL) {
+		if (!p) {
 			err_cpu = cpu;
 			goto err;
 		}
-- 
2.12.2

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

#1636742 — [PATCH 1/7] kernel-trace: Combine two function calls into one in hist_trigger_entry_print()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 5 May 2017 19:40:39 +0200

A bit of data was put into a sequence by two separate function calls.
Print the same data by a single function call instead.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 kernel/trace/trace_events_hist.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 1c21d0e2a145..94999934ffd5 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1017,7 +1017,5 @@ hist_trigger_entry_print(struct seq_file *m,
 
-	seq_puts(m, "}");
-
-	seq_printf(m, " hitcount: %10llu",
+	seq_printf(m, "} hitcount: %10llu",
 		   tracing_map_read_sum(elt, HITCOUNT_IDX));
 
 	for (i = 1; i < hist_data->n_vals; i++) {
-- 
2.12.2

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

#1636743 — [PATCH 3/7] kernel-trace: Adjust two checks for null pointers in compatible_field()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 5 May 2017 20:00:11 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written !…

Thus fix the affected source code place.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 kernel/trace/trace_events_hist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index df566e21344d..36412deac24c 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1324,7 +1324,7 @@ static bool compatible_field(struct ftrace_event_field *field,
 {
 	if (field == test_field)
 		return true;
-	if (field == NULL || test_field == NULL)
+	if (!field || !test_field)
 		return false;
 	if (strcmp(field->name, test_field->name) != 0)
 		return false;
-- 
2.12.2

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

#1636744 — [PATCH 7/7] kernel-trace: Delete an error message for a failed memory allocation in create_trace_uprobe()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 5 May 2017 22:38:51 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: Possible unnecessary 'out of memory' message

Thus remove such a statement here.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 kernel/trace/trace_uprobe.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index e9c1865f7397..d19fa2a6cc7b 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -490,9 +490,7 @@ static int create_trace_uprobe(int argc, char **argv)
 	tu->offset = offset;
 	tu->inode = inode;
 	tu->filename = kstrdup(filename, GFP_KERNEL);
-
 	if (!tu->filename) {
-		pr_info("Failed to allocate filename.\n");
 		ret = -ENOMEM;
 		goto error;
 	}
-- 
2.12.2

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web