From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joe Perches Subject: Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray Date: Thu, 14 Dec 2017 10:23:42 -0800 Message-ID: <1513275822.27409.73.camel@perches.com> References: <20171208223654.GP5858@dastard> <1512838818.26342.7.camel@perches.com> <20171211214300.GT5858@dastard> <1513030348.3036.5.camel@perches.com> <20171211224301.GA3925@bombadil.infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20171211224301.GA3925@bombadil.infradead.org> Sender: owner-linux-mm@kvack.org To: Matthew Wilcox Cc: Dave Chinner , Alan Stern , Byungchul Park , Theodore Ts'o , Matthew Wilcox , Ross Zwisler , Jens Axboe , Rehas Sachdeva , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-nilfs@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-xfs@vger.kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@lge.com List-Id: linux-f2fs-devel.lists.sourceforge.net On Mon, 2017-12-11 at 14:43 -0800, Matthew Wilcox wrote: > - There's no warning for the first paragraph of section 6: > > 6) Functions > ------------ > > Functions should be short and sweet, and do just one thing. They should > fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, > as we all know), and do one thing and do that well. > > I'm not expecting you to be able to write a perl script that checks > the first line, but we have way too many 200-plus line functions in > the kernel. I'd like a warning on anything over 200 lines (a factor > of 4 over Linus's stated goal). Perhaps something like this? (very very lightly tested, more testing appreciated) --- scripts/checkpatch.pl | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 4306b7616cdd..523aead40b87 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -59,6 +59,7 @@ my $conststructsfile = "$D/const_structs.checkpatch"; my $typedefsfile = ""; my $color = "auto"; my $allow_c99_comments = 1; +my $max_function_length = 200; sub help { my ($exitcode) = @_; @@ -2202,6 +2203,7 @@ sub process { my $realcnt = 0; my $here = ''; my $context_function; #undef'd unless there's a known function + my $context_function_linenum; my $in_comment = 0; my $comment_edge = 0; my $first_line = 0; @@ -2341,6 +2343,7 @@ sub process { } else { undef $context_function; } + undef $context_function_linenum; next; # track the line number as we move through the hunk, note that @@ -3200,11 +3203,18 @@ sub process { if ($sline =~ /^\+\{\s*$/ && $prevline =~ /^\+(?:(?:(?:$Storage|$Inline)\s*)*\s*$Type\s*)?($Ident)\(/) { $context_function = $1; + $context_function_linenum = $realline; } # check if this appears to be the end of function declaration if ($sline =~ /^\+\}\s*$/) { + if (defined($context_function_linenum) && + ($realline - $context_function_linenum) > $max_function_length) { + WARN("LONG_FUNCTION", + "'$context_function' function definition is " . ($realline - $context_function_linenum) . " lines, perhaps refactor\n" . $herecurr); + } undef $context_function; + undef $context_function_linenum; } # check indentation of any line with a bare else @@ -5983,6 +5993,7 @@ sub process { defined $stat && $stat =~ /^.\s*(?:$Storage\s+)?$Type\s*($Ident)\s*$balanced_parens\s*{/s) { $context_function = $1; + $context_function_linenum = $realline; # check for multiline function definition with misplaced open brace my $ok = 0; -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [v4,72/73] xfs: Convert mru cache to XArray From: Joe Perches Message-Id: <1513275822.27409.73.camel@perches.com> Date: Thu, 14 Dec 2017 10:23:42 -0800 To: Matthew Wilcox Cc: Dave Chinner , Alan Stern , Byungchul Park , Theodore Ts'o , Matthew Wilcox , Ross Zwisler , Jens Axboe , Rehas Sachdeva , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-nilfs@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-xfs@vger.kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@lge.com List-ID: T24gTW9uLCAyMDE3LTEyLTExIGF0IDE0OjQzIC0wODAwLCBNYXR0aGV3IFdpbGNveCB3cm90ZToK PiAgLSBUaGVyZSdzIG5vIHdhcm5pbmcgZm9yIHRoZSBmaXJzdCBwYXJhZ3JhcGggb2Ygc2VjdGlv biA2Ogo+IAo+IDYpIEZ1bmN0aW9ucwo+IC0tLS0tLS0tLS0tLQo+IAo+IEZ1bmN0aW9ucyBzaG91 bGQgYmUgc2hvcnQgYW5kIHN3ZWV0LCBhbmQgZG8ganVzdCBvbmUgdGhpbmcuICBUaGV5IHNob3Vs ZAo+IGZpdCBvbiBvbmUgb3IgdHdvIHNjcmVlbmZ1bHMgb2YgdGV4dCAodGhlIElTTy9BTlNJIHNj cmVlbiBzaXplIGlzIDgweDI0LAo+IGFzIHdlIGFsbCBrbm93KSwgYW5kIGRvIG9uZSB0aGluZyBh bmQgZG8gdGhhdCB3ZWxsLgo+IAo+ICAgIEknbSBub3QgZXhwZWN0aW5nIHlvdSB0byBiZSBhYmxl IHRvIHdyaXRlIGEgcGVybCBzY3JpcHQgdGhhdCBjaGVja3MKPiAgICB0aGUgZmlyc3QgbGluZSwg YnV0IHdlIGhhdmUgd2F5IHRvbyBtYW55IDIwMC1wbHVzIGxpbmUgZnVuY3Rpb25zIGluCj4gICAg dGhlIGtlcm5lbC4gIEknZCBsaWtlIGEgd2FybmluZyBvbiBhbnl0aGluZyBvdmVyIDIwMCBsaW5l cyAoYSBmYWN0b3IKPiAgICBvZiA0IG92ZXIgTGludXMncyBzdGF0ZWQgZ29hbCkuCgpQZXJoYXBz IHNvbWV0aGluZyBsaWtlIHRoaXM/CgoodmVyeSB2ZXJ5IGxpZ2h0bHkgdGVzdGVkLCBtb3JlIHRl c3RpbmcgYXBwcmVjaWF0ZWQpCi0tLQogc2NyaXB0cy9jaGVja3BhdGNoLnBsIHwgMTEgKysrKysr KysrKysKIDEgZmlsZSBjaGFuZ2VkLCAxMSBpbnNlcnRpb25zKCspCgoKLS0KVG8gdW5zdWJzY3Jp YmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LXVzYiIg aW4KdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcKTW9y ZSBtYWpvcmRvbW8gaW5mbyBhdCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5m by5odG1sCgpkaWZmIC0tZ2l0IGEvc2NyaXB0cy9jaGVja3BhdGNoLnBsIGIvc2NyaXB0cy9jaGVj a3BhdGNoLnBsCmluZGV4IDQzMDZiNzYxNmNkZC4uNTIzYWVhZDQwYjg3IDEwMDc1NQotLS0gYS9z Y3JpcHRzL2NoZWNrcGF0Y2gucGwKKysrIGIvc2NyaXB0cy9jaGVja3BhdGNoLnBsCkBAIC01OSw2 ICs1OSw3IEBAIG15ICRjb25zdHN0cnVjdHNmaWxlID0gIiREL2NvbnN0X3N0cnVjdHMuY2hlY2tw YXRjaCI7CiBteSAkdHlwZWRlZnNmaWxlID0gIiI7CiBteSAkY29sb3IgPSAiYXV0byI7CiBteSAk YWxsb3dfYzk5X2NvbW1lbnRzID0gMTsKK215ICRtYXhfZnVuY3Rpb25fbGVuZ3RoID0gMjAwOwog CiBzdWIgaGVscCB7CiAJbXkgKCRleGl0Y29kZSkgPSBAXzsKQEAgLTIyMDIsNiArMjIwMyw3IEBA IHN1YiBwcm9jZXNzIHsKIAlteSAkcmVhbGNudCA9IDA7CiAJbXkgJGhlcmUgPSAnJzsKIAlteSAk Y29udGV4dF9mdW5jdGlvbjsJCSN1bmRlZidkIHVubGVzcyB0aGVyZSdzIGEga25vd24gZnVuY3Rp b24KKwlteSAkY29udGV4dF9mdW5jdGlvbl9saW5lbnVtOwogCW15ICRpbl9jb21tZW50ID0gMDsK IAlteSAkY29tbWVudF9lZGdlID0gMDsKIAlteSAkZmlyc3RfbGluZSA9IDA7CkBAIC0yMzQxLDYg KzIzNDMsNyBAQCBzdWIgcHJvY2VzcyB7CiAJCQl9IGVsc2UgewogCQkJCXVuZGVmICRjb250ZXh0 X2Z1bmN0aW9uOwogCQkJfQorCQkJdW5kZWYgJGNvbnRleHRfZnVuY3Rpb25fbGluZW51bTsKIAkJ CW5leHQ7CiAKICMgdHJhY2sgdGhlIGxpbmUgbnVtYmVyIGFzIHdlIG1vdmUgdGhyb3VnaCB0aGUg aHVuaywgbm90ZSB0aGF0CkBAIC0zMjAwLDExICszMjAzLDE4IEBAIHN1YiBwcm9jZXNzIHsKIAkJ aWYgKCRzbGluZSA9fiAvXlwrXHtccyokLyAmJgogCQkgICAgJHByZXZsaW5lID1+IC9eXCsoPzoo PzooPzokU3RvcmFnZXwkSW5saW5lKVxzKikqXHMqJFR5cGVccyopPygkSWRlbnQpXCgvKSB7CiAJ CQkkY29udGV4dF9mdW5jdGlvbiA9ICQxOworCQkJJGNvbnRleHRfZnVuY3Rpb25fbGluZW51bSA9 ICRyZWFsbGluZTsKIAkJfQogCiAjIGNoZWNrIGlmIHRoaXMgYXBwZWFycyB0byBiZSB0aGUgZW5k IG9mIGZ1bmN0aW9uIGRlY2xhcmF0aW9uCiAJCWlmICgkc2xpbmUgPX4gL15cK1x9XHMqJC8pIHsK KwkJCWlmIChkZWZpbmVkKCRjb250ZXh0X2Z1bmN0aW9uX2xpbmVudW0pICYmCisJCQkgICAgKCRy ZWFsbGluZSAtICRjb250ZXh0X2Z1bmN0aW9uX2xpbmVudW0pID4gJG1heF9mdW5jdGlvbl9sZW5n dGgpIHsKKwkJCQlXQVJOKCJMT05HX0ZVTkNUSU9OIiwKKwkJCQkgICAgICInJGNvbnRleHRfZnVu Y3Rpb24nIGZ1bmN0aW9uIGRlZmluaXRpb24gaXMgIiAuICgkcmVhbGxpbmUgLSAkY29udGV4dF9m dW5jdGlvbl9saW5lbnVtKSAuICIgbGluZXMsIHBlcmhhcHMgcmVmYWN0b3JcbiIgLiAkaGVyZWN1 cnIpOworCQkJfQogCQkJdW5kZWYgJGNvbnRleHRfZnVuY3Rpb247CisJCQl1bmRlZiAkY29udGV4 dF9mdW5jdGlvbl9saW5lbnVtOwogCQl9CiAKICMgY2hlY2sgaW5kZW50YXRpb24gb2YgYW55IGxp bmUgd2l0aCBhIGJhcmUgZWxzZQpAQCAtNTk4Myw2ICs1OTkzLDcgQEAgc3ViIHByb2Nlc3Mgewog CQkgICAgZGVmaW5lZCAkc3RhdCAmJgogCQkgICAgJHN0YXQgPX4gL14uXHMqKD86JFN0b3JhZ2Vc cyspPyRUeXBlXHMqKCRJZGVudClccyokYmFsYW5jZWRfcGFyZW5zXHMqey9zKSB7CiAJCQkkY29u dGV4dF9mdW5jdGlvbiA9ICQxOworCQkJJGNvbnRleHRfZnVuY3Rpb25fbGluZW51bSA9ICRyZWFs bGluZTsKIAogIyBjaGVjayBmb3IgbXVsdGlsaW5lIGZ1bmN0aW9uIGRlZmluaXRpb24gd2l0aCBt aXNwbGFjZWQgb3BlbiBicmFjZQogCQkJbXkgJG9rID0gMDsK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtprelay0169.hostedemail.com ([216.40.44.169]:44917 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754027AbdLNSXt (ORCPT ); Thu, 14 Dec 2017 13:23:49 -0500 Message-ID: <1513275822.27409.73.camel@perches.com> Subject: Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray From: Joe Perches Date: Thu, 14 Dec 2017 10:23:42 -0800 In-Reply-To: <20171211224301.GA3925@bombadil.infradead.org> References: <20171208223654.GP5858@dastard> <1512838818.26342.7.camel@perches.com> <20171211214300.GT5858@dastard> <1513030348.3036.5.camel@perches.com> <20171211224301.GA3925@bombadil.infradead.org> Content-Type: text/plain; charset="ISO-8859-1" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Matthew Wilcox Cc: Dave Chinner , Alan Stern , Byungchul Park , Theodore Ts'o , Matthew Wilcox , Ross Zwisler , Jens Axboe , Rehas Sachdeva , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-nilfs@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-xfs@vger.kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@lge.com On Mon, 2017-12-11 at 14:43 -0800, Matthew Wilcox wrote: > - There's no warning for the first paragraph of section 6: > > 6) Functions > ------------ > > Functions should be short and sweet, and do just one thing. They should > fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, > as we all know), and do one thing and do that well. > > I'm not expecting you to be able to write a perl script that checks > the first line, but we have way too many 200-plus line functions in > the kernel. I'd like a warning on anything over 200 lines (a factor > of 4 over Linus's stated goal). Perhaps something like this? (very very lightly tested, more testing appreciated) --- scripts/checkpatch.pl | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 4306b7616cdd..523aead40b87 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -59,6 +59,7 @@ my $conststructsfile = "$D/const_structs.checkpatch"; my $typedefsfile = ""; my $color = "auto"; my $allow_c99_comments = 1; +my $max_function_length = 200; sub help { my ($exitcode) = @_; @@ -2202,6 +2203,7 @@ sub process { my $realcnt = 0; my $here = ''; my $context_function; #undef'd unless there's a known function + my $context_function_linenum; my $in_comment = 0; my $comment_edge = 0; my $first_line = 0; @@ -2341,6 +2343,7 @@ sub process { } else { undef $context_function; } + undef $context_function_linenum; next; # track the line number as we move through the hunk, note that @@ -3200,11 +3203,18 @@ sub process { if ($sline =~ /^\+\{\s*$/ && $prevline =~ /^\+(?:(?:(?:$Storage|$Inline)\s*)*\s*$Type\s*)?($Ident)\(/) { $context_function = $1; + $context_function_linenum = $realline; } # check if this appears to be the end of function declaration if ($sline =~ /^\+\}\s*$/) { + if (defined($context_function_linenum) && + ($realline - $context_function_linenum) > $max_function_length) { + WARN("LONG_FUNCTION", + "'$context_function' function definition is " . ($realline - $context_function_linenum) . " lines, perhaps refactor\n" . $herecurr); + } undef $context_function; + undef $context_function_linenum; } # check indentation of any line with a bare else @@ -5983,6 +5993,7 @@ sub process { defined $stat && $stat =~ /^.\s*(?:$Storage\s+)?$Type\s*($Ident)\s*$balanced_parens\s*{/s) { $context_function = $1; + $context_function_linenum = $realline; # check for multiline function definition with misplaced open brace my $ok = 0;