* Re: [PATCH RFC] scripts: add a script to handle Documentation/features
From: Greg Kroah-Hartman @ 2019-06-17 18:11 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Jonathan Corbet, Mauro Carvalho Chehab,
linux-kernel, ksummit-discuss
In-Reply-To: <98ce589a7c50e2693ab6be158e03afde19aed81e.1560794401.git.mchehab+samsung@kernel.org>
On Mon, Jun 17, 2019 at 03:05:07PM -0300, Mauro Carvalho Chehab wrote:
> The Documentation/features contains a set of parseable files.
> It is not worth converting them to ReST format, as they're
> useful the way it is. It is, however, interesting to parse
> them and produce output on different formats:
>
> 1) Output the contents of a feature in ReST format;
>
> 2) Output what features a given architecture supports;
>
> 3) Output a matrix with features x architectures.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
>
> As commented at KS mailing list, converting the Documentation/features
> file to ReST may not be the best way to handle it.
>
> This script allows validating the features files and to generate ReST files
> on three different formats.
>
> The goal is to support it via a sphinx extension, in order to be able to add
> the features inside the Kernel documentation.
>
> scripts/get_feat.pl | 470 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 470 insertions(+)
> create mode 100755 scripts/get_feat.pl
>
> diff --git a/scripts/get_feat.pl b/scripts/get_feat.pl
> new file mode 100755
> index 000000000000..c5a267b12f49
> --- /dev/null
> +++ b/scripts/get_feat.pl
> @@ -0,0 +1,470 @@
> +#!/usr/bin/perl
> +
No SPDX line :(
^ permalink raw reply
* [PATCH RFC] scripts: add a script to handle Documentation/features
From: Mauro Carvalho Chehab @ 2019-06-17 18:05 UTC (permalink / raw)
To: Linux Doc Mailing List, Jonathan Corbet, Greg Kroah-Hartman
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
ksummit-discuss
In-Reply-To: <20190617142117.76478570@coco.lan>
The Documentation/features contains a set of parseable files.
It is not worth converting them to ReST format, as they're
useful the way it is. It is, however, interesting to parse
them and produce output on different formats:
1) Output the contents of a feature in ReST format;
2) Output what features a given architecture supports;
3) Output a matrix with features x architectures.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
As commented at KS mailing list, converting the Documentation/features
file to ReST may not be the best way to handle it.
This script allows validating the features files and to generate ReST files
on three different formats.
The goal is to support it via a sphinx extension, in order to be able to add
the features inside the Kernel documentation.
scripts/get_feat.pl | 470 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 470 insertions(+)
create mode 100755 scripts/get_feat.pl
diff --git a/scripts/get_feat.pl b/scripts/get_feat.pl
new file mode 100755
index 000000000000..c5a267b12f49
--- /dev/null
+++ b/scripts/get_feat.pl
@@ -0,0 +1,470 @@
+#!/usr/bin/perl
+
+use strict;
+use Pod::Usage;
+use Getopt::Long;
+use File::Find;
+use Fcntl ':mode';
+
+my $help;
+my $man;
+my $debug;
+my $arch;
+my $feat;
+my $prefix="Documentation/features";
+
+GetOptions(
+ "debug|d+" => \$debug,
+ 'help|?' => \$help,
+ 'arch=s' => \$arch,
+ 'feat=s' => \$feat,
+ man => \$man
+) or pod2usage(2);
+
+pod2usage(1) if $help;
+pod2usage(-exitstatus => 0, -verbose => 2) if $man;
+
+pod2usage(2) if (scalar @ARGV < 1 || @ARGV > 2);
+
+my ($cmd, $arg) = @ARGV;
+
+pod2usage(2) if ($cmd ne "current" && $cmd ne "rest" && $cmd ne "validate");
+
+require Data::Dumper if ($debug);
+
+my %data;
+my %archs;
+
+#
+# Displays an error message, printing file name and line
+#
+sub parse_error($$$$) {
+ my ($file, $ln, $msg, $data) = @_;
+
+ $data =~ s/\s+$/\n/;
+
+ print STDERR "Warning: file $file#$ln:\n\t$msg";
+
+ if ($data ne "") {
+ print STDERR ". Line\n\t\t$data";
+ } else {
+ print STDERR "\n";
+ }
+}
+
+#
+# Parse a features file, storing its contents at %data
+#
+
+my $h_name = "Feature";
+my $h_kconfig = "Kconfig";
+my $h_description = "Description";
+my $h_subsys = "Subsystem";
+my $h_status = "Status";
+my $h_arch = "Architecture";
+
+my $max_size_name = length($h_name);
+my $max_size_kconfig = length($h_kconfig);
+my $max_size_description = length($h_description);
+my $max_size_subsys = length($h_subsys);
+my $max_size_status = length($h_status);
+my $max_size_arch = length($h_arch);
+
+sub parse_feat {
+ my $file = $File::Find::name;
+
+ my $mode = (stat($file))[2];
+ return if ($mode & S_IFDIR);
+ return if ($file =~ m,($prefix)/arch-support.txt,);
+ return if (!($file =~ m,arch-support.txt$,));
+
+ my $subsys = "";
+ $subsys = $2 if ( m,.*($prefix)/([^/]+).*,);
+
+ if (length($subsys) > $max_size_subsys) {
+ $max_size_subsys = length($subsys);
+ }
+
+ my $name;
+ my $kconfig;
+ my $description;
+ my $comments = "";
+ my $last_status;
+ my $ln;
+ my %arch_table;
+
+ print STDERR "Opening $file\n" if ($debug > 1);
+ open IN, $file;
+
+ while(<IN>) {
+ $ln++;
+
+ if (m/^\#\s+Feature\s+name:\s*(.*\S)/) {
+ $name = $1;
+ if (length($name) > $max_size_name) {
+ $max_size_name = length($name);
+ }
+ next;
+ }
+ if (m/^\#\s+Kconfig:\s*(.*\S)/) {
+ $kconfig = $1;
+ if (length($kconfig) > $max_size_kconfig) {
+ $max_size_kconfig = length($kconfig);
+ }
+ next;
+ }
+ if (m/^\#\s+description:\s*(.*\S)/) {
+ $description = $1;
+ if (length($description) > $max_size_description) {
+ $max_size_description = length($description);
+ }
+ next;
+ }
+ next if (m/^\\s*$/);
+ next if (m/^\s*\-+\s*$/);
+ next if (m/^\s*\|\s*arch\s*\|\s*status\s*\|\s*$/);
+
+ if (m/^\#\s*(.*)/) {
+ $comments .= "$1\n";
+ next;
+ }
+ if (m/^\s*\|\s*(\S+):\s*\|\s*(\S+)\s*\|\s*$/) {
+ my $a = $1;
+ my $status = $2;
+
+ if (length($status) > $max_size_status) {
+ $max_size_status = length($status);
+ }
+ if (length($a) > $max_size_arch) {
+ $max_size_arch = length($a);
+ }
+
+ $archs{$a} = 1;
+ $arch_table{$a} = $status;
+ next;
+ }
+
+ #Everything else is an error
+ parse_error($file, $ln, "line is invalid", $_);
+ }
+ close IN;
+
+ if (!$name) {
+ parse_error($file, $ln, "Feature name not found", "");
+ return;
+ }
+
+ parse_error($file, $ln, "Subsystem not found", "") if (!$subsys);
+ parse_error($file, $ln, "Kconfig not found", "") if (!$kconfig);
+ parse_error($file, $ln, "Description not found", "") if (!$description);
+
+ if (!%arch_table) {
+ parse_error($file, $ln, "Architecture table not found", "");
+ return;
+ }
+
+ $data{$name}->{where} = $file;
+ $data{$name}->{subsys} = $subsys;
+ $data{$name}->{kconfig} = $kconfig;
+ $data{$name}->{description} = $description;
+ $data{$name}->{comments} = $comments;
+ $data{$name}->{table} = \%arch_table;
+}
+
+#
+# Output feature(s) for a given architecture
+#
+sub output_arch_table {
+ my $title = "Feature status on $arch architecture";
+
+ print "=" x length($title) . "\n";
+ print "$title\n";
+ print "=" x length($title) . "\n\n";
+
+ print "=" x $max_size_subsys;
+ print " ";
+ print "=" x $max_size_name;
+ print " ";
+ print "=" x $max_size_kconfig;
+ print " ";
+ print "=" x $max_size_status;
+ print " ";
+ print "=" x $max_size_description;
+ print "\n";
+ printf "%-${max_size_subsys}s ", $h_subsys;
+ printf "%-${max_size_name}s ", $h_name;
+ printf "%-${max_size_kconfig}s ", $h_kconfig;
+ printf "%-${max_size_status}s ", $h_status;
+ printf "%-${max_size_description}s\n", $h_description;
+ print "=" x $max_size_subsys;
+ print " ";
+ print "=" x $max_size_name;
+ print " ";
+ print "=" x $max_size_kconfig;
+ print " ";
+ print "=" x $max_size_status;
+ print " ";
+ print "=" x $max_size_description;
+ print "\n";
+
+ foreach my $name (sort {
+ ($data{$a}->{subsys} cmp $data{$b}->{subsys}) ||
+ ($data{$a}->{name} cmp $data{$b}->{name})
+ } keys %data) {
+ next if ($feat && $name ne $feat);
+
+ my %arch_table = %{$data{$name}->{table}};
+ printf "%-${max_size_subsys}s ", $data{$name}->{subsys};
+ printf "%-${max_size_name}s ", $name;
+ printf "%-${max_size_kconfig}s ", $data{$name}->{kconfig};
+ printf "%-${max_size_status}s ", $arch_table{$arch};
+ printf "%-${max_size_description}s\n", $data{$name}->{description};
+ }
+
+ print "=" x $max_size_subsys;
+ print " ";
+ print "=" x $max_size_name;
+ print " ";
+ print "=" x $max_size_kconfig;
+ print " ";
+ print "=" x $max_size_status;
+ print " ";
+ print "=" x $max_size_description;
+ print "\n";
+}
+
+#
+# Output a feature on all architectures
+#
+sub output_feature {
+ my $title = "Feature $feat";
+
+ print "=" x length($title) . "\n";
+ print "$title\n";
+ print "=" x length($title) . "\n\n";
+
+ print ":Subsystem: $data{$feat}->{subsys} \n" if ($data{$feat}->{subsys});
+ print ":Kconfig: $data{$feat}->{kconfig} \n" if ($data{$feat}->{kconfig});
+
+ my $desc = $data{$feat}->{description};
+ $desc =~ s/^([a-z])/\U$1/;
+ $desc =~ s/\.?\s*//;
+ print "\n$desc.\n\n";
+
+ my $com = $data{$feat}->{comments};
+ $com =~ s/^\s+//;
+ $com =~ s/\s+$//;
+ if ($com) {
+ print "Comments\n";
+ print "--------\n\n";
+ print "$com\n\n";
+ }
+
+ print "=" x $max_size_arch;
+ print " ";
+ print "=" x $max_size_status;
+ print "\n";
+
+ printf "%-${max_size_arch}s ", $h_arch;
+ printf "%-${max_size_status}s", $h_status . "\n";
+
+ print "=" x $max_size_arch;
+ print " ";
+ print "=" x $max_size_status;
+ print "\n";
+
+ my %arch_table = %{$data{$feat}->{table}};
+ foreach my $arch (sort keys %arch_table) {
+ printf "%-${max_size_arch}s ", $arch;
+ printf "%-${max_size_status}s\n", $arch_table{$arch};
+ }
+
+ print "=" x $max_size_arch;
+ print " ";
+ print "=" x $max_size_status;
+ print "\n";
+}
+
+#
+# Output all features for all architectures
+#
+
+sub matrix_lines {
+ print "=" x $max_size_subsys;
+ print " ";
+ print "=" x $max_size_name;
+ print " ";
+
+ foreach my $arch (sort keys %archs) {
+ my $len = $max_size_status;
+
+ $len = length($arch) if ($len < length($arch));
+
+ print "=" x $len;
+ print " ";
+ }
+ print "=" x $max_size_kconfig;
+ print " ";
+ print "=" x $max_size_description;
+ print "\n";
+}
+
+sub output_matrix {
+
+ my $title = "Feature List (feature x architecture)";
+
+ print "=" x length($title) . "\n";
+ print "$title\n";
+ print "=" x length($title) . "\n\n";
+
+ matrix_lines;
+
+ printf "%-${max_size_subsys}s ", $h_subsys;
+ printf "%-${max_size_name}s ", $h_name;
+
+ foreach my $arch (sort keys %archs) {
+ printf "%-${max_size_status}s ", $arch;
+ }
+ printf "%-${max_size_kconfig}s ", $h_kconfig;
+ printf "%-${max_size_description}s\n", $h_description;
+
+ matrix_lines;
+
+ foreach my $name (sort {
+ ($data{$a}->{subsys} cmp $data{$b}->{subsys}) ||
+ ($data{$a}->{name} cmp $data{$b}->{name})
+ } keys %data) {
+ printf "%-${max_size_subsys}s ", $data{$name}->{subsys};
+ printf "%-${max_size_name}s ", $name;
+
+ my %arch_table = %{$data{$name}->{table}};
+
+ foreach my $arch (sort keys %arch_table) {
+ my $len = $max_size_status;
+
+ $len = length($arch) if ($len < length($arch));
+
+ printf "%-${len}s ", $arch_table{$arch};
+ }
+ printf "%-${max_size_kconfig}s ", $data{$name}->{kconfig};
+ printf "%-${max_size_description}s\n", $data{$name}->{description};
+ }
+
+ matrix_lines;
+}
+
+
+#
+# Parses all feature files located at $prefix dir
+#
+find({wanted =>\&parse_feat, no_chdir => 1}, $prefix);
+
+print STDERR Data::Dumper->Dump([\%data], [qw(*data)]) if ($debug);
+
+#
+# Handles the command
+#
+if ($cmd eq "current") {
+ $arch = qx(uname -m | sed 's/x86_64/x86/' | sed 's/i386/x86/');
+ $arch =~s/\s+$//;
+}
+
+if ($cmd ne "validate") {
+ if ($arch) {
+ output_arch_table;
+ } elsif ($feat) {
+ output_feature;
+ } else {
+ output_matrix;
+ }
+}
+
+__END__
+
+=head1 NAME
+
+get_feat.pl - parse the Linux Feature files and produce a ReST book.
+
+=head1 SYNOPSIS
+
+B<get_feat.pl> [--debug] [--man] [--help] [--dir=<dir>]
+ [--arch=<arch>] [--feat=<feature>] <COMAND> [<ARGUMENT>]
+
+Where <COMMAND> can be:
+
+=over 8
+
+B<current> - output features for this machine's architecture
+
+B<rest> - output features in ReST markup language
+
+B<validate> - validate the feature contents
+
+=back
+
+=head1 OPTIONS
+
+=over 8
+
+=item B<--arch>
+
+Output features for an specific architecture, optionally filtering for
+a single specific feature.
+
+=item B<--feat>
+
+Output features for a single specific architecture.
+
+=item B<--dir>
+
+Changes the location of the Feature files. By default, it uses
+the Documentation/features directory.
+
+=item B<--debug>
+
+Put the script in verbose mode, useful for debugging. Can be called multiple
+times, to increase verbosity.
+
+=item B<--help>
+
+Prints a brief help message and exits.
+
+=item B<--man>
+
+Prints the manual page and exits.
+
+=back
+
+=head1 DESCRIPTION
+
+Parse the Linux feature files from Documentation/features (by default),
+optionally producing results at ReST format.
+
+It supports output data per architecture, per feature or a
+feature x arch matrix.
+
+When used with B<rest> command, it will use either one of the tree formats:
+
+If neither B<--arch> or B<--feature> arguments are used, it will output a
+matrix with features per architecture.
+
+If B<--arch> argument is used, it will output the features availability for
+a given architecture.
+
+If B<--feat> argument is used, it will output the content of the feature
+file using ReStructured Text markup.
+
+=head1 BUGS
+
+Report bugs to Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
+
+=head1 COPYRIGHT
+
+Copyright (c) 2019 by Mauro Carvalho Chehab <mchehab+samsung@kernel.org>.
+
+License GPLv2: GNU GPL version 2 <http://gnu.org/licenses/gpl.html>.
+
+This is free software: you are free to change and redistribute it.
+There is NO WARRANTY, to the extent permitted by law.
+
+=cut
--
2.21.0
^ permalink raw reply related
* Re: [PATCH 0/2] arm64: Introduce boot parameter to disable TLB flush instruction within the same inner shareable domain
From: Will Deacon @ 2019-06-17 17:03 UTC (permalink / raw)
To: Takao Indoh
Cc: Jonathan Corbet, Catalin Marinas, linux-doc, linux-kernel,
linux-arm-kernel, QI Fuli, Takao Indoh, peterz
In-Reply-To: <20190617143255.10462-1-indou.takao@jp.fujitsu.com>
Hi Takao,
[+Peter Z]
On Mon, Jun 17, 2019 at 11:32:53PM +0900, Takao Indoh wrote:
> From: Takao Indoh <indou.takao@fujitsu.com>
>
> I found a performance issue related on the implementation of Linux's TLB
> flush for arm64.
>
> When I run a single-threaded test program on moderate environment, it
> usually takes 39ms to finish its work. However, when I put a small
> apprication, which just calls mprotest() continuously, on one of sibling
> cores and run it simultaneously, the test program slows down significantly.
> It becomes 49ms(125%) on ThunderX2. I also detected the same problem on
> ThunderX1 and Fujitsu A64FX.
This is a problem for any applications that share hardware resources with
each other, so I don't think it's something we should be too concerned about
addressing unless there is a practical DoS scenario, which there doesn't
appear to be in this case. It may be that the real answer is "don't call
mprotect() in a loop".
> I suppose the root cause of this issue is the implementation of Linux's TLB
> flush for arm64, especially use of TLBI-is instruction which is a broadcast
> to all processor core on the system. In case of the above situation,
> TLBI-is is called by mprotect().
On the flip side, Linux is providing the hardware with enough information
not to broadcast to cores for which the remote TLBs don't have entries
allocated for the ASID being invalidated. I would say that the root cause
of the issue is that this filtering is not taking place.
> This is not a problem for small environment, but this causes a significant
> performance noise for large-scale HPC environment, which has more than
> thousand nodes with low latency interconnect.
If you have a system with over a thousand nodes, without snoop filtering
for DVM messages and you expect performance to scale in the face of tight
mprotect() loops then I think you have a problem irrespective of this patch.
What happens if somebody runs I-cache invalidation in a loop?
> To fix this problem, this patch adds new boot parameter
> 'disable_tlbflush_is'. In the case of flush_tlb_mm() *without* this
> parameter, TLB entry is invalidated by __tlbi(aside1is, asid). By this
> instruction, all CPUs within the same inner shareable domain check if there
> are TLB entries which have this ASID, this causes performance noise. OTOH,
> when this new parameter is specified, TLB entry is invalidated by
> __tlbi(aside1, asid) only on the CPUs specified by mm_cpumask(mm).
> Therefore TLB flush is done on minimal CPUs and performance problem does
> not occur. Actually I confirm the performance problem is fixed by this
> patch.
Other than my comments above, my overall concern with this patch is that
it introduces divergent behaviour for our TLB invalidation flow, which is
undesirable from both maintainability and usability perspectives. If you
wish to change the code, please don't put it behind a command-line option,
but instead improve the code that is already there. However, I suspect that
blowing away the local TLB on every context-switch may have hidden costs
which are only apparent with workloads different from the contrived case
that you're seeking to improve. You also haven't taken into account the
effects of virtualisation, where it's likely that the hypervisor will
upgrade non-shareable operations to inner-shareable ones anyway.
Thanks,
Will
^ permalink raw reply
* Re: [PATCH 12/14] doc-rst: add ABI documentation to the admin-guide book
From: Markus Heiser @ 2019-06-17 16:31 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Jonathan Corbet, Jani Nikula, Linux Doc Mailing List,
Greg Kroah-Hartman, Mauro Carvalho Chehab, Mauro Carvalho Chehab,
linux-kernel
In-Reply-To: <20190617061146.06975213@coco.lan>
Am 17.06.19 um 11:11 schrieb Mauro Carvalho Chehab:
> Em Sun, 16 Jun 2019 18:04:01 +0200
> Markus Heiser <markus.heiser@darmarit.de> escreveu:
>
>> Am 14.06.19 um 16:15 schrieb Jonathan Corbet:
>>> On Fri, 14 Jun 2019 16:10:31 +0200
>>> Markus Heiser <markus.heiser@darmarit.de> wrote:
>>>
>>>> I agree with Jani. No matter how the decision ends, since I can't help here, I'd
>>>> rather not show up in the copyright.
>>>
>>> Is there something specific you are asking us to do here?
>>>
>>
>>
>> I have lost the overview, but there was a patch Mauro added a
>> kernel_abi.py. There was my name (Markus Heiser) listed with a
>> copyright notation.
>>
>> I guess Mauro picked up some old RFC or an other old patch of
>> mine from 2016 and made some C&P .. whatever .. ATM I do not have
>> time to give any support on parsing ABI and I'am not interested
>> in holding copyrights on a C&P of a old source ;)
>
> Well, the code was basically written by you :-)
>
> It was written to be a script capable of running a generic
> script. On that time, my contribution to it was basically
> to hardcode it to run "get_abi.pl".
Thanks for clarifying.
>
> This came from an old branch where the last change was back in 2017.
> It was resurrected due to a discussion at KS ML.
>
> There, the discussion was related to what's left to be converted
> to ReST.
>
> While I can't simply remove your copyright, would you be happy
> with something like that?
Yes, but basically I share Jani's and Jon's doubts about this solution.
-- Markus --
>
>
> Thanks,
> Mauro
>
> diff --git a/Documentation/sphinx/kernel_abi.py b/Documentation/sphinx/kernel_abi.py
> index 2d5d582207f7..ef91b1e1ff4b 100644
> --- a/Documentation/sphinx/kernel_abi.py
> +++ b/Documentation/sphinx/kernel_abi.py
> @@ -7,7 +7,8 @@ u"""
> Implementation of the ``kernel-abi`` reST-directive.
>
> :copyright: Copyright (C) 2016 Markus Heiser
> - :copyright: Copyright (C) 2016 Mauro Carvalho Chehab
> + :copyright: Copyright (C) 2016-2019 Mauro Carvalho Chehab
> + :maintained-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> :license: GPL Version 2, June 1991 see Linux/COPYING for details.
>
> The ``kernel-abi`` (:py:class:`KernelCmd`) directive calls the
>
>
^ permalink raw reply
* Re: [PATCH 14/14] docs: sphinx/kernel_abi.py: fix UTF-8 support
From: Mauro Carvalho Chehab @ 2019-06-17 15:55 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Markus Heiser, Greg Kroah-Hartman, Linux Doc Mailing List,
Mauro Carvalho Chehab, linux-kernel
In-Reply-To: <20190617075608.696cf037@lwn.net>
Em Mon, 17 Jun 2019 07:56:08 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:
> On Mon, 17 Jun 2019 06:16:59 -0300
> Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
>
> > > No need to change, the emacs notation is also OK, see your link
> > >
> > > """or (using formats recognized by popular editors):"""
> > >
> > > https://www.python.org/dev/peps/pep-0263/#defining-the-encoding
> > >
> > > I prefer emacs notation, this is also evaluated by many other editors / tools.
> >
> > The usage of emacs notation is something that we don't like at the
> > Linux Kernel. With ~4K developers per release, if we add tags to
> > every single editor people use, it would be really messy, as one
> > developer would be adding a tag and the next one replacing it by its
> > some other favorite editor's tag.
>
> So "we" like a language-specific notation instead? That seems a little
> strange to me. Lots of things understand the Emacs notation, it doesn't
> seem like something that needs to be actively avoided here.
From my side, I don't have any strong preference. Just saying that
people usually complain when e-macs or vim specific tags appear at the
Kernel. That's why I would prefer an editor-agnostic macro.
It won't make any difference for me, anyway, as the editors I use
don't recognize it.
Whatever you want is OK to me, provided that we use the same notation on
all Sphinx extensions... right now there's a mix of notations.
Thanks,
Mauro
^ permalink raw reply
* [PATCH 2/2] arm64: tlb: Add boot parameter to disable TLB flush within the same inner shareable domain
From: Takao Indoh @ 2019-06-17 14:32 UTC (permalink / raw)
To: Jonathan Corbet, Catalin Marinas, Will Deacon
Cc: linux-doc, linux-kernel, linux-arm-kernel, QI Fuli, Takao Indoh
In-Reply-To: <20190617143255.10462-1-indou.takao@jp.fujitsu.com>
From: Takao Indoh <indou.takao@fujitsu.com>
This patch adds new boot parameter 'disable_tlbflush_is' to disable TLB
flush within the same inner shareable domain for performance tuning.
In the case of flush_tlb_mm() *without* this parameter, TLB entry is
invalidated by __tlbi(aside1is, asid). By this instruction, all CPUs within
the same inner shareable domain check if there are TLB entries which have
this ASID, this causes performance noise, especially at large-scale HPC
environment, which has more than thousand nodes with low latency
interconnect.
When this new parameter is specified, TLB entry is invalidated by
__tlbi(aside1, asid) only on the CPUs specified by mm_cpumask(mm).
Therefore TLB flush is done on minimal CPUs and performance problem does
not occur.
Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
Signed-off-by: Takao Indoh <indou.takao@fujitsu.com>
---
.../admin-guide/kernel-parameters.txt | 4 +
arch/arm64/include/asm/tlbflush.h | 61 ++-----
arch/arm64/kernel/Makefile | 2 +-
arch/arm64/kernel/tlbflush.c | 155 ++++++++++++++++++
4 files changed, 172 insertions(+), 50 deletions(-)
create mode 100644 arch/arm64/kernel/tlbflush.c
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 138f6664b2e2..a693eea34e48 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -848,6 +848,10 @@
disable= [IPV6]
See Documentation/networking/ipv6.txt.
+ disable_tlbflush_is
+ [ARM64] Disable using TLB instruction to flush
+ all PE within the same inner shareable domain.
+
hardened_usercopy=
[KNL] Under CONFIG_HARDENED_USERCOPY, whether
hardening is enabled for this boot. Hardened
diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
index dff8f9ea5754..ba2b3fd0b63c 100644
--- a/arch/arm64/include/asm/tlbflush.h
+++ b/arch/arm64/include/asm/tlbflush.h
@@ -139,6 +139,13 @@
* on top of these routines, since that is our interface to the mmu_gather
* API as used by munmap() and friends.
*/
+
+void flush_tlb_mm(struct mm_struct *mm);
+void flush_tlb_page_nosync(struct vm_area_struct *vma,
+ unsigned long uaddr);
+void __flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
+ unsigned long end, unsigned long stride, bool last_level);
+
static inline void local_flush_tlb_all(void)
{
dsb(nshst);
@@ -155,24 +162,14 @@ static inline void flush_tlb_all(void)
isb();
}
-static inline void flush_tlb_mm(struct mm_struct *mm)
+static inline void local_flush_tlb_mm(struct mm_struct *mm)
{
unsigned long asid = __TLBI_VADDR(0, ASID(mm));
- dsb(ishst);
- __tlbi(aside1is, asid);
- __tlbi_user(aside1is, asid);
- dsb(ish);
-}
-
-static inline void flush_tlb_page_nosync(struct vm_area_struct *vma,
- unsigned long uaddr)
-{
- unsigned long addr = __TLBI_VADDR(uaddr, ASID(vma->vm_mm));
-
- dsb(ishst);
- __tlbi(vale1is, addr);
- __tlbi_user(vale1is, addr);
+ dsb(nshst);
+ __tlbi(aside1, asid);
+ __tlbi_user(aside1, asid);
+ dsb(nsh);
}
static inline void flush_tlb_page(struct vm_area_struct *vma,
@@ -188,40 +185,6 @@ static inline void flush_tlb_page(struct vm_area_struct *vma,
*/
#define MAX_TLBI_OPS PTRS_PER_PTE
-static inline void __flush_tlb_range(struct vm_area_struct *vma,
- unsigned long start, unsigned long end,
- unsigned long stride, bool last_level)
-{
- unsigned long asid = ASID(vma->vm_mm);
- unsigned long addr;
-
- start = round_down(start, stride);
- end = round_up(end, stride);
-
- if ((end - start) >= (MAX_TLBI_OPS * stride)) {
- flush_tlb_mm(vma->vm_mm);
- return;
- }
-
- /* Convert the stride into units of 4k */
- stride >>= 12;
-
- start = __TLBI_VADDR(start, asid);
- end = __TLBI_VADDR(end, asid);
-
- dsb(ishst);
- for (addr = start; addr < end; addr += stride) {
- if (last_level) {
- __tlbi(vale1is, addr);
- __tlbi_user(vale1is, addr);
- } else {
- __tlbi(vae1is, addr);
- __tlbi_user(vae1is, addr);
- }
- }
- dsb(ish);
-}
-
static inline void flush_tlb_range(struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 9e7dcb2c31c7..266c9a57b081 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -19,7 +19,7 @@ obj-y := debug-monitors.o entry.o irq.o fpsimd.o \
return_address.o cpuinfo.o cpu_errata.o \
cpufeature.o alternative.o cacheinfo.o \
smp.o smp_spin_table.o topology.o smccc-call.o \
- syscall.o
+ syscall.o tlbflush.o
extra-$(CONFIG_EFI) := efi-entry.o
diff --git a/arch/arm64/kernel/tlbflush.c b/arch/arm64/kernel/tlbflush.c
new file mode 100644
index 000000000000..52c9a237759a
--- /dev/null
+++ b/arch/arm64/kernel/tlbflush.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2019 FUJITSU LIMITED
+
+#include <linux/smp.h>
+#include <asm/tlbflush.h>
+
+struct tlb_args {
+ struct vm_area_struct *ta_vma;
+ unsigned long ta_start;
+ unsigned long ta_end;
+ unsigned long ta_stride;
+ bool ta_last_level;
+};
+
+int disable_tlbflush_is;
+
+static int __init disable_tlbflush_is_setup(char *str)
+{
+ disable_tlbflush_is = 1;
+
+ return 0;
+}
+__setup("disable_tlbflush_is", disable_tlbflush_is_setup);
+
+static inline void __flush_tlb_mm(struct mm_struct *mm)
+{
+ unsigned long asid = __TLBI_VADDR(0, ASID(mm));
+
+ dsb(ishst);
+ __tlbi(aside1is, asid);
+ __tlbi_user(aside1is, asid);
+ dsb(ish);
+}
+
+static inline void ipi_flush_tlb_mm(void *arg)
+{
+ struct mm_struct *mm = arg;
+
+ local_flush_tlb_mm(mm);
+}
+
+void flush_tlb_mm(struct mm_struct *mm)
+{
+ if (disable_tlbflush_is)
+ on_each_cpu_mask(mm_cpumask(mm), ipi_flush_tlb_mm,
+ (void *)mm, true);
+ else
+ __flush_tlb_mm(mm);
+}
+
+static inline void __flush_tlb_page_nosync(unsigned long addr)
+{
+ dsb(ishst);
+ __tlbi(vale1is, addr);
+ __tlbi_user(vale1is, addr);
+}
+
+static inline void __local_flush_tlb_page_nosync(unsigned long addr)
+{
+ dsb(nshst);
+ __tlbi(vale1, addr);
+ __tlbi_user(vale1, addr);
+ dsb(nsh);
+}
+
+static inline void ipi_flush_tlb_page_nosync(void *arg)
+{
+ unsigned long addr = *(unsigned long *)arg;
+
+ __local_flush_tlb_page_nosync(addr);
+}
+
+void flush_tlb_page_nosync(struct vm_area_struct *vma, unsigned long uaddr)
+{
+ unsigned long addr = __TLBI_VADDR(uaddr, ASID(vma->vm_mm));
+
+ if (disable_tlbflush_is)
+ on_each_cpu_mask(mm_cpumask(vma->vm_mm),
+ ipi_flush_tlb_page_nosync, &addr, true);
+ else
+ __flush_tlb_page_nosync(addr);
+}
+
+static inline void ___flush_tlb_range(unsigned long start, unsigned long end,
+ unsigned long stride, bool last_level)
+{
+ unsigned long addr;
+
+ dsb(ishst);
+ for (addr = start; addr < end; addr += stride) {
+ if (last_level) {
+ __tlbi(vale1is, addr);
+ __tlbi_user(vale1is, addr);
+ } else {
+ __tlbi(vae1is, addr);
+ __tlbi_user(vae1is, addr);
+ }
+ }
+ dsb(ish);
+}
+
+static inline void __local_flush_tlb_range(unsigned long addr, bool last_level)
+{
+ dsb(nshst);
+ if (last_level) {
+ __tlbi(vale1, addr);
+ __tlbi_user(vale1, addr);
+ } else {
+ __tlbi(vae1, addr);
+ __tlbi_user(vae1, addr);
+ }
+ dsb(nsh);
+}
+
+static inline void ipi_flush_tlb_range(void *arg)
+{
+ struct tlb_args *ta = (struct tlb_args *)arg;
+ unsigned long addr;
+
+ for (addr = ta->ta_start; addr < ta->ta_end; addr += ta->ta_stride)
+ __local_flush_tlb_range(addr, ta->ta_last_level);
+}
+
+void __flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
+ unsigned long end, unsigned long stride, bool last_level)
+{
+ unsigned long asid = ASID(vma->vm_mm);
+
+ start = round_down(start, stride);
+ end = round_up(end, stride);
+
+ if ((end - start) >= (MAX_TLBI_OPS * stride)) {
+ flush_tlb_mm(vma->vm_mm);
+ return;
+ }
+
+ /* Convert the stride into units of 4k */
+ stride >>= 12;
+
+ start = __TLBI_VADDR(start, asid);
+ end = __TLBI_VADDR(end, asid);
+
+ if (disable_tlbflush_is) {
+ struct tlb_args ta = {
+ .ta_start = start,
+ .ta_end = end,
+ .ta_stride = stride,
+ .ta_last_level = last_level,
+ };
+
+ on_each_cpu_mask(mm_cpumask(vma->vm_mm), ipi_flush_tlb_range,
+ &ta, true);
+ } else
+ ___flush_tlb_range(start, end, stride, last_level);
+}
--
2.20.1
^ permalink raw reply related
* [PATCH 0/2] arm64: Introduce boot parameter to disable TLB flush instruction within the same inner shareable domain
From: Takao Indoh @ 2019-06-17 14:32 UTC (permalink / raw)
To: Jonathan Corbet, Catalin Marinas, Will Deacon
Cc: linux-doc, linux-kernel, linux-arm-kernel, QI Fuli, Takao Indoh
From: Takao Indoh <indou.takao@fujitsu.com>
I found a performance issue related on the implementation of Linux's TLB
flush for arm64.
When I run a single-threaded test program on moderate environment, it
usually takes 39ms to finish its work. However, when I put a small
apprication, which just calls mprotest() continuously, on one of sibling
cores and run it simultaneously, the test program slows down significantly.
It becomes 49ms(125%) on ThunderX2. I also detected the same problem on
ThunderX1 and Fujitsu A64FX.
I suppose the root cause of this issue is the implementation of Linux's TLB
flush for arm64, especially use of TLBI-is instruction which is a broadcast
to all processor core on the system. In case of the above situation,
TLBI-is is called by mprotect().
This is not a problem for small environment, but this causes a significant
performance noise for large-scale HPC environment, which has more than
thousand nodes with low latency interconnect.
To fix this problem, this patch adds new boot parameter
'disable_tlbflush_is'. In the case of flush_tlb_mm() *without* this
parameter, TLB entry is invalidated by __tlbi(aside1is, asid). By this
instruction, all CPUs within the same inner shareable domain check if there
are TLB entries which have this ASID, this causes performance noise. OTOH,
when this new parameter is specified, TLB entry is invalidated by
__tlbi(aside1, asid) only on the CPUs specified by mm_cpumask(mm).
Therefore TLB flush is done on minimal CPUs and performance problem does
not occur. Actually I confirm the performance problem is fixed by this
patch.
Takao Indoh (2):
arm64: mm: Restore mm_cpumask (revert commit 38d96287504a ("arm64: mm:
kill mm_cpumask usage"))
arm64: tlb: Add boot parameter to disable TLB flush within the same
inner shareable domain
.../admin-guide/kernel-parameters.txt | 4 +
arch/arm64/include/asm/mmu_context.h | 7 +-
arch/arm64/include/asm/tlbflush.h | 61 ++-----
arch/arm64/kernel/Makefile | 2 +-
arch/arm64/kernel/smp.c | 6 +
arch/arm64/kernel/tlbflush.c | 155 ++++++++++++++++++
arch/arm64/mm/context.c | 2 +
7 files changed, 186 insertions(+), 51 deletions(-)
create mode 100644 arch/arm64/kernel/tlbflush.c
--
2.20.1
^ permalink raw reply
* [PATCH 1/2] arm64: mm: Restore mm_cpumask (revert commit 38d96287504a ("arm64: mm: kill mm_cpumask usage"))
From: Takao Indoh @ 2019-06-17 14:32 UTC (permalink / raw)
To: Jonathan Corbet, Catalin Marinas, Will Deacon
Cc: linux-doc, linux-kernel, linux-arm-kernel, QI Fuli, Takao Indoh
In-Reply-To: <20190617143255.10462-1-indou.takao@jp.fujitsu.com>
From: Takao Indoh <indou.takao@fujitsu.com>
mm_cpumask was deleted by the commit 38d96287504a ("arm64: mm: kill
mm_cpumask usage") because it was not used at that time. Now this is needed
to find appropriate CPUs for TLB flush, so this patch reverts this commit.
Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
Signed-off-by: Takao Indoh <indou.takao@fujitsu.com>
---
arch/arm64/include/asm/mmu_context.h | 7 ++++++-
arch/arm64/kernel/smp.c | 6 ++++++
arch/arm64/mm/context.c | 2 ++
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index 2da3e478fd8f..21ef11590bcb 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -241,8 +241,13 @@ static inline void
switch_mm(struct mm_struct *prev, struct mm_struct *next,
struct task_struct *tsk)
{
- if (prev != next)
+ unsigned int cpu = smp_processor_id();
+
+ if (prev != next) {
__switch_mm(next);
+ cpumask_clear_cpu(cpu, mm_cpumask(prev));
+ local_flush_tlb_mm(prev);
+ }
/*
* Update the saved TTBR0_EL1 of the scheduled-in task as the previous
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index bb4b3f07761a..12a922d1cdd7 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -218,6 +218,7 @@ asmlinkage notrace void secondary_start_kernel(void)
*/
mmgrab(mm);
current->active_mm = mm;
+ cpumask_set_cpu(cpu, mm_cpumask(mm));
/*
* TTBR0 is only used for the identity mapping at this stage. Make it
@@ -320,6 +321,11 @@ int __cpu_disable(void)
*/
irq_migrate_all_off_this_cpu();
+ /*
+ * Remove this CPU from the vm mask set of all processes.
+ */
+ clear_tasks_mm_cpumask(cpu);
+
return 0;
}
diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
index 1f0ea2facf24..ff3ab2924074 100644
--- a/arch/arm64/mm/context.c
+++ b/arch/arm64/mm/context.c
@@ -188,6 +188,7 @@ static u64 new_context(struct mm_struct *mm)
set_asid:
__set_bit(asid, asid_map);
cur_idx = asid;
+ cpumask_clear(mm_cpumask(mm));
return idx2asid(asid) | generation;
}
@@ -239,6 +240,7 @@ void check_and_switch_context(struct mm_struct *mm, unsigned int cpu)
switch_mm_fastpath:
arm64_apply_bp_hardening();
+ cpumask_set_cpu(cpu, mm_cpumask(mm));
/*
* Defer TTBR0_EL1 setting for user threads to uaccess_enable() when
--
2.20.1
^ permalink raw reply related
* [PATCH] doc-rst: Add missing newline at end of file
From: Geert Uytterhoeven @ 2019-06-17 14:34 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: linux-doc, linux-kernel, Geert Uytterhoeven
"git diff" says:
\ No newline at end of file
after modifying the file.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Documentation/docutils.conf | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/docutils.conf b/Documentation/docutils.conf
index 2830772264c877ca..f1a180b97dec5cb3 100644
--- a/Documentation/docutils.conf
+++ b/Documentation/docutils.conf
@@ -4,4 +4,4 @@
# http://docutils.sourceforge.net/docs/user/config.html
[general]
-halt_level: severe
\ No newline at end of file
+halt_level: severe
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v5 0/3] Bitops instrumentation for KASAN
From: Marco Elver @ 2019-06-17 14:00 UTC (permalink / raw)
To: Peter Zijlstra, Andrey Ryabinin, Dmitry Vyukov,
Alexander Potapenko, Andrey Konovalov, Mark Rutland,
H. Peter Anvin, Andrew Morton
Cc: Jonathan Corbet, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
the arch/x86 maintainers, Arnd Bergmann, Josh Poimboeuf,
open list:DOCUMENTATION, LKML, linux-arch, kasan-dev,
Linux Memory Management List
In-Reply-To: <20190613125950.197667-1-elver@google.com>
All 3 patches have now been Acked and Reviewed. Which tree should this land in?
Since this is related to KASAN, would this belong into the MM tree?
Many thanks,
-- Marco
On Thu, 13 Jun 2019 at 15:00, Marco Elver <elver@google.com> wrote:
>
> Previous version:
> http://lkml.kernel.org/r/20190613123028.179447-1-elver@google.com
>
> * Only changed lib/test_kasan in this version.
>
> Marco Elver (3):
> lib/test_kasan: Add bitops tests
> x86: Use static_cpu_has in uaccess region to avoid instrumentation
> asm-generic, x86: Add bitops instrumentation for KASAN
>
> Documentation/core-api/kernel-api.rst | 2 +-
> arch/x86/ia32/ia32_signal.c | 2 +-
> arch/x86/include/asm/bitops.h | 189 ++++------------
> arch/x86/kernel/signal.c | 2 +-
> include/asm-generic/bitops-instrumented.h | 263 ++++++++++++++++++++++
> lib/test_kasan.c | 81 ++++++-
> 6 files changed, 382 insertions(+), 157 deletions(-)
> create mode 100644 include/asm-generic/bitops-instrumented.h
>
> --
> 2.22.0.rc2.383.gf4fbbf30c2-goog
>
^ permalink raw reply
* Re: [PATCH 14/14] docs: sphinx/kernel_abi.py: fix UTF-8 support
From: Jonathan Corbet @ 2019-06-17 13:56 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Markus Heiser, Greg Kroah-Hartman, Linux Doc Mailing List,
Mauro Carvalho Chehab, linux-kernel
In-Reply-To: <20190617061659.22596fc3@coco.lan>
On Mon, 17 Jun 2019 06:16:59 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> > No need to change, the emacs notation is also OK, see your link
> >
> > """or (using formats recognized by popular editors):"""
> >
> > https://www.python.org/dev/peps/pep-0263/#defining-the-encoding
> >
> > I prefer emacs notation, this is also evaluated by many other editors / tools.
>
> The usage of emacs notation is something that we don't like at the
> Linux Kernel. With ~4K developers per release, if we add tags to
> every single editor people use, it would be really messy, as one
> developer would be adding a tag and the next one replacing it by its
> some other favorite editor's tag.
So "we" like a language-specific notation instead? That seems a little
strange to me. Lots of things understand the Emacs notation, it doesn't
seem like something that needs to be actively avoided here.
Thanks,
jon
^ permalink raw reply
* Re: [PATCH 12/14] doc-rst: add ABI documentation to the admin-guide book
From: Mauro Carvalho Chehab @ 2019-06-17 13:51 UTC (permalink / raw)
To: Jani Nikula
Cc: Greg Kroah-Hartman, Linux Doc Mailing List, Mauro Carvalho Chehab,
Mauro Carvalho Chehab, linux-kernel, Jonathan Corbet
In-Reply-To: <874l4ov16m.fsf@intel.com>
Em Mon, 17 Jun 2019 15:36:17 +0300
Jani Nikula <jani.nikula@linux.intel.com> escreveu:
> On Fri, 14 Jun 2019, Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> > Em Fri, 14 Jun 2019 16:06:03 +0200
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> >
> >> On Fri, Jun 14, 2019 at 04:42:20PM +0300, Jani Nikula wrote:
> >> > 2) Have the python extension read the ABI files directly, without an
> >> > extra pipeline.
> >>
> >> He who writes the script, get's to dictate the language of the script :)
>
> The point is, it's an extension to a python based tool, written in perl,
> using pipes for communication, and losing any advantages of integrating
> with the tool it's extending.
>
> I doubt you'd want to see system() to be used to subsequently extend the
> perl tool.
>
> I think it's just sad to see the documentation system slowly drift
> further away from the ideals we had, and towards the old ways we worked
> so hard to fix.
Actually, it is a perl script that can be used standalone (just like
get_maintainers.pl and kernel-doc) with have some features including
producing a ReST output. We could easily get rid of the python extension,
if we add this to the Makefile (adjusted to work with O= option):
./scripts/get_api.pl rest > Documentation/output/admin-guide/abi.rst
>
> > No idea about how much time it would take if written in python,
> > but this perl script is really fast:
> >
> > $ time ./scripts/get_abi.pl search voltage_max >/dev/null
> > real 0m0,139s
> > user 0m0,132s
> > sys 0m0,006s
> >
> > That's the time it takes here (SSD disks) to read all files under
> > Documentation/ABI, parse them and seek for a string.
> >
> > That's about half of the time a python script takes to just import the
> > the sphinx modules and print its version, running at the same machine:
> >
> > $ time sphinx-build --version >/dev/null
> >
> > real 0m0,224s
> > user 0m0,199s
> > sys 0m0,024s
>
> Please at least use fair and sensible comparisons. If you want to make
> the extension usable standalone on the command-line, bypassing Sphinx,
> you can do that. No need to factor in Sphinx to your comparisons.
Yeah, I guess it should be possible to do that. How a python script
can identify if it was called by Sphinx, or if it was called directly?
Thanks,
Mauro
^ permalink raw reply
* Re: [PATCH 12/14] doc-rst: add ABI documentation to the admin-guide book
From: Jonathan Corbet @ 2019-06-17 13:48 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jani Nikula, Mauro Carvalho Chehab, Linux Doc Mailing List,
Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel
In-Reply-To: <20190617125438.GA18554@kroah.com>
On Mon, 17 Jun 2019 14:54:38 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > I think it's just sad to see the documentation system slowly drift
> > further away from the ideals we had, and towards the old ways we worked
> > so hard to fix.
>
> What are those ideals?
>
> I thought the goal was to be able to write documentation in a as much
> as a normal text file as possible and have automation turn those files
> into "pretty" documentation that we can all use.
That was indeed one of the goals. Another was to replace the incredible
pile of fragile duct tape that the docs build system had become with
something more robust, understandable, and maintainable. We did that, to
an extent at least, and life is better.
Jani worries that we have been regressing toward duct-tape mode, and I
suspect he may be right. I'm certainly as guilty as anybody of tossing
stuff in because it's expedient right now. It is right to ask whether we
should continue in that direction.
Can we slow down just a bit on the ABI files? It may be that Mauro's
solution is the best one, but I would really like to think a bit about
how all this stuff fits together, and life isn't really even giving me
time to tie my shoes these days. I don't think that this is screamingly
urgent right now.
Thanks,
jon
^ permalink raw reply
* Re: [PATCH 12/14] doc-rst: add ABI documentation to the admin-guide book
From: Jani Nikula @ 2019-06-17 13:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Mauro Carvalho Chehab, Linux Doc Mailing List,
Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet
In-Reply-To: <20190617125438.GA18554@kroah.com>
On Mon, 17 Jun 2019, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> On Mon, Jun 17, 2019 at 03:36:17PM +0300, Jani Nikula wrote:
>> On Fri, 14 Jun 2019, Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
>> > Em Fri, 14 Jun 2019 16:06:03 +0200
>> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
>> >
>> >> On Fri, Jun 14, 2019 at 04:42:20PM +0300, Jani Nikula wrote:
>> >> > 2) Have the python extension read the ABI files directly, without an
>> >> > extra pipeline.
>> >>
>> >> He who writes the script, get's to dictate the language of the script :)
>>
>> The point is, it's an extension to a python based tool, written in perl,
>> using pipes for communication, and losing any advantages of integrating
>> with the tool it's extending.
>>
>> I doubt you'd want to see system() to be used to subsequently extend the
>> perl tool.
>>
>> I think it's just sad to see the documentation system slowly drift
>> further away from the ideals we had, and towards the old ways we worked
>> so hard to fix.
>
> What are those ideals?
For example, have a single coherent system, instead of a fragile pipe
with each stage written in a different language, each having its own
idiosynchracies, each step losing something in translation.
Have a system that a normal developer can actually look at and
understand. It didn't use to be that way.
> I thought the goal was to be able to write documentation in a as much
> as a normal text file as possible and have automation turn those files
> into "pretty" documentation that we can all use.
>
> And I think that fits with the way this patch set goes, right? We are
> not on a quest for purity of scripts to generate the documentation at
> the expense of having to force hundreds, or thousands, of developers to
> change their ways, or to force a "flag day" conversion of existing
> documentation resulting in a huge merge mess.
Fair enough, let's dismiss the thought of changing the ABI files. But I
never meant that would somehow be for the "purity of scripts", or that
those two would somehow be at odds here.
> So, we are stuck with the current structure that I totally made up for
> Documentation/ABI/. Turns out it is almost parsable, as Mauro's tool
> shows. His tool also validates the existing text, which is great, and
> has caused fixes for it.
>
> If someone wants to write that tool in some other language, like python,
> wonderful, I have no objection, but as it is, this is a useful tool
> already, allowing us to validate, and search, existing documentation
> entries that we have never been able to do before. It also provides an
> output that can be turned into pretty html/pdf/whatever files by other
> tools in the pipeline, a totally bonus benefit.
>
> So what is going backwards here?
>
> Maybe the processing pipeline isn't as nice as you would like, but
> remember to view this from a normal developer's point of view, not a
> documentation pipeline developer's point of view please.
>
> So, in short, my requirements are:
> - keep Documentation/ABI/ file formats as close as possible to
> what we have today, preventing any flag-day issues or merge
> problems
> - be able to query and validate Documentation/ABI/
> - be able to turn Documentation/ABI into pretty documentation.
>
> If you object to the mechanics of the last requirement here, I don't
> object either, provide something else that works better. But don't
> throw away the whole thing just because you don't like how things are
> hooked up here.
>
> I'm going to go apply most of the rest of these patches to my
> driver-core tree, stopping at the "hook it up to the kernel
> documentation" point. Is that ok?
I'll leave it all up to Jon's discretion; I trust he'll understand my
concerns. I have no authority beyond the opinion I've voiced here
anyway.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply
* Re: [PATCH] dt: leds-lm36274.txt: fix a broken reference to ti-lmu.txt
From: Dan Murphy @ 2019-06-17 13:45 UTC (permalink / raw)
To: Pavel Machek, Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Jacek Anaszewski, Rob Herring, Mark Rutland,
linux-leds, devicetree
In-Reply-To: <20190617131220.GD21113@amd>
On 6/17/19 8:12 AM, Pavel Machek wrote:
> On Thu 2019-06-13 07:23:15, Mauro Carvalho Chehab wrote:
>> There's a typo there:
>> ti_lmu.txt -> ti-lmu.txt
>>
>> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> Acked-by: Pavel Machek <pavel@ucw.cz>
Acked-by: Dan Murphy <dmurphy@ti.com>
>
>> @@ -6,7 +6,7 @@ up to 29V total output voltage. The 11-bit LED current is programmable via
>> the I2C bus and/or controlled via a logic level PWM input from 60 uA to 30 mA.
>>
>> Parent device properties are documented in
>> -Documentation/devicetree/bindings/mfd/ti_lmu.txt
>> +Documentation/devicetree/bindings/mfd/ti-lmu.txt
>>
>> Regulator properties are documented in
>> Documentation/devicetree/bindings/regulator/lm363x-regulator.txt
^ permalink raw reply
* Re: [PATCH] dt: leds-lm36274.txt: fix a broken reference to ti-lmu.txt
From: Pavel Machek @ 2019-06-17 13:12 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Jacek Anaszewski, Dan Murphy, Rob Herring,
Mark Rutland, linux-leds, devicetree
In-Reply-To: <79b9bf3388eb231da77c6a804862d21339262d0a.1560421387.git.mchehab+samsung@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 823 bytes --]
On Thu 2019-06-13 07:23:15, Mauro Carvalho Chehab wrote:
> There's a typo there:
> ti_lmu.txt -> ti-lmu.txt
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
> @@ -6,7 +6,7 @@ up to 29V total output voltage. The 11-bit LED current is programmable via
> the I2C bus and/or controlled via a logic level PWM input from 60 uA to 30 mA.
>
> Parent device properties are documented in
> -Documentation/devicetree/bindings/mfd/ti_lmu.txt
> +Documentation/devicetree/bindings/mfd/ti-lmu.txt
>
> Regulator properties are documented in
> Documentation/devicetree/bindings/regulator/lm363x-regulator.txt
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v2 03/16] scripts: add an script to parse the ABI files
From: Greg Kroah-Hartman @ 2019-06-17 12:57 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet
In-Reply-To: <680fb978ef9322c705eca9927c79b220cd3ccc4a.1560534648.git.mchehab+samsung@kernel.org>
On Fri, Jun 14, 2019 at 02:52:17PM -0300, Mauro Carvalho Chehab wrote:
> Add a script to parse the Documentation/ABI files and produce
> an output with all entries inside an ABI (sub)directory.
>
> Right now, it outputs its contents on ReST format. It shouldn't
> be hard to make it produce other kind of outputs, since the ABI
> file parser is implemented in separate than the output generator.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
> scripts/get_abi.pl | 212 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 212 insertions(+)
> create mode 100755 scripts/get_abi.pl
>
> diff --git a/scripts/get_abi.pl b/scripts/get_abi.pl
> new file mode 100755
> index 000000000000..f7c9944a833c
> --- /dev/null
> +++ b/scripts/get_abi.pl
> @@ -0,0 +1,212 @@
> +#!/usr/bin/perl
> +
Ok, I was going to apply this, but there is no SPDX line on the script.
Can you resend this series with that on it, so that I can apply the
patches of the series that adds the script to the kernel tree?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 12/14] doc-rst: add ABI documentation to the admin-guide book
From: Greg Kroah-Hartman @ 2019-06-17 12:54 UTC (permalink / raw)
To: Jani Nikula
Cc: Mauro Carvalho Chehab, Linux Doc Mailing List,
Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet
In-Reply-To: <874l4ov16m.fsf@intel.com>
On Mon, Jun 17, 2019 at 03:36:17PM +0300, Jani Nikula wrote:
> On Fri, 14 Jun 2019, Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> > Em Fri, 14 Jun 2019 16:06:03 +0200
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> >
> >> On Fri, Jun 14, 2019 at 04:42:20PM +0300, Jani Nikula wrote:
> >> > 2) Have the python extension read the ABI files directly, without an
> >> > extra pipeline.
> >>
> >> He who writes the script, get's to dictate the language of the script :)
>
> The point is, it's an extension to a python based tool, written in perl,
> using pipes for communication, and losing any advantages of integrating
> with the tool it's extending.
>
> I doubt you'd want to see system() to be used to subsequently extend the
> perl tool.
>
> I think it's just sad to see the documentation system slowly drift
> further away from the ideals we had, and towards the old ways we worked
> so hard to fix.
What are those ideals?
I thought the goal was to be able to write documentation in a as much
as a normal text file as possible and have automation turn those files
into "pretty" documentation that we can all use.
And I think that fits with the way this patch set goes, right? We are
not on a quest for purity of scripts to generate the documentation at
the expense of having to force hundreds, or thousands, of developers to
change their ways, or to force a "flag day" conversion of existing
documentation resulting in a huge merge mess.
So, we are stuck with the current structure that I totally made up for
Documentation/ABI/. Turns out it is almost parsable, as Mauro's tool
shows. His tool also validates the existing text, which is great, and
has caused fixes for it.
If someone wants to write that tool in some other language, like python,
wonderful, I have no objection, but as it is, this is a useful tool
already, allowing us to validate, and search, existing documentation
entries that we have never been able to do before. It also provides an
output that can be turned into pretty html/pdf/whatever files by other
tools in the pipeline, a totally bonus benefit.
So what is going backwards here?
Maybe the processing pipeline isn't as nice as you would like, but
remember to view this from a normal developer's point of view, not a
documentation pipeline developer's point of view please.
So, in short, my requirements are:
- keep Documentation/ABI/ file formats as close as possible to
what we have today, preventing any flag-day issues or merge
problems
- be able to query and validate Documentation/ABI/
- be able to turn Documentation/ABI into pretty documentation.
If you object to the mechanics of the last requirement here, I don't
object either, provide something else that works better. But don't
throw away the whole thing just because you don't like how things are
hooked up here.
I'm going to go apply most of the rest of these patches to my
driver-core tree, stopping at the "hook it up to the kernel
documentation" point. Is that ok?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 12/14] doc-rst: add ABI documentation to the admin-guide book
From: Jani Nikula @ 2019-06-17 12:36 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab,
Mauro Carvalho Chehab, linux-kernel, Jonathan Corbet
In-Reply-To: <20190614122755.1c7b4898@coco.lan>
On Fri, 14 Jun 2019, Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> Em Fri, 14 Jun 2019 16:06:03 +0200
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
>
>> On Fri, Jun 14, 2019 at 04:42:20PM +0300, Jani Nikula wrote:
>> > 2) Have the python extension read the ABI files directly, without an
>> > extra pipeline.
>>
>> He who writes the script, get's to dictate the language of the script :)
The point is, it's an extension to a python based tool, written in perl,
using pipes for communication, and losing any advantages of integrating
with the tool it's extending.
I doubt you'd want to see system() to be used to subsequently extend the
perl tool.
I think it's just sad to see the documentation system slowly drift
further away from the ideals we had, and towards the old ways we worked
so hard to fix.
> No idea about how much time it would take if written in python,
> but this perl script is really fast:
>
> $ time ./scripts/get_abi.pl search voltage_max >/dev/null
> real 0m0,139s
> user 0m0,132s
> sys 0m0,006s
>
> That's the time it takes here (SSD disks) to read all files under
> Documentation/ABI, parse them and seek for a string.
>
> That's about half of the time a python script takes to just import the
> the sphinx modules and print its version, running at the same machine:
>
> $ time sphinx-build --version >/dev/null
>
> real 0m0,224s
> user 0m0,199s
> sys 0m0,024s
Please at least use fair and sensible comparisons. If you want to make
the extension usable standalone on the command-line, bypassing Sphinx,
you can do that. No need to factor in Sphinx to your comparisons.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply
* Re: [PATCH v7 22/27] binfmt_elf: Extract .note.gnu.property from an ELF file
From: Thomas Gleixner @ 2019-06-17 12:20 UTC (permalink / raw)
To: Florian Weimer
Cc: Dave Martin, Yu-cheng Yu, x86, H. Peter Anvin, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue
In-Reply-To: <87imt4jwpt.fsf@oldenburg2.str.redhat.com>
On Mon, 17 Jun 2019, Florian Weimer wrote:
> * Dave Martin:
> > On Tue, Jun 11, 2019 at 12:31:34PM -0700, Yu-cheng Yu wrote:
> >> We can probably check PT_GNU_PROPERTY first, and fallback (based on ld-linux
> >> version?) to PT_NOTE scanning?
> >
> > For arm64, we can check for PT_GNU_PROPERTY and then give up
> > unconditionally.
> >
> > For x86, we would fall back to PT_NOTE scanning, but this will add a bit
> > of cost to binaries that don't have NT_GNU_PROPERTY_TYPE_0. The ld.so
> > version doesn't tell you what ELF ABI a given executable conforms to.
> >
> > Since this sounds like it's largely a distro-specific issue, maybe there
> > could be a Kconfig option to turn the fallback PT_NOTE scanning on?
>
> I'm worried that this causes interop issues similarly to what we see
> with VSYSCALL today. If we need both and a way to disable it, it should
> be something like a personality flag which can be configured for each
> process tree separately. Ideally, we'd settle on one correct approach
> (i.e., either always process both, or only process PT_GNU_PROPERTY) and
> enforce that.
Chose one and only the one which makes technically sense and is not some
horrible vehicle.
Everytime we did those 'oh we need to make x fly workarounds' we regretted
it sooner than later.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH v7 22/27] binfmt_elf: Extract .note.gnu.property from an ELF file
From: Florian Weimer @ 2019-06-17 11:08 UTC (permalink / raw)
To: Dave Martin
Cc: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue
In-Reply-To: <20190612093238.GQ28398@e103592.cambridge.arm.com>
* Dave Martin:
> On Tue, Jun 11, 2019 at 12:31:34PM -0700, Yu-cheng Yu wrote:
>> On Tue, 2019-06-11 at 12:41 +0100, Dave Martin wrote:
>> > On Mon, Jun 10, 2019 at 07:24:43PM +0200, Florian Weimer wrote:
>> > > * Yu-cheng Yu:
>> > >
>> > > > To me, looking at PT_GNU_PROPERTY and not trying to support anything is a
>> > > > logical choice. And it breaks only a limited set of toolchains.
>> > > >
>> > > > I will simplify the parser and leave this patch as-is for anyone who wants
>> > > > to
>> > > > back-port. Are there any objections or concerns?
>> > >
>> > > Red Hat Enterprise Linux 8 does not use PT_GNU_PROPERTY and is probably
>> > > the largest collection of CET-enabled binaries that exists today.
>> >
>> > For clarity, RHEL is actively parsing these properties today?
>> >
>> > > My hope was that we would backport the upstream kernel patches for CET,
>> > > port the glibc dynamic loader to the new kernel interface, and be ready
>> > > to run with CET enabled in principle (except that porting userspace
>> > > libraries such as OpenSSL has not really started upstream, so many
>> > > processes where CET is particularly desirable will still run without
>> > > it).
>> > >
>> > > I'm not sure if it is a good idea to port the legacy support if it's not
>> > > part of the mainline kernel because it comes awfully close to creating
>> > > our own private ABI.
>> >
>> > I guess we can aim to factor things so that PT_NOTE scanning is
>> > available as a fallback on arches for which the absence of
>> > PT_GNU_PROPERTY is not authoritative.
>>
>> We can probably check PT_GNU_PROPERTY first, and fallback (based on ld-linux
>> version?) to PT_NOTE scanning?
>
> For arm64, we can check for PT_GNU_PROPERTY and then give up
> unconditionally.
>
> For x86, we would fall back to PT_NOTE scanning, but this will add a bit
> of cost to binaries that don't have NT_GNU_PROPERTY_TYPE_0. The ld.so
> version doesn't tell you what ELF ABI a given executable conforms to.
>
> Since this sounds like it's largely a distro-specific issue, maybe there
> could be a Kconfig option to turn the fallback PT_NOTE scanning on?
I'm worried that this causes interop issues similarly to what we see
with VSYSCALL today. If we need both and a way to disable it, it should
be something like a personality flag which can be configured for each
process tree separately. Ideally, we'd settle on one correct approach
(i.e., either always process both, or only process PT_GNU_PROPERTY) and
enforce that.
Thanks,
Florian
^ permalink raw reply
* Re: [PATCH 14/14] docs: sphinx/kernel_abi.py: fix UTF-8 support
From: Mauro Carvalho Chehab @ 2019-06-17 9:16 UTC (permalink / raw)
To: Markus Heiser
Cc: Greg Kroah-Hartman, Linux Doc Mailing List, Mauro Carvalho Chehab,
linux-kernel, Jonathan Corbet
In-Reply-To: <28aca947-4e88-7186-7f07-9a3ccb379649@darmarit.de>
Em Sun, 16 Jun 2019 17:43:50 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:
> Am 14.06.19 um 18:25 schrieb Mauro Carvalho Chehab:
> > Em Fri, 14 Jun 2019 18:18:37 +0200
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> >
> >> On Thu, Jun 13, 2019 at 11:04:20PM -0300, Mauro Carvalho Chehab wrote:
> >>> The parser breaks with UTF-8 characters with Sphinx 1.4.
> >>>
> >>> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> >>> ---
> >>> Documentation/sphinx/kernel_abi.py | 10 ++++++----
> >>> 1 file changed, 6 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/Documentation/sphinx/kernel_abi.py b/Documentation/sphinx/kernel_abi.py
> >>> index 7fa7806532dc..460cee48a245 100644
> >>> --- a/Documentation/sphinx/kernel_abi.py
> >>> +++ b/Documentation/sphinx/kernel_abi.py
> >>> @@ -1,4 +1,5 @@
> >>> -# -*- coding: utf-8; mode: python -*-
> >>> +# coding=utf-8
> >>> +#
> >>
> >> Is this an emacs vs. vim fight?
> >
> > No. This is a python-specific thing:
> >
> > https://www.python.org/dev/peps/pep-0263/
>
> No need to change, the emacs notation is also OK, see your link
>
> """or (using formats recognized by popular editors):"""
>
> https://www.python.org/dev/peps/pep-0263/#defining-the-encoding
>
> I prefer emacs notation, this is also evaluated by many other editors / tools.
The usage of emacs notation is something that we don't like at the
Linux Kernel. With ~4K developers per release, if we add tags to
every single editor people use, it would be really messy, as one
developer would be adding a tag and the next one replacing it by its
some other favorite editor's tag.
There's even an item at Documentation/process/coding-style.rst
due to that (item 9).
Thanks,
Mauro
^ permalink raw reply
* Re: [PATCH 12/14] doc-rst: add ABI documentation to the admin-guide book
From: Mauro Carvalho Chehab @ 2019-06-17 9:11 UTC (permalink / raw)
To: Markus Heiser
Cc: Jonathan Corbet, Jani Nikula, Linux Doc Mailing List,
Greg Kroah-Hartman, Mauro Carvalho Chehab, Mauro Carvalho Chehab,
linux-kernel
In-Reply-To: <327067f6-2609-41e6-c987-e37620e7154e@darmarit.de>
Em Sun, 16 Jun 2019 18:04:01 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:
> Am 14.06.19 um 16:15 schrieb Jonathan Corbet:
> > On Fri, 14 Jun 2019 16:10:31 +0200
> > Markus Heiser <markus.heiser@darmarit.de> wrote:
> >
> >> I agree with Jani. No matter how the decision ends, since I can't help here, I'd
> >> rather not show up in the copyright.
> >
> > Is there something specific you are asking us to do here?
> >
>
>
> I have lost the overview, but there was a patch Mauro added a
> kernel_abi.py. There was my name (Markus Heiser) listed with a
> copyright notation.
>
> I guess Mauro picked up some old RFC or an other old patch of
> mine from 2016 and made some C&P .. whatever .. ATM I do not have
> time to give any support on parsing ABI and I'am not interested
> in holding copyrights on a C&P of a old source ;)
Well, the code was basically written by you :-)
It was written to be a script capable of running a generic
script. On that time, my contribution to it was basically
to hardcode it to run "get_abi.pl".
This came from an old branch where the last change was back in 2017.
It was resurrected due to a discussion at KS ML.
There, the discussion was related to what's left to be converted
to ReST.
While I can't simply remove your copyright, would you be happy
with something like that?
Thanks,
Mauro
diff --git a/Documentation/sphinx/kernel_abi.py b/Documentation/sphinx/kernel_abi.py
index 2d5d582207f7..ef91b1e1ff4b 100644
--- a/Documentation/sphinx/kernel_abi.py
+++ b/Documentation/sphinx/kernel_abi.py
@@ -7,7 +7,8 @@ u"""
Implementation of the ``kernel-abi`` reST-directive.
:copyright: Copyright (C) 2016 Markus Heiser
- :copyright: Copyright (C) 2016 Mauro Carvalho Chehab
+ :copyright: Copyright (C) 2016-2019 Mauro Carvalho Chehab
+ :maintained-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
:license: GPL Version 2, June 1991 see Linux/COPYING for details.
The ``kernel-abi`` (:py:class:`KernelCmd`) directive calls the
^ permalink raw reply related
* [PATCH v5 03/18] kunit: test: add string_stream a std::stream like string builder
From: Brendan Higgins @ 2019-06-17 8:25 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190617082613.109131-1-brendanhiggins@google.com>
A number of test features need to do pretty complicated string printing
where it may not be possible to rely on a single preallocated string
with parameters.
So provide a library for constructing the string as you go similar to
C++'s std::string.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes Since Last Revision:
- Renamed new_string_stream() to alloc_string_stream() as suggested by
Stephen Boyd.
- Made string-stream a KUnit managed object - based on a suggestion
made by Stephen Boyd.
- Made a number of other small changes like renaming parameters as
suggested by Stephen Boyd.
---
include/kunit/string-stream.h | 49 ++++++++++++
kunit/Makefile | 3 +-
kunit/string-stream.c | 147 ++++++++++++++++++++++++++++++++++
3 files changed, 198 insertions(+), 1 deletion(-)
create mode 100644 include/kunit/string-stream.h
create mode 100644 kunit/string-stream.c
diff --git a/include/kunit/string-stream.h b/include/kunit/string-stream.h
new file mode 100644
index 0000000000000..0552a05781afe
--- /dev/null
+++ b/include/kunit/string-stream.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * C++ stream style string builder used in KUnit for building messages.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#ifndef _KUNIT_STRING_STREAM_H
+#define _KUNIT_STRING_STREAM_H
+
+#include <linux/types.h>
+#include <linux/spinlock.h>
+#include <linux/kref.h>
+#include <stdarg.h>
+
+struct string_stream_fragment {
+ struct list_head node;
+ char *fragment;
+};
+
+struct string_stream {
+ size_t length;
+ struct list_head fragments;
+ /* length and fragments are protected by this lock */
+ spinlock_t lock;
+};
+
+struct kunit;
+
+struct string_stream *alloc_string_stream(struct kunit *test);
+
+void string_stream_get(struct string_stream *stream);
+
+int string_stream_put(struct string_stream *stream);
+
+int string_stream_add(struct string_stream *stream, const char *fmt, ...);
+
+int string_stream_vadd(struct string_stream *stream,
+ const char *fmt,
+ va_list args);
+
+char *string_stream_get_string(struct string_stream *stream);
+
+void string_stream_clear(struct string_stream *stream);
+
+bool string_stream_is_empty(struct string_stream *stream);
+
+#endif /* _KUNIT_STRING_STREAM_H */
diff --git a/kunit/Makefile b/kunit/Makefile
index 5efdc4dea2c08..275b565a0e81f 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@@ -1 +1,2 @@
-obj-$(CONFIG_KUNIT) += test.o
+obj-$(CONFIG_KUNIT) += test.o \
+ string-stream.o
diff --git a/kunit/string-stream.c b/kunit/string-stream.c
new file mode 100644
index 0000000000000..0463a92dad74b
--- /dev/null
+++ b/kunit/string-stream.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * C++ stream style string builder used in KUnit for building messages.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins <brendanhiggins@google.com>
+ */
+
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <kunit/string-stream.h>
+#include <kunit/test.h>
+
+int string_stream_vadd(struct string_stream *stream,
+ const char *fmt,
+ va_list args)
+{
+ struct string_stream_fragment *frag_container;
+ int len;
+ va_list args_for_counting;
+ unsigned long flags;
+
+ /* Make a copy because `vsnprintf` could change it */
+ va_copy(args_for_counting, args);
+
+ /* Need space for null byte. */
+ len = vsnprintf(NULL, 0, fmt, args_for_counting) + 1;
+
+ va_end(args_for_counting);
+
+ frag_container = kmalloc(sizeof(*frag_container), GFP_KERNEL);
+ if (!frag_container)
+ return -ENOMEM;
+
+ frag_container->fragment = kmalloc(len, GFP_KERNEL);
+ if (!frag_container->fragment) {
+ kfree(frag_container);
+ return -ENOMEM;
+ }
+
+ len = vsnprintf(frag_container->fragment, len, fmt, args);
+ spin_lock_irqsave(&stream->lock, flags);
+ stream->length += len;
+ list_add_tail(&frag_container->node, &stream->fragments);
+ spin_unlock_irqrestore(&stream->lock, flags);
+
+ return 0;
+}
+
+int string_stream_add(struct string_stream *stream, const char *fmt, ...)
+{
+ va_list args;
+ int result;
+
+ va_start(args, fmt);
+ result = string_stream_vadd(stream, fmt, args);
+ va_end(args);
+
+ return result;
+}
+
+void string_stream_clear(struct string_stream *stream)
+{
+ struct string_stream_fragment *frag_container, *frag_container_safe;
+ unsigned long flags;
+
+ spin_lock_irqsave(&stream->lock, flags);
+ list_for_each_entry_safe(frag_container,
+ frag_container_safe,
+ &stream->fragments,
+ node) {
+ list_del(&frag_container->node);
+ kfree(frag_container->fragment);
+ kfree(frag_container);
+ }
+ stream->length = 0;
+ spin_unlock_irqrestore(&stream->lock, flags);
+}
+
+char *string_stream_get_string(struct string_stream *stream)
+{
+ struct string_stream_fragment *frag_container;
+ size_t buf_len = stream->length + 1; /* +1 for null byte. */
+ char *buf;
+ unsigned long flags;
+
+ buf = kzalloc(buf_len, GFP_KERNEL);
+ if (!buf)
+ return NULL;
+
+ spin_lock_irqsave(&stream->lock, flags);
+ list_for_each_entry(frag_container, &stream->fragments, node)
+ strlcat(buf, frag_container->fragment, buf_len);
+ spin_unlock_irqrestore(&stream->lock, flags);
+
+ return buf;
+}
+
+bool string_stream_is_empty(struct string_stream *stream)
+{
+ bool is_empty;
+ unsigned long flags;
+
+ spin_lock_irqsave(&stream->lock, flags);
+ is_empty = list_empty(&stream->fragments);
+ spin_unlock_irqrestore(&stream->lock, flags);
+
+ return is_empty;
+}
+
+static int string_stream_init(struct kunit_resource *res, void *context)
+{
+ struct string_stream *stream;
+
+ stream = kzalloc(sizeof(*stream), GFP_KERNEL);
+ if (!stream)
+ return -ENOMEM;
+
+ res->allocation = stream;
+ INIT_LIST_HEAD(&stream->fragments);
+ spin_lock_init(&stream->lock);
+
+ return 0;
+}
+
+static void string_stream_free(struct kunit_resource *res)
+{
+ struct string_stream *stream = res->allocation;
+
+ string_stream_clear(stream);
+ kfree(stream);
+}
+
+struct string_stream *alloc_string_stream(struct kunit *test)
+{
+ struct kunit_resource *res;
+
+ res = kunit_alloc_resource(test,
+ string_stream_init,
+ string_stream_free,
+ NULL);
+
+ if (!res)
+ return NULL;
+
+ return res->allocation;
+}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH v5 02/18] kunit: test: add test resource management API
From: Brendan Higgins @ 2019-06-17 8:25 UTC (permalink / raw)
To: frowand.list, gregkh, jpoimboe, keescook, kieran.bingham, mcgrof,
peterz, robh, sboyd, shuah, tytso, yamada.masahiro
Cc: devicetree, dri-devel, kunit-dev, linux-doc, linux-fsdevel,
linux-kbuild, linux-kernel, linux-kselftest, linux-nvdimm,
linux-um, Alexander.Levin, Tim.Bird, amir73il, dan.carpenter,
daniel, jdike, joel, julia.lawall, khilman, knut.omang, logang,
mpe, pmladek, rdunlap, richard, rientjes, rostedt, wfg,
Brendan Higgins
In-Reply-To: <20190617082613.109131-1-brendanhiggins@google.com>
Create a common API for test managed resources like memory and test
objects. A lot of times a test will want to set up infrastructure to be
used in test cases; this could be anything from just wanting to allocate
some memory to setting up a driver stack; this defines facilities for
creating "test resources" which are managed by the test infrastructure
and are automatically cleaned up at the conclusion of the test.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes Since Last Revision:
- Found and fixed bug in resource deallocation logic. Bug was
discovered as a result of making a change suggested by Stephen Boyd.
This does not substantially change how any of the code works
conceptually.
---
include/kunit/test.h | 110 +++++++++++++++++++++++++++++++++++++++++++
kunit/test.c | 95 +++++++++++++++++++++++++++++++++++++
2 files changed, 205 insertions(+)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 8476b3d371cb9..27bd95b6b5523 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -10,6 +10,70 @@
#define _KUNIT_TEST_H
#include <linux/types.h>
+#include <linux/slab.h>
+
+struct kunit_resource;
+
+typedef int (*kunit_resource_init_t)(struct kunit_resource *, void *);
+typedef void (*kunit_resource_free_t)(struct kunit_resource *);
+
+/**
+ * struct kunit_resource - represents a *test managed resource*
+ * @allocation: for the user to store arbitrary data.
+ * @free: a user supplied function to free the resource. Populated by
+ * kunit_alloc_resource().
+ *
+ * Represents a *test managed resource*, a resource which will automatically be
+ * cleaned up at the end of a test case.
+ *
+ * Example:
+ *
+ * .. code-block:: c
+ *
+ * struct kunit_kmalloc_params {
+ * size_t size;
+ * gfp_t gfp;
+ * };
+ *
+ * static int kunit_kmalloc_init(struct kunit_resource *res, void *context)
+ * {
+ * struct kunit_kmalloc_params *params = context;
+ * res->allocation = kmalloc(params->size, params->gfp);
+ *
+ * if (!res->allocation)
+ * return -ENOMEM;
+ *
+ * return 0;
+ * }
+ *
+ * static void kunit_kmalloc_free(struct kunit_resource *res)
+ * {
+ * kfree(res->allocation);
+ * }
+ *
+ * void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp)
+ * {
+ * struct kunit_kmalloc_params params;
+ * struct kunit_resource *res;
+ *
+ * params.size = size;
+ * params.gfp = gfp;
+ *
+ * res = kunit_alloc_resource(test, kunit_kmalloc_init,
+ * kunit_kmalloc_free, ¶ms);
+ * if (res)
+ * return res->allocation;
+ *
+ * return NULL;
+ * }
+ */
+struct kunit_resource {
+ void *allocation;
+ kunit_resource_free_t free;
+
+ /* private: internal use only. */
+ struct list_head node;
+};
struct kunit;
@@ -103,6 +167,7 @@ struct kunit {
const char *name; /* Read only after initialization! */
spinlock_t lock; /* Gaurds all mutable test state. */
bool success; /* Protected by lock. */
+ struct list_head resources; /* Protected by lock. */
};
void kunit_init_test(struct kunit *test, const char *name);
@@ -123,6 +188,51 @@ int kunit_run_tests(struct kunit_module *module);
} \
late_initcall(module_kunit_init##module)
+/**
+ * kunit_alloc_resource() - Allocates a *test managed resource*.
+ * @test: The test context object.
+ * @init: a user supplied function to initialize the resource.
+ * @free: a user supplied function to free the resource.
+ * @context: for the user to pass in arbitrary data to the init function.
+ *
+ * Allocates a *test managed resource*, a resource which will automatically be
+ * cleaned up at the end of a test case. See &struct kunit_resource for an
+ * example.
+ */
+struct kunit_resource *kunit_alloc_resource(struct kunit *test,
+ kunit_resource_init_t init,
+ kunit_resource_free_t free,
+ void *context);
+
+void kunit_free_resource(struct kunit *test, struct kunit_resource *res);
+
+/**
+ * kunit_kmalloc() - Like kmalloc() except the allocation is *test managed*.
+ * @test: The test context object.
+ * @size: The size in bytes of the desired memory.
+ * @gfp: flags passed to underlying kmalloc().
+ *
+ * Just like `kmalloc(...)`, except the allocation is managed by the test case
+ * and is automatically cleaned up after the test case concludes. See &struct
+ * kunit_resource for more information.
+ */
+void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp);
+
+/**
+ * kunit_kzalloc() - Just like kunit_kmalloc(), but zeroes the allocation.
+ * @test: The test context object.
+ * @size: The size in bytes of the desired memory.
+ * @gfp: flags passed to underlying kmalloc().
+ *
+ * See kzalloc() and kunit_kmalloc() for more information.
+ */
+static inline void *kunit_kzalloc(struct kunit *test, size_t size, gfp_t gfp)
+{
+ return kunit_kmalloc(test, size, gfp | __GFP_ZERO);
+}
+
+void kunit_cleanup(struct kunit *test);
+
void __printf(3, 4) kunit_printk(const char *level,
const struct kunit *test,
const char *fmt, ...);
diff --git a/kunit/test.c b/kunit/test.c
index d05d254f1521f..53838f5394303 100644
--- a/kunit/test.c
+++ b/kunit/test.c
@@ -142,6 +142,7 @@ static void kunit_print_test_case_ok_not_ok(struct kunit_case *test_case,
void kunit_init_test(struct kunit *test, const char *name)
{
spin_lock_init(&test->lock);
+ INIT_LIST_HEAD(&test->resources);
test->name = name;
test->success = true;
}
@@ -172,6 +173,8 @@ static void kunit_run_case(struct kunit_module *module,
if (module->exit)
module->exit(&test);
+ kunit_cleanup(&test);
+
test_case->success = kunit_get_success(&test);
}
@@ -192,6 +195,98 @@ int kunit_run_tests(struct kunit_module *module)
return 0;
}
+struct kunit_resource *kunit_alloc_resource(struct kunit *test,
+ kunit_resource_init_t init,
+ kunit_resource_free_t free,
+ void *context)
+{
+ struct kunit_resource *res;
+ unsigned long flags;
+ int ret;
+
+ res = kzalloc(sizeof(*res), GFP_KERNEL);
+ if (!res)
+ return NULL;
+
+ ret = init(res, context);
+ if (ret)
+ return NULL;
+
+ res->free = free;
+ spin_lock_irqsave(&test->lock, flags);
+ list_add_tail(&res->node, &test->resources);
+ spin_unlock_irqrestore(&test->lock, flags);
+
+ return res;
+}
+
+void kunit_free_resource(struct kunit *test, struct kunit_resource *res)
+{
+ res->free(res);
+ list_del(&res->node);
+ kfree(res);
+}
+
+struct kunit_kmalloc_params {
+ size_t size;
+ gfp_t gfp;
+};
+
+static int kunit_kmalloc_init(struct kunit_resource *res, void *context)
+{
+ struct kunit_kmalloc_params *params = context;
+
+ res->allocation = kmalloc(params->size, params->gfp);
+ if (!res->allocation)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void kunit_kmalloc_free(struct kunit_resource *res)
+{
+ kfree(res->allocation);
+}
+
+void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp)
+{
+ struct kunit_kmalloc_params params;
+ struct kunit_resource *res;
+
+ params.size = size;
+ params.gfp = gfp;
+
+ res = kunit_alloc_resource(test,
+ kunit_kmalloc_init,
+ kunit_kmalloc_free,
+ ¶ms);
+
+ if (res)
+ return res->allocation;
+
+ return NULL;
+}
+
+void kunit_cleanup(struct kunit *test)
+{
+ struct kunit_resource *resource, *resource_safe;
+ unsigned long flags;
+
+ spin_lock_irqsave(&test->lock, flags);
+ /*
+ * test->resources is a stack - each allocation must be freed in the
+ * reverse order from which it was added since one resource may depend
+ * on another for its entire lifetime.
+ */
+ list_for_each_entry_safe_reverse(resource,
+ resource_safe,
+ &test->resources,
+ node) {
+ kunit_free_resource(test, resource);
+ }
+ spin_unlock_irqrestore(&test->lock, flags);
+}
+
void kunit_printk(const char *level,
const struct kunit *test,
const char *fmt, ...)
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox