Re: [PATCH v3] tracepoint: add new `tcp:tcp_ca_event` trace event
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2023-08-13 01:01:49
Also in:
bpf, linux-trace-kernel, lkml
Subsystem:
checkpatch, the rest · Maintainers:
Andy Whitcroft, Joe Perches, Linus Torvalds
On Sat, 12 Aug 2023 20:59:05 -0400 Steven Rostedt [off-list ref] wrote:
On Sat, 12 Aug 2023 20:12:50 +0000 Zheao Li [off-list ref] wrote:quoted
+TRACE_EVENT(tcp_ca_event, + + TP_PROTO(struct sock *sk, const u8 ca_event), + + TP_ARGS(sk, ca_event), + + TP_STRUCT__entry( + __field(const void *, skaddr) + __field(__u16, sport) + __field(__u16, dport) + __field(__u16, family) + __array(__u8, saddr, 4) + __array(__u8, daddr, 4) + __array(__u8, saddr_v6, 16) + __array(__u8, daddr_v6, 16) + __field(__u8, ca_event)Please DO NOT LISTEN TO CHECKPATCH! The above looks horrendous! Put it back to:quoted
+ __field( const void *, skaddr ) + __field( __u16, sport ) + __field( __u16, dport ) + __field( __u16, family ) + __array( __u8, saddr, 4 ) + __array( __u8, daddr, 4 ) + __array( __u8, saddr_v6, 16 ) + __array( __u8, daddr_v6, 16 ) + __field( __u8, ca_event )See how much better it looks I can see fields this way. The "checkpatch" way is a condensed mess.
The below patch makes checkpatch not complain about some of this. But there's still more to do. -- Steve
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1e5e66ae5a52..24df11e8c861 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl@@ -73,6 +73,7 @@ my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANC my $git_command ='export LANGUAGE=en_US.UTF-8; git'; my $tabsize = 8; my ${CONFIG_} = "CONFIG_"; +my $trace_macros = "__array|__dynamic_array|__field|__string|EMe?"; sub help { my ($exitcode) = @_;
@@ -5387,7 +5388,8 @@ sub process { # check spacing on parentheses if ($line =~ /\(\s/ && $line !~ /\(\s*(?:\\)?$/ && - $line !~ /for\s*\(\s+;/) { + $line !~ /for\s*\(\s+;/ && + $line !~ m/$trace_macros/) { if (ERROR("SPACING", "space prohibited after that open parenthesis '('\n" . $herecurr) && $fix) {
@@ -5397,7 +5399,8 @@ sub process { } if ($line =~ /(\s+)\)/ && $line !~ /^.\s*\)/ && $line !~ /for\s*\(.*;\s+\)/ && - $line !~ /:\s+\)/) { + $line !~ /:\s+\)/ && + $line !~ m/$trace_macros/) { if (ERROR("SPACING", "space prohibited before that close parenthesis ')'\n" . $herecurr) && $fix) {
@@ -5906,6 +5909,7 @@ sub process { $dstat !~ /^for\s*$Constant$/ && # for (...) $dstat !~ /^for\s*$Constant\s+(?:$Ident|-?$Constant)$/ && # for (...) bar() $dstat !~ /^do\s*{/ && # do {... + $dstat !~ /^EMe?\s*1u/ && # EM( and EMe( are commonly used with TRACE_DEFINE_ENUM $dstat !~ /^\(\{/ && # ({... $ctx !~ /^.\s*#\s*define\s+TRACE_(?:SYSTEM|INCLUDE_FILE|INCLUDE_PATH)\b/) {
@@ -6017,7 +6021,8 @@ sub process { WARN("DO_WHILE_MACRO_WITH_TRAILING_SEMICOLON", "do {} while (0) macros should not be semicolon terminated\n" . "$herectx"); } - } elsif ($dstat =~ /^\+\s*#\s*define\s+$Ident.*;\s*$/) { + } elsif ($dstat =~ /^\+\s*#\s*define\s+$Ident.*;\s*$/ && + $dstat !~ /TRACE_DEFINE_ENUM\(/) { $ctx =~ s/\n*$//; my $cnt = statement_rawlines($ctx); my $herectx = get_stat_here($linenr, $cnt, $here);