Re: [PATCH v2 1/2] get_maintainer: add patch-only keyword-matching
On Thu, 2023-09-28 at 14:03 +0900, Justin Stitt wrote: > On Thu, Sep 28, 2023 at 1:46 PM Joe Perches wrote: > > > > On Thu, 2023-09-28 at 04:23 +, Justin Stitt wrote: > > > Add the "D:" type which behaves the same as "K:" but will only match > > > content present in a patch file. [] > > > My opinion: Nack. > > > > I think something like this would be better > > as it avoids duplication of K and D content. > > If I understand correctly, this puts the onus on the get_maintainer users > to select the right argument whereas adding "D:", albeit with some > duplicate code, allows maintainers themselves to decide in exactly > which context they receive mail. Maybe, but I doubt it'll be significantly different. > This could all be a moot point, though, as I believe Konstantin > is trying to separate out the whole idea of a patch-sender needing > to specify the recipients of a patch. As I understand it, by using get_maintainer.
Re: [PATCH v2 1/2] get_maintainer: add patch-only keyword-matching
On Thu, Sep 28, 2023 at 1:46 PM Joe Perches wrote:
>
> On Thu, 2023-09-28 at 04:23 +, Justin Stitt wrote:
> > Add the "D:" type which behaves the same as "K:" but will only match
> > content present in a patch file.
> >
> > To illustrate:
> >
> > Imagine this entry in MAINTAINERS:
> >
> > NEW REPUBLIC
> > M: Han Solo
> > W: https://www.jointheresistance.org
> > D: \bstrncpy\b
> >
> > Our maintainer, Han, will only be added to the recipients if a patch
> > file is passed to get_maintainer (like what b4 does):
> > $ ./scripts/get_maintainer.pl 0004-some-change.patch
> >
> > If the above patch has a `strncpy` present in the subject, commit log or
> > diff then Han will be to/cc'd.
> >
> > However, in the event of a file from the tree given like:
> > $ ./scripts/get_maintainer.pl ./lib/string.c
> >
> > Han will not be noisily to/cc'd (like a K: type would in this
> > circumstance)
> >
> > Signed-off-by: Justin Stitt
> > ---
> > MAINTAINERS | 5 +
> > scripts/get_maintainer.pl | 12 ++--
> > 2 files changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index b19995690904..94e431daa7c2 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -59,6 +59,11 @@ Descriptions of section entries and preferred order
> > matches patches or files that contain one or more of the words
> > printk, pr_info or pr_err
> > One regex pattern per line. Multiple K: lines acceptable.
> > + D: *Diff content regex* (perl extended) pattern match that applies only
> > to
> > + patches and not entire files (e.g. when using the get_maintainers.pl
> > + script).
> > + Usage same as K:.
> > +
> >
> > Maintainers List
> >
> > diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> > index ab123b498fd9..a3e697926ddd 100755
> > --- a/scripts/get_maintainer.pl
> > +++ b/scripts/get_maintainer.pl
> > @@ -342,6 +342,7 @@ if ($tree && !top_of_kernel_tree($lk_path)) {
> >
> > my @typevalue = ();
> > my %keyword_hash;
> > +my %patch_keyword_hash;
> > my @mfiles = ();
> > my @self_test_info = ();
> >
> > @@ -369,8 +370,10 @@ sub read_maintainer_file {
> > $value =~ s@([^/])$@$1/@;
> > }
> > } elsif ($type eq "K") {
> > - $keyword_hash{@typevalue} = $value;
> > - }
> > + $keyword_hash{@typevalue} = $value;
> > + } elsif ($type eq "D") {
> > + $patch_keyword_hash{@typevalue} = $value;
> > + }
> > push(@typevalue, "$type:$value");
> > } elsif (!(/^\s*$/ || /^\s*\#/)) {
> > push(@typevalue, $line);
> > @@ -607,6 +610,11 @@ foreach my $file (@ARGV) {
> > push(@keyword_tvi, $line);
> > }
> > }
> > +foreach my $line(keys %patch_keyword_hash) {
> > + if ($patch_line =~ m/${patch_prefix}$patch_keyword_hash{$line}/x) {
> > +push(@keyword_tvi, $line);
> > + }
> > +}
> > }
> > }
> > close($patch);
> >
>
>
> My opinion: Nack.
>
> I think something like this would be better
> as it avoids duplication of K and D content.
If I understand correctly, this puts the onus on the get_maintainer users
to select the right argument whereas adding "D:", albeit with some
duplicate code, allows maintainers themselves to decide in exactly
which context they receive mail.
Adding a command line flag means sometimes K: is treated one
way and sometimes treated a different way depending on
the specific incantation.
This could all be a moot point, though, as I believe Konstantin
is trying to separate out the whole idea of a patch-sender needing
to specify the recipients of a patch. Instead some middleware would
capture mail and delegate automatically based on some queries
set up by maintainers.
Exciting idea, I wonder what the progress is on that?
Cc: Konstantin Ryabitsev
[1]: https://lore.kernel.org/all/20230726-june-mocha-ad6809@meerkat/
> ---
> scripts/get_maintainer.pl | 16 +---
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index ab123b498fd9..07e7d744cadb 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -57,6 +57,7 @@ my $subsystem = 0;
> my $status = 0;
> my $letters = "";
> my $keywords = 1;
> +my $keywords_in_file = 0;
> my $sections = 0;
> my $email_file_emails = 0;
> my $from_filename = 0;
> @@ -272,6 +273,7 @@ if (!GetOptions(
> 'letters=s' => \$letters,
> 'pattern-depth=i' => \$pattern_depth,
> 'k|keywords!' => \$keywords,
> + 'kf|keywords-in-file!' => \$keywords_in_file,
> 'sections!' => \$sections,
> 'fe|file-emails!' => \$email_file_emails,
> 'f|file' => \$from_filename,
> @@ -318,6 +320,7 @@ if ($sections || $letters ne "") {
> $subsystem = 0;
>
Re: [PATCH v2 0/2] get_maintainer: add patch-only keyword matching
On Thu, 2023-09-28 at 04:23 +, Justin Stitt wrote: > Changes in v2: > - remove formatting pass (thanks Joe) (but seriously the formatting is > bad, is there opportunity to get a formatting pass in here at some > point?) Why? What is it that makes you believe the formatting is bad?
Re: [PATCH v2 2/2] MAINTAINERS: migrate some K to D
On Thu, 2023-09-28 at 04:23 +, Justin Stitt wrote: > Let's get the ball rolling with some changes from K to D. > > Ultimately, if it turns out that 100% of K users want to change to D > then really the behavior of K could just be changed. Given my suggestion to 1/2, this would be unnecessary. > > Signed-off-by: Justin Stitt > Original-author: Kees Cook > --- > MAINTAINERS | 16 +--- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 94e431daa7c2..80ffdaa8f044 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -5038,7 +5038,7 @@ F: Documentation/kbuild/llvm.rst > F: include/linux/compiler-clang.h > F: scripts/Makefile.clang > F: scripts/clang-tools/ > -K: \b(?i:clang|llvm)\b > +D: \b(?i:clang|llvm)\b > > CLK API > M: Russell King > @@ -8149,7 +8149,7 @@ F: lib/strcat_kunit.c > F: lib/strscpy_kunit.c > F: lib/test_fortify/* > F: scripts/test_fortify.sh > -K: \b__NO_FORTIFY\b > +D: \b__NO_FORTIFY\b > > FPGA DFL DRIVERS > M: Wu Hao > @@ -11405,8 +11405,10 @@ F: > Documentation/ABI/testing/sysfs-kernel-warn_count > F: include/linux/overflow.h > F: include/linux/randomize_kstack.h > F: mm/usercopy.c > -K: \b(add|choose)_random_kstack_offset\b > -K: \b__check_(object_size|heap_object)\b > +D: \b(add|choose)_random_kstack_offset\b > +D: \b__check_(object_size|heap_object)\b > +D: \b__counted_by\b > + > > KERNEL JANITORS > L: kernel-janit...@vger.kernel.org > @@ -17290,7 +17292,7 @@ F:drivers/acpi/apei/erst.c > F: drivers/firmware/efi/efi-pstore.c > F: fs/pstore/ > F: include/linux/pstore* > -K: \b(pstore|ramoops) > +D: \b(pstore|ramoops) > > PTP HARDWARE CLOCK SUPPORT > M: Richard Cochran > @@ -19231,8 +19233,8 @@ F:include/uapi/linux/seccomp.h > F: kernel/seccomp.c > F: tools/testing/selftests/kselftest_harness.h > F: tools/testing/selftests/seccomp/* > -K: \bsecure_computing > -K: \bTIF_SECCOMP\b > +D: \bsecure_computing > +D: \bTIF_SECCOMP\b > > SECURE DIGITAL HOST CONTROLLER INTERFACE (SDHCI) Broadcom BRCMSTB DRIVER > M: Kamal Dasu >
Re: [PATCH v2 1/2] get_maintainer: add patch-only keyword-matching
On Thu, 2023-09-28 at 04:23 +, Justin Stitt wrote:
> Add the "D:" type which behaves the same as "K:" but will only match
> content present in a patch file.
>
> To illustrate:
>
> Imagine this entry in MAINTAINERS:
>
> NEW REPUBLIC
> M: Han Solo
> W: https://www.jointheresistance.org
> D: \bstrncpy\b
>
> Our maintainer, Han, will only be added to the recipients if a patch
> file is passed to get_maintainer (like what b4 does):
> $ ./scripts/get_maintainer.pl 0004-some-change.patch
>
> If the above patch has a `strncpy` present in the subject, commit log or
> diff then Han will be to/cc'd.
>
> However, in the event of a file from the tree given like:
> $ ./scripts/get_maintainer.pl ./lib/string.c
>
> Han will not be noisily to/cc'd (like a K: type would in this
> circumstance)
>
> Signed-off-by: Justin Stitt
> ---
> MAINTAINERS | 5 +
> scripts/get_maintainer.pl | 12 ++--
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b19995690904..94e431daa7c2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -59,6 +59,11 @@ Descriptions of section entries and preferred order
> matches patches or files that contain one or more of the words
> printk, pr_info or pr_err
> One regex pattern per line. Multiple K: lines acceptable.
> + D: *Diff content regex* (perl extended) pattern match that applies only to
> + patches and not entire files (e.g. when using the get_maintainers.pl
> + script).
> + Usage same as K:.
> +
>
> Maintainers List
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index ab123b498fd9..a3e697926ddd 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -342,6 +342,7 @@ if ($tree && !top_of_kernel_tree($lk_path)) {
>
> my @typevalue = ();
> my %keyword_hash;
> +my %patch_keyword_hash;
> my @mfiles = ();
> my @self_test_info = ();
>
> @@ -369,8 +370,10 @@ sub read_maintainer_file {
> $value =~ s@([^/])$@$1/@;
> }
> } elsif ($type eq "K") {
> - $keyword_hash{@typevalue} = $value;
> - }
> + $keyword_hash{@typevalue} = $value;
> + } elsif ($type eq "D") {
> + $patch_keyword_hash{@typevalue} = $value;
> + }
> push(@typevalue, "$type:$value");
> } elsif (!(/^\s*$/ || /^\s*\#/)) {
> push(@typevalue, $line);
> @@ -607,6 +610,11 @@ foreach my $file (@ARGV) {
> push(@keyword_tvi, $line);
> }
> }
> +foreach my $line(keys %patch_keyword_hash) {
> + if ($patch_line =~ m/${patch_prefix}$patch_keyword_hash{$line}/x) {
> +push(@keyword_tvi, $line);
> + }
> +}
> }
> }
> close($patch);
>
My opinion: Nack.
I think something like this would be better
as it avoids duplication of K and D content.
---
scripts/get_maintainer.pl | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index ab123b498fd9..07e7d744cadb 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -57,6 +57,7 @@ my $subsystem = 0;
my $status = 0;
my $letters = "";
my $keywords = 1;
+my $keywords_in_file = 0;
my $sections = 0;
my $email_file_emails = 0;
my $from_filename = 0;
@@ -272,6 +273,7 @@ if (!GetOptions(
'letters=s' => \$letters,
'pattern-depth=i' => \$pattern_depth,
'k|keywords!' => \$keywords,
+ 'kf|keywords-in-file!' => \$keywords_in_file,
'sections!' => \$sections,
'fe|file-emails!' => \$email_file_emails,
'f|file' => \$from_filename,
@@ -318,6 +320,7 @@ if ($sections || $letters ne "") {
$subsystem = 0;
$web = 0;
$keywords = 0;
+$keywords_in_file = 0;
$interactive = 0;
} else {
my $selections = $email + $scm + $status + $subsystem + $web;
@@ -548,16 +551,14 @@ foreach my $file (@ARGV) {
$file =~ s/^\Q${cur_path}\E//; #strip any absolute path
$file =~ s/^\Q${lk_path}\E//; #or the path to the lk tree
push(@files, $file);
- if ($file ne "MAINTAINERS" && -f $file && $keywords) {
+ if ($file ne "MAINTAINERS" && -f $file && $keywords &&
$keywords_in_file) {
open(my $f, '<', $file)
or die "$P: Can't open $file: $!\n";
my $text = do { local($/) ; <$f> };
close($f);
- if ($keywords) {
- foreach my $line (keys %keyword_hash) {
- if ($text =~ m/$keyword_hash{$line}/x) {
- push(@keyword_tvi, $line);
- }
+ foreach my $line (keys %keyword_hash) {
+ if ($text =~ m/$keyword_hash{$line}/x) {
+ push(@keyword_tvi, $line);
}
}
}
@@
[PATCH v2 2/2] MAINTAINERS: migrate some K to D
Let's get the ball rolling with some changes from K to D. Ultimately, if it turns out that 100% of K users want to change to D then really the behavior of K could just be changed. Signed-off-by: Justin Stitt Original-author: Kees Cook --- MAINTAINERS | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 94e431daa7c2..80ffdaa8f044 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5038,7 +5038,7 @@ F:Documentation/kbuild/llvm.rst F: include/linux/compiler-clang.h F: scripts/Makefile.clang F: scripts/clang-tools/ -K: \b(?i:clang|llvm)\b +D: \b(?i:clang|llvm)\b CLK API M: Russell King @@ -8149,7 +8149,7 @@ F:lib/strcat_kunit.c F: lib/strscpy_kunit.c F: lib/test_fortify/* F: scripts/test_fortify.sh -K: \b__NO_FORTIFY\b +D: \b__NO_FORTIFY\b FPGA DFL DRIVERS M: Wu Hao @@ -11405,8 +11405,10 @@ F: Documentation/ABI/testing/sysfs-kernel-warn_count F: include/linux/overflow.h F: include/linux/randomize_kstack.h F: mm/usercopy.c -K: \b(add|choose)_random_kstack_offset\b -K: \b__check_(object_size|heap_object)\b +D: \b(add|choose)_random_kstack_offset\b +D: \b__check_(object_size|heap_object)\b +D: \b__counted_by\b + KERNEL JANITORS L: kernel-janit...@vger.kernel.org @@ -17290,7 +17292,7 @@ F: drivers/acpi/apei/erst.c F: drivers/firmware/efi/efi-pstore.c F: fs/pstore/ F: include/linux/pstore* -K: \b(pstore|ramoops) +D: \b(pstore|ramoops) PTP HARDWARE CLOCK SUPPORT M: Richard Cochran @@ -19231,8 +19233,8 @@ F: include/uapi/linux/seccomp.h F: kernel/seccomp.c F: tools/testing/selftests/kselftest_harness.h F: tools/testing/selftests/seccomp/* -K: \bsecure_computing -K: \bTIF_SECCOMP\b +D: \bsecure_computing +D: \bTIF_SECCOMP\b SECURE DIGITAL HOST CONTROLLER INTERFACE (SDHCI) Broadcom BRCMSTB DRIVER M: Kamal Dasu -- 2.42.0.582.g8ccd20d70d-goog
[PATCH v2 1/2] get_maintainer: add patch-only keyword-matching
Add the "D:" type which behaves the same as "K:" but will only match
content present in a patch file.
To illustrate:
Imagine this entry in MAINTAINERS:
NEW REPUBLIC
M: Han Solo
W: https://www.jointheresistance.org
D: \bstrncpy\b
Our maintainer, Han, will only be added to the recipients if a patch
file is passed to get_maintainer (like what b4 does):
$ ./scripts/get_maintainer.pl 0004-some-change.patch
If the above patch has a `strncpy` present in the subject, commit log or
diff then Han will be to/cc'd.
However, in the event of a file from the tree given like:
$ ./scripts/get_maintainer.pl ./lib/string.c
Han will not be noisily to/cc'd (like a K: type would in this
circumstance)
Signed-off-by: Justin Stitt
---
MAINTAINERS | 5 +
scripts/get_maintainer.pl | 12 ++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index b19995690904..94e431daa7c2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -59,6 +59,11 @@ Descriptions of section entries and preferred order
matches patches or files that contain one or more of the words
printk, pr_info or pr_err
One regex pattern per line. Multiple K: lines acceptable.
+ D: *Diff content regex* (perl extended) pattern match that applies only to
+ patches and not entire files (e.g. when using the get_maintainers.pl
+ script).
+ Usage same as K:.
+
Maintainers List
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index ab123b498fd9..a3e697926ddd 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -342,6 +342,7 @@ if ($tree && !top_of_kernel_tree($lk_path)) {
my @typevalue = ();
my %keyword_hash;
+my %patch_keyword_hash;
my @mfiles = ();
my @self_test_info = ();
@@ -369,8 +370,10 @@ sub read_maintainer_file {
$value =~ s@([^/])$@$1/@;
}
} elsif ($type eq "K") {
- $keyword_hash{@typevalue} = $value;
- }
+ $keyword_hash{@typevalue} = $value;
+ } elsif ($type eq "D") {
+ $patch_keyword_hash{@typevalue} = $value;
+ }
push(@typevalue, "$type:$value");
} elsif (!(/^\s*$/ || /^\s*\#/)) {
push(@typevalue, $line);
@@ -607,6 +610,11 @@ foreach my $file (@ARGV) {
push(@keyword_tvi, $line);
}
}
+foreach my $line(keys %patch_keyword_hash) {
+ if ($patch_line =~ m/${patch_prefix}$patch_keyword_hash{$line}/x) {
+push(@keyword_tvi, $line);
+ }
+}
}
}
close($patch);
--
2.42.0.582.g8ccd20d70d-goog
[PATCH v2 0/2] get_maintainer: add patch-only keyword matching
This series aims to add "D:" which behaves exactly the same as "K:" but works only on patch files. The goal of this is to reduce noise when folks use get_maintainer on tree files as opposed to patches. "D:" should help maintainers reduce noise in their inboxes, especially when matching omnipresent keywords like [1]. In the event of [1] Kees would be to/cc'd from folks running get_maintainer on _any_ file containing "__counted_by". The number of these files is rising and I fear for his inbox as his goal, as I understand it, is to simply monitor the introduction of new __counted_by annotations to ensure accurate semantics. Joe mentioned in v1 that perhaps K: should be reworked to only consider patch files. I wonder, though, if folks are intentionally using the current behavior of K: and thus would be peeved from a change there. I see this series as, at the very least, a gentle migration in behavior which is purely opt-in and at some point could eagerly be merged with K:. [1]: https://lore.kernel.org/all/20230925172037.work.853-k...@kernel.org/ Signed-off-by: Justin Stitt --- Changes in v2: - remove bits about non-patch usage being bad (thanks Greg, Kees, et al.) - remove formatting pass (thanks Joe) (but seriously the formatting is bad, is there opportunity to get a formatting pass in here at some point?) - add some migration from K to D (thanks Kees, Nick) - Link to v1: https://lore.kernel.org/r/20230927-get_maintainer_add_d-v1-0-28c207229...@google.com --- Justin Stitt (2): get_maintainer: add patch-only keyword-matching MAINTAINERS: migrate some K to D MAINTAINERS | 21 ++--- scripts/get_maintainer.pl | 12 ++-- 2 files changed, 24 insertions(+), 9 deletions(-) --- base-commit: 6465e260f48790807eef06b583b38ca9789b6072 change-id: 20230926-get_maintainer_add_d-07424a814e72 Best regards, -- Justin Stitt
Re: [PATCH v5 02/18] cgroup/misc: Add SGX EPC resource type and export APIs for SGX driver
On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote:
> From: Kristen Carlson Accardi
>
> Add SGX EPC memory, MISC_CG_RES_SGX_EPC, to be a valid resource type
> for the misc controller.
>
> Add per resource type private data so that SGX can store additional per
> cgroup data in misc_cg->misc_cg_res[MISC_CG_RES_SGX_EPC].
To be honest I don't quite understand why putting the above two changes in this
patch together with exporting misc_cg_root/parent() below.
Any reason why the above two cannot be done together with patch (" x86/sgx:
Limit process EPC usage with misc cgroup controller"), where these changes are
actually related?
We all already know that a new EPC misc cgroup will be added. There's no need
to actually introduce the new type here only to justify exporting some helper
functions.
>
> Export misc_cg_root() so the SGX driver can initialize and add those
> additional structures to the root misc cgroup as part of initialization
> for EPC cgroup support. This bootstraps the same additional
> initialization for non-root cgroups in the 'alloc()' callback added in the
> previous patch.
>
> The SGX driver, as the EPC memory provider, will have a background
> worker to reclaim EPC pages to make room for new allocations in the same
> cgroup when its usage counter reaches near the limit controlled by the
> cgroup and its ancestors. Therefore it needs to do a walk from the
> current cgroup up to the root. To enable this walk, move parent_misc()
> into misc_cgroup.h and make inline to make this function available to
> SGX, rename it to misc_cg_parent(), and update kernel/cgroup/misc.c to
> use the new name.
Looks too many details in the above two paragraphs. Could we have a more
concise justification for exporting these two functions?
And if it were me, I would put it at a relatively later position (e.g., before
the patch actually implements EPC cgroup) for better review. This also applies
to the first patch.
>
> Signed-off-by: Kristen Carlson Accardi
> Signed-off-by: Haitao Huang
> ---
> V5:
> - Revised commit message (Jarkko)
>
> V4:
> - Moved this to the second in the series.
> ---
> include/linux/misc_cgroup.h | 29 +
> kernel/cgroup/misc.c| 25 -
> 2 files changed, 41 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/misc_cgroup.h b/include/linux/misc_cgroup.h
> index 96a88822815a..87f29f8597e1 100644
> --- a/include/linux/misc_cgroup.h
> +++ b/include/linux/misc_cgroup.h
> @@ -17,6 +17,10 @@ enum misc_res_type {
> MISC_CG_RES_SEV,
> /* AMD SEV-ES ASIDs resource */
> MISC_CG_RES_SEV_ES,
> +#endif
> +#ifdef CONFIG_CGROUP_SGX_EPC
> + /* SGX EPC memory resource */
> + MISC_CG_RES_SGX_EPC,
> #endif
> MISC_CG_RES_TYPES
> };
> @@ -37,6 +41,7 @@ struct misc_res {
> u64 max;
> atomic64_t usage;
> atomic64_t events;
> + void *priv;
>
> /* per resource callback ops */
> int (*alloc)(struct misc_cg *cg);
> @@ -59,6 +64,7 @@ struct misc_cg {
> struct misc_res res[MISC_CG_RES_TYPES];
> };
>
> +struct misc_cg *misc_cg_root(void);
> u64 misc_cg_res_total_usage(enum misc_res_type type);
> int misc_cg_set_capacity(enum misc_res_type type, u64 capacity);
> int misc_cg_try_charge(enum misc_res_type type, struct misc_cg *cg, u64
> amount);
> @@ -78,6 +84,20 @@ static inline struct misc_cg *css_misc(struct
> cgroup_subsys_state *css)
> return css ? container_of(css, struct misc_cg, css) : NULL;
> }
>
> +/**
> + * misc_cg_parent() - Get the parent of the passed misc cgroup.
> + * @cgroup: cgroup whose parent needs to be fetched.
> + *
> + * Context: Any context.
> + * Return:
> + * * struct misc_cg* - Parent of the @cgroup.
> + * * %NULL - If @cgroup is null or the passed cgroup does not have a parent.
> + */
> +static inline struct misc_cg *misc_cg_parent(struct misc_cg *cgroup)
> +{
> + return cgroup ? css_misc(cgroup->css.parent) : NULL;
> +}
> +
> /*
> * get_current_misc_cg() - Find and get the misc cgroup of the current task.
> *
> @@ -102,6 +122,15 @@ static inline void put_misc_cg(struct misc_cg *cg)
> }
>
> #else /* !CONFIG_CGROUP_MISC */
> +static inline struct misc_cg *misc_cg_root(void)
> +{
> + return NULL;
> +}
> +
> +static inline struct misc_cg *misc_cg_parent(struct misc_cg *cg)
> +{
> + return NULL;
> +}
>
> static inline u64 misc_cg_res_total_usage(enum misc_res_type type)
> {
> diff --git a/kernel/cgroup/misc.c b/kernel/cgroup/misc.c
> index 62c9198dee21..4633b8629e63 100644
> --- a/kernel/cgroup/misc.c
> +++ b/kernel/cgroup/misc.c
> @@ -24,6 +24,10 @@ static const char *const misc_res_name[] = {
> /* AMD SEV-ES ASIDs resource */
> "sev_es",
> #endif
> +#ifdef CONFIG_CGROUP_SGX_EPC
> + /* Intel SGX EPC memory bytes */
> + "sgx_epc",
> +#endif
> };
>
> /* Root misc cgroup */
> @@ -40,18 +44,13 @@ static struct misc_cg root_cg;
> static u64
Re: [PATCH] ARM: dts: qcom: apq8026-samsung-matisse-wifi: Fix inverted hall sensor
On Fri, 22 Sep 2023 04:12:11 +0300, Matti Lehtimäki wrote: > Fix hall sensor GPIO polarity and also allow disabling the sensor. > Remove unneeded interrupt. > > Applied, thanks! [1/1] ARM: dts: qcom: apq8026-samsung-matisse-wifi: Fix inverted hall sensor commit: 0b73519790d29e4bc71afc4882a9aa9ea649bcf7 Best regards, -- Bjorn Andersson
Re: [PATCH 0/2] Add rpm-master-stats nodes for MSM8226 and MSM8974
On Fri, 22 Sep 2023 03:35:31 +0300, Matti Lehtimäki wrote: > Add rpm-master-stats nodes and the required RPM MSG RAM slices > for MSM8226 and MSM8974. > > Matti Lehtimäki (2): > ARM: qcom: msm8226: Add rpm-master-stats node > ARM: qcom: msm8974: Add rpm-master-stats node > > [...] Applied, thanks! [1/2] ARM: qcom: msm8226: Add rpm-master-stats node commit: bd837be0ff3879209df6fb85cf9e22fd1ba7f79b [2/2] ARM: qcom: msm8974: Add rpm-master-stats node commit: 02c58ac774a03ffefd3708f9c17ea4d911e0ade7 Best regards, -- Bjorn Andersson
Re: [PATCH v3 1/1] scripts: Add add-maintainer.py
On Sep 26 2023 17:32, Pavan Kondeti wrote:
> On Sat, Aug 26, 2023 at 01:07:42AM -0700, Guru Das Srinagesh wrote:
> > +def gather_maintainers_of_file(patch_file):
> > +all_entities_of_patch = dict()
> > +
> > +# Run get_maintainer.pl on patch file
> > +logging.info("GET: Patch: {}".format(os.path.basename(patch_file)))
> > +cmd = ['scripts/get_maintainer.pl']
> > +cmd.extend([patch_file])
> > +
> > +try:
> > +p = subprocess.run(cmd, stdout=subprocess.PIPE, check=True)
> > +except:
> > +sys.exit(1)
> > +
> > +logging.debug("\n{}".format(p.stdout.decode()))
> > +
> > +entries = p.stdout.decode().splitlines()
> > +
> > +maintainers = []
> > +lists = []
> > +others = []
> > +
> > +for entry in entries:
> > +entity = entry.split('(')[0].strip()
> > +if any(role in entry for role in ["maintainer", "reviewer"]):
> > +maintainers.append(entity)
> > +elif "list" in entry:
> > +lists.append(entity)
> > +else:
> > +others.append(entity)
> > +
> > +all_entities_of_patch["maintainers"] = set(maintainers)
> > +all_entities_of_patch["lists"] = set(lists)
> > +all_entities_of_patch["others"] = set(others)
> > +
> > +return all_entities_of_patch
> > +
>
> FYI, there are couple of issues found while playing around.
Thanks for testing this out :) I am no longer working on this due to pushback
from the maintainers in favour of b4.
>
> - Some entries in MAINTAINERS could be "supporter"
This was intentional - I didn't want to include "supporter"s.
> - When names contain ("company"), the script fails to extract name.
Interesting... I had not tested this out.
In any case, I am not devoting resources to work on this unless I see some
interest from maintainers, which, as it stands currently, is nil.
Thanks for the support, Pavan.
Guru Das.
Re: [PATCH v3 1/1] scripts: Add add-maintainer.py
On Sep 27 2023 10:21, Pavan Kondeti wrote: > On Mon, Aug 28, 2023 at 10:21:32AM +0200, Krzysztof Kozlowski wrote: > > On 26/08/2023 10:07, Guru Das Srinagesh wrote: > > > This script runs get_maintainer.py on a given patch file (or multiple > > > patch files) and adds its output to the patch file in place with the > > > appropriate email headers "To: " or "Cc: " as the case may be. These new > > > headers are added after the "From: " line in the patch. > > > > > > Currently, for a single patch, maintainers and reviewers are added as > > > "To: ", mailing lists and all other roles are added as "Cc: ". > > > > > > For a series of patches, however, a set-union scheme is employed in > > > order to solve the all-too-common problem of ending up sending only > > > subsets of a patch series to some lists, which results in important > > > pieces of context such as the cover letter (or other patches in the > > > series) being dropped from those lists. This scheme is as follows: > > > > > > - Create set-union of all maintainers and reviewers from all patches and > > > use this to do the following per patch: > > > - add only that specific patch's maintainers and reviewers as "To: " > > > - add the other maintainers and reviewers from the other patches as > > > "Cc: " > > > > > > - Create set-union of all mailing lists corresponding to all patches and > > > add this to all patches as "Cc: " > > > > > > - Create set-union of all other roles corresponding to all patches and > > > add this to all patches as "Cc: " > > > > > > Please note that patch files that don't have any "Maintainer"s or > > > "Reviewers" explicitly listed in their `get_maintainer.pl` output will > > > > So before you will ignoring the reviewers, right? One more reason to not > > get it right... > > > > > not have any "To: " entries added to them; developers are expected to > > > manually make edits to the added entries in such cases to convert some > > > "Cc: " entries to "To: " as desired. > > > > > > The script is quiet by default (only prints errors) and its verbosity > > > can be adjusted via an optional parameter. > > > > > > Signed-off-by: Guru Das Srinagesh > > > --- > > > MAINTAINERS | 5 ++ > > > scripts/add-maintainer.py | 164 ++ > > > 2 files changed, 169 insertions(+) > > > create mode 100755 scripts/add-maintainer.py > > > > > > > I do not see the benefits of this script. For me - it's unnecessarily > > more complicated instead of my simple bash function which makes > > everything one command less than here. > > One more thing to maintain. > > Thanks for your bash script. I slightly modified it to keep maintainers > in To and rest in Cc. > > git send-email --dry-run --to="$(scripts/get_maintainer.pl --no-multiline > --separator=, --no-r --no-l --no-git --no-roles --no-rolestats > --no-git-fallback *.patch)" --cc="$(scripts/get_maintainer.pl --no-multiline > --separator=, --no-m --no-git --no-roles --no-rolestats --no-git-fallback > *.patch)" *.patch FYI, b4 has "b4.send-auto-to-cmd" and "b4.send-auto-cc-cmd" [1] as well to override its default behaviour. You can just set the above two b4 config options as you like and it will do the right thing. Guru Das.
Re: [PATCH v5 09/18] x86/sgx: Store struct sgx_encl when allocating new VA pages
On Wed, 2023-09-27 at 10:35 -0500, Haitao Huang wrote:
> > > +
> > > + /* Possible owner types */
> > > + union {
> > > + struct sgx_encl_page *encl_page;
> > > + struct sgx_encl *encl;
> > > + };
> >
> > Sadly for virtual EPC page the owner is set to the 'sgx_vepc' instance it
> > belongs to.
> >
> > Given how sgx_{alloc|free}_epc_page() arbitrarily uses encl_page,
> > perhaps we
> > should do below?
> >
> > union {
> > struct sgx_encl_page *encl_page;
> > struct sgx_encl *encl;
> > struct sgx_vepc *vepc;
> > void *owner;
> > };
> >
> > And in sgx_{alloc|free}_epc_page() we can use 'owner' instead.
> >
>
> As I mentioned in cover letter and change log in 11/18, this series does
> not track virtual EPC.
It's not about how does the cover letter says. We cannot ignore the fact that
currently virtual EPC uses owner too.
But given the virtual EPC code currently doesn't use the owner, I can live with
not having the 'vepc' member in the union now.
> We can add vepc field into the union in future if such tracking is needed.
> Don't think "void *owner" is needed though.
As mentioned, using 'encl_page' arbitrarily in sgx_alloc_epc_page() doesn't look
nice. Do you have example in the current kernel code to prove it is acceptable?
Re: [PATCH 3/3] get_maintainer: add patch-only pattern matching type
On Wed, 2023-09-27 at 09:15 -0700, Kees Cook wrote: > On Wed, Sep 27, 2023 at 03:19:16AM +, Justin Stitt wrote: > > Add the "D:" type which behaves the same as "K:" but will only match > > content present in a patch file. > > > > To illustrate: > > > > Imagine this entry in MAINTAINERS: > > > > NEW REPUBLIC > > M: Han Solo > > W: https://www.jointheresistance.org > > D: \bstrncpy\b > > > > Our maintainer, Han, will only be added to the recipients if a patch > > file is passed to get_maintainer (like what b4 does): > > $ ./scripts/get_maintainer.pl 0004-some-change.patch > > > > If the above patch has a `strncpy` present in the subject, commit log or > > diff then Han will be to/cc'd. > > > > However, in the event of a file from the tree given like: > > $ ./scripts/get_maintainer.pl ./lib/string.c > > > > Han will not be noisily to/cc'd (like a K: type would in this > > circumstance) > > > > Note that folks really shouldn't be using get_maintainer on tree files > > anyways [1]. > > > > [1]: https://lore.kernel.org/all/20230726151515.1650519-1-k...@kernel.org/ > > As Greg suggested, please drop the above paragraph and link. Then this > looks good to me. > > I would immediately want to send this patch too, so please feel free to > add this to your series (and I bet many other hints on "git grep 'K:.\\b'" > would want to switch from K: to D: too): > > diff --git a/MAINTAINERS b/MAINTAINERS [] > @@ -5057,7 +5057,7 @@ F: Documentation/kbuild/llvm.rst > F: include/linux/compiler-clang.h > F: scripts/Makefile.clang > F: scripts/clang-tools/ > -K: \b(?i:clang|llvm)\b > +D: \b(?i:clang|llvm)\b etc... My assumption is that the K: --file use is just unnecessary and it'd be better to only use the K: lookup on patches. (and I've somehow stuffed up the receiving side of my email configuration so please ignore any emails to me that bounce for a while)
Re: [PATCH 3/3] get_maintainer: add patch-only pattern matching type
On Wed, 2023-09-27 at 03:19 +, Justin Stitt wrote: > Add the "D:" type which behaves the same as "K:" but will only match > content present in a patch file. Likely it'd be less aggravating just to document that K: is only for patches and add a !$file test.
[PATCH v5] scripts/link-vmlinux.sh: Add alias to duplicate symbols for kallsyms
It is not uncommon for drivers or modules related to similar peripherals to have symbols with the exact same name. While this is not a problem for the kernel's binary itself, it becomes an issue when attempting to trace or probe specific functions using infrastructure like ftrace or kprobe. The tracing subsystem relies on the `nm -n vmlinux` output, which provides symbol information from the kernel's ELF binary. However, when multiple symbols share the same name, the standard nm output does not differentiate between them. This can lead to confusion and difficulty when trying to probe the intended symbol. ~ # cat /proc/kallsyms | grep " name_show" 8c4f76d0 t name_show 8c9cccb0 t name_show 8cb0ac20 t name_show 8cc728c0 t name_show 8ce0efd0 t name_show 8ce126c0 t name_show 8ce1dd20 t name_show 8ce24e70 t name_show 8d1104c0 t name_show 8d1fe480 t name_show kas_alias addresses this challenge by enhancing symbol names with meaningful suffixes generated from the source file and line number during the kernel build process. These newly generated aliases provide tracers with the ability to comprehend the symbols they are interacting with when utilizing the ftracefs interface. This approach may also allow for the probing by name of previously inaccessible symbols. ~ # cat /proc/kallsyms | grep gic_mask_irq d15671e505ac t gic_mask_irq d15671e505ac t gic_mask_irq@drivers_irqchip_irq_gic_c_167 d15671e532a4 t gic_mask_irq d15671e532a4 t gic_mask_irq@drivers_irqchip_irq_gic_v3_c_407 ~ # Changes from v1: - Integrated changes requested by Masami to exclude symbols with prefixes "_cfi" and "_pfx". - Introduced a small framework to handle patterns that need to be excluded from the alias production. - Excluded other symbols using the framework. - Introduced the ability to discriminate between text and data symbols. - Added two new config symbols in this version: CONFIG_KALLSYMS_ALIAS_DATA, which allows data for data, and CONFIG_KALLSYMS_ALIAS_DATA_ALL, which excludes all filters and provides an alias for each duplicated symbol. https://lore.kernel.org/all/20230711151925.1092080-1-alessandro.carmin...@gmail.com/ Changes from v2: - Alias tags are created by querying DWARF information from the vmlinux. - The filename + line number is normalized and appended to the original name. - The tag begins with '@' to indicate the symbol source. - Not a change, but worth mentioning, since the alias is added to the existing list, the old duplicated name is preserved, and the livepatch way of dealing with duplicates is maintained. - Acknowledging the existence of scenarios where inlined functions declared in header files may result in multiple copies due to compiler behavior, though it is not actionable as it does not pose an operational issue. - Highlighting a single exception where the same name refers to different functions: the case of "compat_binfmt_elf.c," which directly includes "binfmt_elf.c" producing identical function copies in two separate modules. https://lore.kernel.org/all/20230714150326.1152359-1-alessandro.carmin...@gmail.com/ Changes from v3: - kas_alias was rewritten in Python to create a more concise and maintainable codebase. - The previous automation process used by kas_alias to locate the vmlinux and the addr2line has been replaced with an explicit command-line switch for specifying these requirements. - addr2line has been added into the main Makefile. - A new command-line switch has been introduced, enabling users to extend the alias to global data names. https://lore.kernel.org/all/20230828080423.3539686-1-alessandro.carmin...@gmail.com/ Changes from v4: - Fixed the O= build issue - The tool halts execution upon encountering major issues, thereby ensuring the pipeline is interrupted. - A cmdline option to specify the source directory added. - Minor code adjusments. - Tested on mips32 and i386 https://lore.kernel.org/all/20230919193948.465340-1-alessandro.carmin...@gmail.com/ NOTE: About the symbols name duplication that happens as consequence of the inclusion compat_binfmt_elf.c does, it is evident that this corner is inherently challenging the addr2line approach. Attempting to conceal this limitation would be counterproductive. compat_binfmt_elf.c includes directly binfmt_elf.c, addr2line can't help but report all functions and data declared by that file, coming from binfmt_elf.c. My position is that, rather than producing a more complicated pipeline to handle this corner case, it is better to fix the compat_binfmt_elf.c anomaly. This patch does not deal with the two potentially problematic symbols defined by compat_binfmt_elf.c Signed-off-by: Alessandro Carminati (Red Hat) --- Makefile| 4 +- init/Kconfig| 22 +++ scripts/kas_alias.py| 136 scripts/link-vmlinux.sh | 21
Re: droid4 -- weird behaviour when attempting to use usb host
On Wed, 2023-09-27 at 17:52 +0200, Pavel Machek wrote: > Hi! > > > > I'm having some fun with usb host. Good news is it works with > > > externally powered hub... after a while. I get some error > > > messages > > > about inability to go host mode, but with enough patience it > > > eventually does enter host mode and I see my keyboard/mouse. > > > > > > And usually in that process, one of my cpu cores disappear. top > > > no > > > longer shows 2 cores, and I was wondering for a while if d4 is > > > single-core system. It is not, my two cores are back after > > > reboot. > > > > > > That's with 6.1.9 kernel from leste. Ideas how to debug this > > > would be > > > welcome. (Do you use usb host?) > > > > You are using a "proper" non-standard usb micro-b cable that > > grounds > > the id pin, right? > > Yes. > > > If not, try with one of those as it allows the hardware to do what > > it's > > supposed to do. > > > > And presumably you don't have a hacked usb hub that feeds back the > > vbus to your phone, right? > > Do have hacked hub. Or more precisely, have device that needs > external > power (spinning rust), and hub passes it back to the device. > > I'll retry with a keyboard... but I recall it behaved funny with > that, too. > > > If you have, that should not be used as the pmic can feed vbus. > > Well, my plan was to use it as a desktop, and external power is > useful > that as Droid battery is not that big. > > Best regards, > Pavel I use usb host quite a bit with xt875, mostly to have a keyboard. I have noted that several hubs i have don't work reliably, im not sure why. Its possible that they use or request too mutch power from the port as cpcap has a pretty small vbus out limit. All my regular otg adapters work fine however. I also have a powered hub that works quite well, its item 373697032160 on ebay
Re: [PATCH 3/3] get_maintainer: add patch-only pattern matching type
On Wed, Sep 27, 2023 at 03:19:16AM +, Justin Stitt wrote: > Add the "D:" type which behaves the same as "K:" but will only match > content present in a patch file. > > To illustrate: > > Imagine this entry in MAINTAINERS: > > NEW REPUBLIC > M: Han Solo > W: https://www.jointheresistance.org > D: \bstrncpy\b > > Our maintainer, Han, will only be added to the recipients if a patch > file is passed to get_maintainer (like what b4 does): > $ ./scripts/get_maintainer.pl 0004-some-change.patch > > If the above patch has a `strncpy` present in the subject, commit log or > diff then Han will be to/cc'd. > > However, in the event of a file from the tree given like: > $ ./scripts/get_maintainer.pl ./lib/string.c > > Han will not be noisily to/cc'd (like a K: type would in this > circumstance) > > Note that folks really shouldn't be using get_maintainer on tree files > anyways [1]. > > [1]: https://lore.kernel.org/all/20230726151515.1650519-1-k...@kernel.org/ As Greg suggested, please drop the above paragraph and link. Then this looks good to me. I would immediately want to send this patch too, so please feel free to add this to your series (and I bet many other hints on "git grep 'K:.\\b'" would want to switch from K: to D: too): diff --git a/MAINTAINERS b/MAINTAINERS index 5f18c6ba3c3c..830e10866acf 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5057,7 +5057,7 @@ F:Documentation/kbuild/llvm.rst F: include/linux/compiler-clang.h F: scripts/Makefile.clang F: scripts/clang-tools/ -K: \b(?i:clang|llvm)\b +D: \b(?i:clang|llvm)\b CLK API M: Russell King @@ -8199,7 +8199,7 @@ F:lib/strcat_kunit.c F: lib/strscpy_kunit.c F: lib/test_fortify/* F: scripts/test_fortify.sh -K: \b__NO_FORTIFY\b +D: \b__NO_FORTIFY\b FPGA DFL DRIVERS M: Wu Hao @@ -11457,9 +11457,9 @@ F: include/linux/overflow.h F: include/linux/randomize_kstack.h F: kernel/configs/hardening.config F: mm/usercopy.c -K: \b(add|choose)_random_kstack_offset\b -K: \b__check_(object_size|heap_object)\b -K: \b__counted_by\b +D: \b(add|choose)_random_kstack_offset\b +D: \b__check_(object_size|heap_object)\b +D: \b__counted_by\b KERNEL JANITORS L: kernel-janit...@vger.kernel.org @@ -17354,7 +17354,7 @@ F: drivers/acpi/apei/erst.c F: drivers/firmware/efi/efi-pstore.c F: fs/pstore/ F: include/linux/pstore* -K: \b(pstore|ramoops) +D: \b(pstore|ramoops) PTP HARDWARE CLOCK SUPPORT M: Richard Cochran @@ -19302,8 +19302,8 @@ F: include/uapi/linux/seccomp.h F: kernel/seccomp.c F: tools/testing/selftests/kselftest_harness.h F: tools/testing/selftests/seccomp/* -K: \bsecure_computing -K: \bTIF_SECCOMP\b +D: \bsecure_computing +D: \bTIF_SECCOMP\b SECURE DIGITAL HOST CONTROLLER INTERFACE (SDHCI) Broadcom BRCMSTB DRIVER M: Kamal Dasu -- Kees Cook
Re: [PATCH 1/3] MAINTAINERS: add documentation for D:
On Wed, Sep 27, 2023 at 03:19:14AM +, Justin Stitt wrote: > Document what "D:" does. > > This is more or less the same as what "K:" does but only works for patch > files. > > See [3/3] for more info and an illustrative example. > --- > MAINTAINERS | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index b19995690904..de68d2c0cf29 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -59,6 +59,9 @@ Descriptions of section entries and preferred order > matches patches or files that contain one or more of the words > printk, pr_info or pr_err > One regex pattern per line. Multiple K: lines acceptable. > + D: *Content regex* (perl extended) pattern match patches only. > + Usage same as K:. > + The "emphasis" tags here are used when rendering: https://docs.kernel.org/process/maintainers.html In this case, I assume "D" is inspired by "Diff", so perhaps reword this to get a proper emphasis hint, and add additional context: D: *Diff content regex* (perl extended) pattern match that applies only to patches and not entire files (e.g. when using the get_maintainers.pl script). -- Kees Cook
Re: [PATCH 0/3] get_maintainer: add patch-only keyword matching
On Wed, Sep 27, 2023 at 08:24:58AM -0700, Nick Desaulniers wrote: > On Tue, Sep 26, 2023 at 8:19 PM Justin Stitt wrote: > > > > This series aims to add "D:" which behaves exactly the same as "K:" but > > works only on patch files. > > > > The goal of this is to reduce noise when folks use get_maintainer on > > tree files as opposed to patches. This use case should be steered away > > from [1] but "D:" should help maintainers reduce noise in their inboxes > > regardless, especially when matching omnipresent keywords like [2]. In > > the event of [2] Kees would be to/cc'd from folks running get_maintainer > > on _any_ file containing "__counted_by". The number of these files is > > rising and I fear for his inbox as his goal, as I understand it, is to > > simply monitor the introduction of new __counted_by annotations to > > ensure accurate semantics. > > Something like this (whether this series or a different approach) > would be helpful to me as well; we use K: to get cc'ed on patches > mentioning clang or llvm, but our ML also then ends up getting cc'ed > on every follow up patch to most files. > > This is causing excessive posts on our ML. As a result, it's a > struggle to get folks to cc themselves to the ML, which puts the code > review burden on fewer people. > > Whether it's a new D: or refinement to the behavior of K:, I applaud > the effort. Hopefully we can find an approach that works for > everyone. Yes, please! I would use this immediately -- there are a bunch of places where pstore, strings, hardening, etc all want review if certain functions or structures are changed in a patch, but we're not maintainers of the files they appear in. > > Justin Stitt (3): > > MAINTAINERS: add documentation for D: > > get_maintainer: add patch-only pattern matching type Can we squash these two changes together, and then likely add some patches for moving things out of K: ? -- Kees Cook
Re: droid4 -- weird behaviour when attempting to use usb host
Hi! > > I'm having some fun with usb host. Good news is it works with > > externally powered hub... after a while. I get some error messages > > about inability to go host mode, but with enough patience it > > eventually does enter host mode and I see my keyboard/mouse. > > > > And usually in that process, one of my cpu cores disappear. top no > > longer shows 2 cores, and I was wondering for a while if d4 is > > single-core system. It is not, my two cores are back after reboot. > > > > That's with 6.1.9 kernel from leste. Ideas how to debug this would be > > welcome. (Do you use usb host?) > > You are using a "proper" non-standard usb micro-b cable that grounds > the id pin, right? Yes. > If not, try with one of those as it allows the hardware to do what it's > supposed to do. > > And presumably you don't have a hacked usb hub that feeds back the > vbus to your phone, right? Do have hacked hub. Or more precisely, have device that needs external power (spinning rust), and hub passes it back to the device. I'll retry with a keyboard... but I recall it behaved funny with that, too. > If you have, that should not be used as the pmic can feed vbus. Well, my plan was to use it as a desktop, and external power is useful that as Droid battery is not that big. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH v5 09/18] x86/sgx: Store struct sgx_encl when allocating new VA pages
Hi Kai,
On Wed, 27 Sep 2023 06:14:20 -0500, Huang, Kai wrote:
On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote:
From: Sean Christopherson
In a later patch, when a cgroup has exceeded the max capacity for EPC
pages, it may need to identify and OOM kill a less active enclave to
make room for other enclaves within the same group. Such a victim
enclave would have no active pages other than the unreclaimable Version
Array (VA) and SECS pages. Therefore, the cgroup needs examine its
unreclaimable page list, and finding an enclave given a SECS page or a
VA page. This will require a backpointer from a page to an enclave,
which is not available for VA pages.
Because struct sgx_epc_page instances of VA pages are not owned by an
sgx_encl_page instance, mark their owner as sgx_encl: pass the struct
sgx_encl of the enclave allocating the VA page to sgx_alloc_epc_page(),
which will store this value in the owner field of the struct
sgx_epc_page. In a later patch, VA pages will be placed in an
unreclaimable queue that can be examined by the cgroup to select the OOM
killed enclave.
Signed-off-by: Sean Christopherson
Co-developed-by: Kristen Carlson Accardi
Signed-off-by: Kristen Carlson Accardi
Co-developed-by: Haitao Huang
Signed-off-by: Haitao Huang
Cc: Sean Christopherson
[...]
@@ -562,7 +562,7 @@ struct sgx_epc_page *sgx_alloc_epc_page(void
*owner, bool reclaim)
for ( ; ; ) {
page = __sgx_alloc_epc_page();
if (!IS_ERR(page)) {
- page->owner = owner;
+ page->encl_page = owner;
Looks using 'encl_page' is arbitrary.
Also actually for virtual EPC page the owner is set to the 'sgx_vepc'
instance.
break;
}
@@ -607,7 +607,7 @@ void sgx_free_epc_page(struct sgx_epc_page *page)
spin_lock(>lock);
- page->owner = NULL;
+ page->encl_page = NULL;
Ditto.
if (page->poison)
list_add(>list, >sgx_poison_page_list);
else
@@ -642,7 +642,7 @@ static bool __init sgx_setup_epc_section(u64
phys_addr, u64 size,
for (i = 0; i < nr_pages; i++) {
section->pages[i].section = index;
section->pages[i].flags = 0;
- section->pages[i].owner = NULL;
+ section->pages[i].encl_page = NULL;
section->pages[i].poison = 0;
list_add_tail(>pages[i].list, _dirty_page_list);
}
diff --git a/arch/x86/kernel/cpu/sgx/sgx.h
b/arch/x86/kernel/cpu/sgx/sgx.h
index 764cec23f4e5..5110dd433b80 100644
--- a/arch/x86/kernel/cpu/sgx/sgx.h
+++ b/arch/x86/kernel/cpu/sgx/sgx.h
@@ -68,7 +68,12 @@ struct sgx_epc_page {
unsigned int section;
u16 flags;
u16 poison;
- struct sgx_encl_page *owner;
+
+ /* Possible owner types */
+ union {
+ struct sgx_encl_page *encl_page;
+ struct sgx_encl *encl;
+ };
Sadly for virtual EPC page the owner is set to the 'sgx_vepc' instance it
belongs to.
Given how sgx_{alloc|free}_epc_page() arbitrarily uses encl_page,
perhaps we
should do below?
union {
struct sgx_encl_page *encl_page;
struct sgx_encl *encl;
struct sgx_vepc *vepc;
void *owner;
};
And in sgx_{alloc|free}_epc_page() we can use 'owner' instead.
As I mentioned in cover letter and change log in 11/18, this series does
not track virtual EPC.
We can add vepc field into the union in future if such tracking is needed.
Don't think "void *owner" is needed though.
Thanks
Haitao
Re: [PATCH 0/3] get_maintainer: add patch-only keyword matching
On Tue, Sep 26, 2023 at 8:19 PM Justin Stitt wrote: > > This series aims to add "D:" which behaves exactly the same as "K:" but > works only on patch files. > > The goal of this is to reduce noise when folks use get_maintainer on > tree files as opposed to patches. This use case should be steered away > from [1] but "D:" should help maintainers reduce noise in their inboxes > regardless, especially when matching omnipresent keywords like [2]. In > the event of [2] Kees would be to/cc'd from folks running get_maintainer > on _any_ file containing "__counted_by". The number of these files is > rising and I fear for his inbox as his goal, as I understand it, is to > simply monitor the introduction of new __counted_by annotations to > ensure accurate semantics. Something like this (whether this series or a different approach) would be helpful to me as well; we use K: to get cc'ed on patches mentioning clang or llvm, but our ML also then ends up getting cc'ed on every follow up patch to most files. This is causing excessive posts on our ML. As a result, it's a struggle to get folks to cc themselves to the ML, which puts the code review burden on fewer people. Whether it's a new D: or refinement to the behavior of K:, I applaud the effort. Hopefully we can find an approach that works for everyone. And may God have mercy on your soul for having to touch that much perl. :-P > > See [3/3] for an illustrative example. > > This series also includes a formatting pass over get_maintainer because > I personally found it difficult to parse with the human eye. > > [1]: https://lore.kernel.org/all/20230726151515.1650519-1-k...@kernel.org/ > [2]: https://lore.kernel.org/all/20230925172037.work.853-k...@kernel.org/ > > Signed-off-by: Justin Stitt > --- > Justin Stitt (3): > MAINTAINERS: add documentation for D: > get_maintainer: run perltidy > get_maintainer: add patch-only pattern matching type > > MAINTAINERS |3 + > scripts/get_maintainer.pl | 3334 > +++-- > 2 files changed, 1718 insertions(+), 1619 deletions(-) > --- > base-commit: 6465e260f48790807eef06b583b38ca9789b6072 > change-id: 20230926-get_maintainer_add_d-07424a814e72 > > Best regards, > -- > Justin Stitt > -- Thanks, ~Nick Desaulniers
Re: [PATCH] dt-bindings: remoteproc: mtk,scp: Add missing additionalProperties on child node schemas
On Tue, Sep 26, 2023 at 11:45:08AM -0500, Rob Herring wrote: > Just as unevaluatedProperties or additionalProperties are required at > the top level of schemas, they should (and will) also be required for > child node schemas. That ensures only documented properties are > present for any node. > > Signed-off-by: Rob Herring > --- Acked-by: Conor Dooley > Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml > b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml > index 895415772d1d..24422fd56e83 100644 > --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml > +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml > @@ -91,6 +91,7 @@ allOf: > > additionalProperties: >type: object > + additionalProperties: false >description: > Subnodes of the SCP represent rpmsg devices. The names of the devices > are not important. The properties of these nodes are defined by the > -- > 2.40.1 > signature.asc Description: PGP signature
Re: [PATCH] dt-bindings: remoteproc: mtk,scp: Add missing additionalProperties on child node schemas
Il 26/09/23 18:45, Rob Herring ha scritto: Just as unevaluatedProperties or additionalProperties are required at the top level of schemas, they should (and will) also be required for child node schemas. That ensures only documented properties are present for any node. Signed-off-by: Rob Herring Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH] net: appletalk: remove cops support
Greg Kroah-Hartman writes: > The COPS Appletalk support is very old, never said to actually work > properly, and the firmware code for the devices are under a very suspect > license. Remove it all to clear up the license issue, if it is still > needed and actually used by anyone, we can add it back later once the > license is cleared up. > > Reported-by: Prarit Bhargava > Cc: Christoph Hellwig > Cc: Vitaly Kuznetsov FWIW, Reviewed-by: Vitaly Kuznetsov > Cc: jsch...@samba.org > Signed-off-by: Greg Kroah-Hartman > --- > .../device_drivers/appletalk/cops.rst | 80 -- > .../device_drivers/appletalk/index.rst| 18 - > .../networking/device_drivers/index.rst |1 - > drivers/net/Space.c |6 - > drivers/net/appletalk/Kconfig | 30 - > drivers/net/appletalk/Makefile|1 - > drivers/net/appletalk/cops.c | 1005 - > drivers/net/appletalk/cops.h | 61 - > drivers/net/appletalk/cops_ffdrv.h| 532 - > drivers/net/appletalk/cops_ltdrv.h| 241 > include/net/Space.h |1 - > 11 files changed, 1976 deletions(-) > delete mode 100644 Documentation/networking/device_drivers/appletalk/cops.rst > delete mode 100644 > Documentation/networking/device_drivers/appletalk/index.rst > delete mode 100644 drivers/net/appletalk/cops.c > delete mode 100644 drivers/net/appletalk/cops.h > delete mode 100644 drivers/net/appletalk/cops_ffdrv.h > delete mode 100644 drivers/net/appletalk/cops_ltdrv.h > > diff --git a/Documentation/networking/device_drivers/appletalk/cops.rst > b/Documentation/networking/device_drivers/appletalk/cops.rst > deleted file mode 100644 > index 964ba80599a9.. > --- a/Documentation/networking/device_drivers/appletalk/cops.rst > +++ /dev/null > @@ -1,80 +0,0 @@ > -.. SPDX-License-Identifier: GPL-2.0 > - > - > -The COPS LocalTalk Linux driver (cops.c) > - > - > -By Jay Schulist > - > -This driver has two modes and they are: Dayna mode and Tangent mode. > -Each mode corresponds with the type of card. It has been found > -that there are 2 main types of cards and all other cards are > -the same and just have different names or only have minor differences > -such as more IO ports. As this driver is tested it will > -become more clear exactly what cards are supported. > - > -Right now these cards are known to work with the COPS driver. The > -LT-200 cards work in a somewhat more limited capacity than the > -DL200 cards, which work very well and are in use by many people. > - > -TANGENT driver mode: > - - Tangent ATB-II, Novell NL-1000, Daystar Digital LT-200 > - > -DAYNA driver mode: > - - Dayna DL2000/DaynaTalk PC (Half Length), COPS LT-95, > - - Farallon PhoneNET PC III, Farallon PhoneNET PC II > - > -Other cards possibly supported mode unknown though: > - - Dayna DL2000 (Full length) > - > -The COPS driver defaults to using Dayna mode. To change the driver's > -mode if you built a driver with dual support use board_type=1 or > -board_type=2 for Dayna or Tangent with insmod. > - > -Operation/loading of the driver > -=== > - > -Use modprobe like this: /sbin/modprobe cops.o (IO #) (IRQ #) > -If you do not specify any options the driver will try and use the IO = 0x240, > -IRQ = 5. As of right now I would only use IRQ 5 for the card, if autoprobing. > - > -To load multiple COPS driver Localtalk cards you can do one of the > following:: > - > - insmod cops io=0x240 irq=5 > - insmod -o cops2 cops io=0x260 irq=3 > - > -Or in lilo.conf put something like this:: > - > - append="ether=5,0x240,lt0 ether=3,0x260,lt1" > - > -Then bring up the interface with ifconfig. It will look something like this:: > - > - lt0 Link encap:UNSPEC HWaddr > 00-00-00-00-00-00-00-F7-00-00-00-00-00-00-00-00 > - inet addr:192.168.1.2 Bcast:192.168.1.255 Mask:255.255.255.0 > - UP BROADCAST RUNNING NOARP MULTICAST MTU:600 Metric:1 > - RX packets:0 errors:0 dropped:0 overruns:0 frame:0 > - TX packets:0 errors:0 dropped:0 overruns:0 carrier:0 coll:0 > - > -Netatalk Configuration > -== > - > -You will need to configure atalkd with something like the following to make > -it work with the cops.c driver. > - > -* For single LTalk card use:: > - > -dummy -seed -phase 2 -net 2000 -addr 2000.10 -zone "1033" > -lt0 -seed -phase 1 -net 1000 -addr 1000.50 -zone "1033" > - > -* For multiple cards, Ethernet and LocalTalk:: > - > -eth0 -seed -phase 2 -net 3000 -addr 3000.20 -zone "1033" > -lt0 -seed -phase 1 -net 1000 -addr 1000.50 -zone "1033" > - > -* For multiple LocalTalk cards, and an Ethernet card. > - > -* Order seems to matter here, Ethernet last:: > - > -lt0 -seed -phase 1 -net 1000 -addr 1000.10
Re: [PATCH] net: appletalk: remove cops support
On 9/27/23 05:26, Christoph Hellwig wrote: On Wed, Sep 27, 2023 at 11:00:30AM +0200, Greg Kroah-Hartman wrote: The COPS Appletalk support is very old, never said to actually work properly, and the firmware code for the devices are under a very suspect license. Remove it all to clear up the license issue, if it is still needed and actually used by anyone, we can add it back later once the license is cleared up. Looks good: Acked-by: Christoph Hellwig Ditto. Acked-by: Prarit Bhargava P.
Re: [PATCH v5 11/18] x86/sgx: store unreclaimable pages in LRU lists
On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote: > From: Sean Christopherson > > When an OOM event occurs, all pages associated with an enclave will need > to be freed, including pages that are not currently tracked by the > cgroup LRU lists. What are "cgroup LRU lists"? > > Add a new "unreclaimable" list to the sgx_epc_lru_lists struct and > update the "sgx_record/drop_epc_pages()" functions for adding/removing > VA and SECS pages to/from this "unreclaimable" list. Sorry I don't follow the logic between the two paragraphs. What is the exact problem? How does the new "unreclaimable" list solve the problem?
Re: [PATCH v5 09/18] x86/sgx: Store struct sgx_encl when allocating new VA pages
On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote:
> From: Sean Christopherson
>
> In a later patch, when a cgroup has exceeded the max capacity for EPC
> pages, it may need to identify and OOM kill a less active enclave to
> make room for other enclaves within the same group. Such a victim
> enclave would have no active pages other than the unreclaimable Version
> Array (VA) and SECS pages.
>
What does "no active pages" mean?
A "less active enclave" doesn't necessarily mean it has "no active pages"?
> Therefore, the cgroup needs examine its
^
needs to
> unreclaimable page list, and finding an enclave given a SECS page or a
^
find
> VA page. This will require a backpointer from a page to an enclave,
> which is not available for VA pages.
>
> Because struct sgx_epc_page instances of VA pages are not owned by an
> sgx_encl_page instance, mark their owner as sgx_encl: pass the struct
> sgx_encl of the enclave allocating the VA page to sgx_alloc_epc_page(),
> which will store this value in the owner field of the struct
> sgx_epc_page.
>
IMHO this paragraph is hard to understand and can be more concise:
One VA page can be shared by multiple enclave pages thus cannot be associated
with any 'struct sgx_encl_page' instance. Set the owner of VA page to the
enclave instead.
> In a later patch, VA pages will be placed in an
> unreclaimable queue that can be examined by the cgroup to select the OOM
> killed enclave.
The code to "place the VA page to unreclaimable queue" has been done in earlier
patch ("x86/sgx: Introduce EPC page states"). Just the unreclaimable list isn't
introduced yet. I think you should just introduce it first then you can get rid
of those "in a later patch" staff.
And nit: please use "unreclaimable list" consistently (not queue).
Btw, probably a dumb question:
Theoretically if you only need to find a victim enclave you don't need to put VA
pages to the unreclaimable list, because those VA pages will be freed anyway
when enclave is killed. So keeping VA pages in the list is for accounting all
the pages that the cgroup is having?
Re: [PATCH v5 09/18] x86/sgx: Store struct sgx_encl when allocating new VA pages
On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote:
> From: Sean Christopherson
>
> In a later patch, when a cgroup has exceeded the max capacity for EPC
> pages, it may need to identify and OOM kill a less active enclave to
> make room for other enclaves within the same group. Such a victim
> enclave would have no active pages other than the unreclaimable Version
> Array (VA) and SECS pages. Therefore, the cgroup needs examine its
> unreclaimable page list, and finding an enclave given a SECS page or a
> VA page. This will require a backpointer from a page to an enclave,
> which is not available for VA pages.
>
> Because struct sgx_epc_page instances of VA pages are not owned by an
> sgx_encl_page instance, mark their owner as sgx_encl: pass the struct
> sgx_encl of the enclave allocating the VA page to sgx_alloc_epc_page(),
> which will store this value in the owner field of the struct
> sgx_epc_page. In a later patch, VA pages will be placed in an
> unreclaimable queue that can be examined by the cgroup to select the OOM
> killed enclave.
>
> Signed-off-by: Sean Christopherson
> Co-developed-by: Kristen Carlson Accardi
> Signed-off-by: Kristen Carlson Accardi
> Co-developed-by: Haitao Huang
> Signed-off-by: Haitao Huang
> Cc: Sean Christopherson
>
[...]
> @@ -562,7 +562,7 @@ struct sgx_epc_page *sgx_alloc_epc_page(void *owner, bool
> reclaim)
> for ( ; ; ) {
> page = __sgx_alloc_epc_page();
> if (!IS_ERR(page)) {
> - page->owner = owner;
> + page->encl_page = owner;
Looks using 'encl_page' is arbitrary.
Also actually for virtual EPC page the owner is set to the 'sgx_vepc' instance.
> break;
> }
>
> @@ -607,7 +607,7 @@ void sgx_free_epc_page(struct sgx_epc_page *page)
>
> spin_lock(>lock);
>
> - page->owner = NULL;
> + page->encl_page = NULL;
Ditto.
> if (page->poison)
> list_add(>list, >sgx_poison_page_list);
> else
> @@ -642,7 +642,7 @@ static bool __init sgx_setup_epc_section(u64 phys_addr,
> u64 size,
> for (i = 0; i < nr_pages; i++) {
> section->pages[i].section = index;
> section->pages[i].flags = 0;
> - section->pages[i].owner = NULL;
> + section->pages[i].encl_page = NULL;
> section->pages[i].poison = 0;
> list_add_tail(>pages[i].list, _dirty_page_list);
> }
> diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h
> index 764cec23f4e5..5110dd433b80 100644
> --- a/arch/x86/kernel/cpu/sgx/sgx.h
> +++ b/arch/x86/kernel/cpu/sgx/sgx.h
> @@ -68,7 +68,12 @@ struct sgx_epc_page {
> unsigned int section;
> u16 flags;
> u16 poison;
> - struct sgx_encl_page *owner;
> +
> + /* Possible owner types */
> + union {
> + struct sgx_encl_page *encl_page;
> + struct sgx_encl *encl;
> + };
Sadly for virtual EPC page the owner is set to the 'sgx_vepc' instance it
belongs to.
Given how sgx_{alloc|free}_epc_page() arbitrarily uses encl_page, perhaps we
should do below?
union {
struct sgx_encl_page *encl_page;
struct sgx_encl *encl;
struct sgx_vepc *vepc;
void *owner;
};
And in sgx_{alloc|free}_epc_page() we can use 'owner' instead.
Re: [PATCH v5 07/18] x86/sgx: Introduce RECLAIM_IN_PROGRESS state
> @@ -312,13 +312,15 @@ static void sgx_reclaim_pages(void)
> list_del_init(_page->list);
> encl_page = epc_page->owner;
>
> - if (kref_get_unless_zero(_page->encl->refcount) != 0)
> + if (kref_get_unless_zero(_page->encl->refcount) != 0) {
> + sgx_epc_page_set_state(epc_page,
> SGX_EPC_PAGE_RECLAIM_IN_PROGRESS);
> chunk[cnt++] = epc_page;
> - else
> + } else {
> /* The owner is freeing the page. No need to add the
>* page back to the list of reclaimable pages.
>*/
Please use proper comment style:
For single line comment:
/* ... */
For multiple lines comment:
/*
* ...
*/
> sgx_epc_page_reset_state(epc_page);
> + }
Nit: unintended new {} around 'else'?
> }
> spin_unlock(_global_lru.lock);
>
> @@ -528,16 +530,13 @@ void sgx_record_epc_page(struct sgx_epc_page *page,
> unsigned long flags)
> int sgx_drop_epc_page(struct sgx_epc_page *page)
> {
> spin_lock(_global_lru.lock);
> - if (sgx_epc_page_reclaimable(page->flags)) {
> - /* The page is being reclaimed. */
> - if (list_empty(>list)) {
> - spin_unlock(_global_lru.lock);
> - return -EBUSY;
> - }
> -
> - list_del(>list);
> - sgx_epc_page_reset_state(page);
> + if (sgx_epc_page_reclaim_in_progress(page->flags)) {
> + spin_unlock(_global_lru.lock);
> + return -EBUSY;
> }
> +
> + list_del(>list);
> + sgx_epc_page_reset_state(page);
> spin_unlock(_global_lru.lock);
>
> return 0;
> diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h
> index 2faeb40b345f..764cec23f4e5 100644
> --- a/arch/x86/kernel/cpu/sgx/sgx.h
> +++ b/arch/x86/kernel/cpu/sgx/sgx.h
> @@ -40,6 +40,8 @@ enum sgx_epc_page_state {
>
> /* Page is in use and tracked in a reclaimable LRU list
>* Becomes NOT_TRACKED after sgx_drop_epc()
> + * Becomes RECLAIM_IN_PROGRESS in sgx_reclaim_pages() when identified
> + * for reclaiming
>*/
> SGX_EPC_PAGE_RECLAIMABLE = 2,
>
> @@ -50,6 +52,14 @@ enum sgx_epc_page_state {
>*/
> SGX_EPC_PAGE_UNRECLAIMABLE = 3,
>
> + /* Page is being prepared for reclamation, tracked in a temporary
> + * isolated list by the reclaimer.
> + * Changes in sgx_reclaim_pages() back to RECLAIMABLE if preparation
> + * fails for any reason.
> + * Becomes NOT_TRACKED if reclaimed successfully in sgx_reclaim_pages()
> + * and immediately sgx_free_epc() is called to make it FREE.
> + */
Ditto for the comment style.
And please use blank line to separate paragraphs for better readability.
Re: [PATCH v5 06/18] x86/sgx: Introduce EPC page states
On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote: > Use the lower 3 bits in the flags field of sgx_epc_page struct to > track EPC states in its life cycle and define an enum for possible > states. More state(s) will be added later. This patch does more than what the changelog claims to do. AFAICT it does below: 1) Use the lower 3 bits to track EPC page status 2) Rename SGX_EPC_PAGE_RECLAIMER_TRACKED to SGX_EPC_PAGE_RERCLAIMABLE 3) Introduce a new state SGX_EPC_PAGE_UNRECLAIMABLE 4) Track SECS and VA pages as SGX_EPC_PAGE_UNRECLAIMABLE The changelog only says 1) IIUC. If we really want to do all these in one patch, then the changelog should at least mention the justification of all of them. But I don't see why 3) and 4) need to be done here. Instead, IMHO they should be done in a separate patch, and do it after the unreclaimable list is introduced (or you need to bring that patch forward). For instance, ... [snip] > + > + /* Page is in use but tracked in an unreclaimable LRU list. These are > + * only reclaimable when the whole enclave is OOM killed or the enclave > + * is released, e.g., VA, SECS pages > + * Becomes NOT_TRACKED after sgx_drop_epc() > + */ > + SGX_EPC_PAGE_UNRECLAIMABLE = 3, ... We even don't have the unreclaimable LRU list yet. It's odd to have this comment here.
Re: [PATCH v5 05/18] x86/sgx: Store reclaimable EPC pages in sgx_epc_lru_lists
On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote: > --- a/arch/x86/kernel/cpu/sgx/main.c > +++ b/arch/x86/kernel/cpu/sgx/main.c > @@ -268,7 +268,6 @@ static void sgx_reclaimer_write(struct sgx_epc_page > *epc_page, > goto out; > > sgx_encl_ewb(encl->secs.epc_page, _backing); > - > sgx_encl_free_epc_page(encl->secs.epc_page); > encl->secs.epc_page = NULL; > Nit: perhaps unintended change.
Re: [PATCH] net: appletalk: remove cops support
On Wed, Sep 27, 2023 at 11:00:30AM +0200, Greg Kroah-Hartman wrote: > The COPS Appletalk support is very old, never said to actually work > properly, and the firmware code for the devices are under a very suspect > license. Remove it all to clear up the license issue, if it is still > needed and actually used by anyone, we can add it back later once the > license is cleared up. Looks good: Acked-by: Christoph Hellwig
Re: [PATCH v5 01/18] cgroup/misc: Add per resource callbacks for CSS events
On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote:
> From: Kristen Carlson Accardi
>
> The misc cgroup controller (subsystem) currently does not perform
> resource type specific action for Cgroups Subsystem State (CSS) events:
> the 'css_alloc' event when a cgroup is created and the 'css_free' event
> when a cgroup is destroyed, or in event of user writing the max value to
> the misc.max file to set the usage limit of a specific resource
> [admin-guide/cgroup-v2.rst, 5-9. Misc].
>
> Define callbacks for those events and allow resource providers to
> register the callbacks per resource type as needed. This will be
> utilized later by the EPC misc cgroup support implemented in the SGX
> driver:
> - On css_alloc, allocate and initialize necessary structures for EPC
> reclaiming, e.g., LRU list, work queue, etc.
> - On css_free, cleanup and free those structures created in alloc.
> - On max_write, trigger EPC reclaiming if the new limit is at or below
> current usage.
Nit:
Wondering why we should trigger EPC reclaiming if the new limit is *at* current
usage?
I actually don't quite care about why here, but writing these details in the
changelog may bring unnecessary confusion. I guess you can just remove all the
details about what SGX driver needs to do on these callbacks.
>
> Signed-off-by: Kristen Carlson Accardi
> Signed-off-by: Haitao Huang
> ---
> V5:
> - Remove prefixes from the callback names (tj)
> - Update commit message (Jarkko)
>
> V4:
> - Moved this to the front of the series.
> - Applies on cgroup/for-6.6 with the overflow fix for misc.
>
> V3:
> - Removed the released() callback
> ---
> include/linux/misc_cgroup.h | 5 +
> kernel/cgroup/misc.c| 32 +---
> 2 files changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/misc_cgroup.h b/include/linux/misc_cgroup.h
> index e799b1f8d05b..96a88822815a 100644
> --- a/include/linux/misc_cgroup.h
> +++ b/include/linux/misc_cgroup.h
> @@ -37,6 +37,11 @@ struct misc_res {
> u64 max;
> atomic64_t usage;
> atomic64_t events;
> +
> + /* per resource callback ops */
Nit:
This comment isn't quite useful IMHO. And it seems you should just expand the
existing comment for the 'struct misc_res', which already covers the existing
members.
Or as Jarkko suggested, maybe you can introduce another structure 'misc_res_ops'
and comment more details for all these callbacks just like 'struct misc_res'.
Anyway it's cgroup maintainer's call.
> + int (*alloc)(struct misc_cg *cg);
> + void (*free)(struct misc_cg *cg);
> + void (*max_write)(struct misc_cg *cg);
> };
>
> /**
> diff --git a/kernel/cgroup/misc.c b/kernel/cgroup/misc.c
> index 79a3717a5803..62c9198dee21 100644
> --- a/kernel/cgroup/misc.c
> +++ b/kernel/cgroup/misc.c
> @@ -276,10 +276,13 @@ static ssize_t misc_cg_max_write(struct
> kernfs_open_file *of, char *buf,
>
> cg = css_misc(of_css(of));
>
> - if (READ_ONCE(misc_res_capacity[type]))
> + if (READ_ONCE(misc_res_capacity[type])) {
> WRITE_ONCE(cg->res[type].max, max);
> - else
> + if (cg->res[type].max_write)
> + cg->res[type].max_write(cg);
> + } else {
> ret = -EINVAL;
> + }
>
> return ret ? ret : nbytes;
> }
> @@ -383,23 +386,39 @@ static struct cftype misc_cg_files[] = {
> static struct cgroup_subsys_state *
> misc_cg_alloc(struct cgroup_subsys_state *parent_css)
> {
> + struct misc_cg *parent_cg;
Nit:
The below variable '*cg' can be moved here together with 'parent_cg'.
> enum misc_res_type i;
> struct misc_cg *cg;
> + int ret;
>
> if (!parent_css) {
> cg = _cg;
> + parent_cg = _cg;
Nit:
parent_cg = cg = _cg;
?
> } else {
> cg = kzalloc(sizeof(*cg), GFP_KERNEL);
> if (!cg)
> return ERR_PTR(-ENOMEM);
> + parent_cg = css_misc(parent_css);
> }
>
> for (i = 0; i < MISC_CG_RES_TYPES; i++) {
> WRITE_ONCE(cg->res[i].max, MAX_NUM);
> atomic64_set(>res[i].usage, 0);
> + if (parent_cg->res[i].alloc) {
> + ret = parent_cg->res[i].alloc(cg);
> + if (ret)
> + goto alloc_err;
> + }
> }
>
> return >css;
> +
> +alloc_err:
> + for (i = 0; i < MISC_CG_RES_TYPES; i++)
> + if (parent_cg->res[i].free)
> + cg->res[i].free(cg);
> + kfree(cg);
> + return ERR_PTR(ret);
> }
>
> /**
> @@ -410,7 +429,14 @@ misc_cg_alloc(struct cgroup_subsys_state *parent_css)
> */
> static void misc_cg_free(struct cgroup_subsys_state *css)
> {
> - kfree(css_misc(css));
> + struct misc_cg *cg = css_misc(css);
> + enum misc_res_type i;
> +
> + for (i = 0; i < MISC_CG_RES_TYPES; i++)
> + if (cg->res[i].free)
> +
[PATCH] net: appletalk: remove cops support
The COPS Appletalk support is very old, never said to actually work properly, and the firmware code for the devices are under a very suspect license. Remove it all to clear up the license issue, if it is still needed and actually used by anyone, we can add it back later once the license is cleared up. Reported-by: Prarit Bhargava Cc: Christoph Hellwig Cc: Vitaly Kuznetsov Cc: jsch...@samba.org Signed-off-by: Greg Kroah-Hartman --- .../device_drivers/appletalk/cops.rst | 80 -- .../device_drivers/appletalk/index.rst| 18 - .../networking/device_drivers/index.rst |1 - drivers/net/Space.c |6 - drivers/net/appletalk/Kconfig | 30 - drivers/net/appletalk/Makefile|1 - drivers/net/appletalk/cops.c | 1005 - drivers/net/appletalk/cops.h | 61 - drivers/net/appletalk/cops_ffdrv.h| 532 - drivers/net/appletalk/cops_ltdrv.h| 241 include/net/Space.h |1 - 11 files changed, 1976 deletions(-) delete mode 100644 Documentation/networking/device_drivers/appletalk/cops.rst delete mode 100644 Documentation/networking/device_drivers/appletalk/index.rst delete mode 100644 drivers/net/appletalk/cops.c delete mode 100644 drivers/net/appletalk/cops.h delete mode 100644 drivers/net/appletalk/cops_ffdrv.h delete mode 100644 drivers/net/appletalk/cops_ltdrv.h diff --git a/Documentation/networking/device_drivers/appletalk/cops.rst b/Documentation/networking/device_drivers/appletalk/cops.rst deleted file mode 100644 index 964ba80599a9.. --- a/Documentation/networking/device_drivers/appletalk/cops.rst +++ /dev/null @@ -1,80 +0,0 @@ -.. SPDX-License-Identifier: GPL-2.0 - - -The COPS LocalTalk Linux driver (cops.c) - - -By Jay Schulist - -This driver has two modes and they are: Dayna mode and Tangent mode. -Each mode corresponds with the type of card. It has been found -that there are 2 main types of cards and all other cards are -the same and just have different names or only have minor differences -such as more IO ports. As this driver is tested it will -become more clear exactly what cards are supported. - -Right now these cards are known to work with the COPS driver. The -LT-200 cards work in a somewhat more limited capacity than the -DL200 cards, which work very well and are in use by many people. - -TANGENT driver mode: - - Tangent ATB-II, Novell NL-1000, Daystar Digital LT-200 - -DAYNA driver mode: - - Dayna DL2000/DaynaTalk PC (Half Length), COPS LT-95, - - Farallon PhoneNET PC III, Farallon PhoneNET PC II - -Other cards possibly supported mode unknown though: - - Dayna DL2000 (Full length) - -The COPS driver defaults to using Dayna mode. To change the driver's -mode if you built a driver with dual support use board_type=1 or -board_type=2 for Dayna or Tangent with insmod. - -Operation/loading of the driver -=== - -Use modprobe like this:/sbin/modprobe cops.o (IO #) (IRQ #) -If you do not specify any options the driver will try and use the IO = 0x240, -IRQ = 5. As of right now I would only use IRQ 5 for the card, if autoprobing. - -To load multiple COPS driver Localtalk cards you can do one of the following:: - - insmod cops io=0x240 irq=5 - insmod -o cops2 cops io=0x260 irq=3 - -Or in lilo.conf put something like this:: - - append="ether=5,0x240,lt0 ether=3,0x260,lt1" - -Then bring up the interface with ifconfig. It will look something like this:: - - lt0 Link encap:UNSPEC HWaddr 00-00-00-00-00-00-00-F7-00-00-00-00-00-00-00-00 - inet addr:192.168.1.2 Bcast:192.168.1.255 Mask:255.255.255.0 - UP BROADCAST RUNNING NOARP MULTICAST MTU:600 Metric:1 - RX packets:0 errors:0 dropped:0 overruns:0 frame:0 - TX packets:0 errors:0 dropped:0 overruns:0 carrier:0 coll:0 - -Netatalk Configuration -== - -You will need to configure atalkd with something like the following to make -it work with the cops.c driver. - -* For single LTalk card use:: - -dummy -seed -phase 2 -net 2000 -addr 2000.10 -zone "1033" -lt0 -seed -phase 1 -net 1000 -addr 1000.50 -zone "1033" - -* For multiple cards, Ethernet and LocalTalk:: - -eth0 -seed -phase 2 -net 3000 -addr 3000.20 -zone "1033" -lt0 -seed -phase 1 -net 1000 -addr 1000.50 -zone "1033" - -* For multiple LocalTalk cards, and an Ethernet card. - -* Order seems to matter here, Ethernet last:: - -lt0 -seed -phase 1 -net 1000 -addr 1000.10 -zone "LocalTalk1" -lt1 -seed -phase 1 -net 2000 -addr 2000.20 -zone "LocalTalk2" -eth0 -seed -phase 2 -net 3000 -addr 3000.30 -zone "EtherTalk" diff --git a/Documentation/networking/device_drivers/appletalk/index.rst
Re: [PATCH v2 5/7] dt-bindings: pinctrl: qcom,sc7280: Allow gpio-reserved-ranges
On Tue, Sep 19, 2023 at 2:46 PM Luca Weiss wrote: > Allow the gpio-reserved-ranges property on SC7280 TLMM. > > Acked-by: Linus Walleij > Acked-by: Krzysztof Kozlowski > Signed-off-by: Luca Weiss This patch 5/7 applied to the pinctrl tree! Yours, Linus Walleij
Re: [PATCH v2 1/2] pinctrl: qcom: msm8226: Add MPM pin mappings
On Sat, Sep 23, 2023 at 3:14 PM Matti Lehtimäki wrote: > Add pin <-> wakeirq mappings to allow for waking up the AP from sleep > through MPM-connected pins. > > Signed-off-by: Matti Lehtimäki Both v2 patches applied! Yours, Linus Walleij
Re: [PATCH v2 0/3] Add blsp1_i2c6 and blsp1_uart2 to MSM8226 SoC
On Fri, Sep 22, 2023 at 6:56 PM Luca Weiss wrote: > Add the I2C bus and UART interface found on the MSM8226. For the I2C bus > we also first need to add the pinctrl function in the driver. > > Signed-off-by: Luca Weiss v2 looks fine and ACKs in place, so patches applied! Yours, Linus Walleij
Re: [PATCH 3/3] get_maintainer: add patch-only pattern matching type
On Wed, Sep 27, 2023 at 03:46:30PM +0900, Justin Stitt wrote: > On Wed, Sep 27, 2023 at 3:14 PM Greg KH wrote: > > > > On Wed, Sep 27, 2023 at 03:19:16AM +, Justin Stitt wrote: > > > Note that folks really shouldn't be using get_maintainer on tree files > > > anyways [1]. > > > > That's not true, Linus and I use it on a daily basis this way, it's part > > of our normal workflow, AND the workflow of the kernel security team. > > > > So please don't take that valid use-case away from us. > > Fair. I'm on the side of keeping the "K:'' behavior the way it is and > that's why I'm proposing adding "D:" to provide a more granular > content matching type operating strictly on patches. It's purely > opt-in. > > The patch I linked mentioned steering folks away from using > tree files but not necessarily removing the behavior. Please don't steer folks away from it, it is a valid use case of the tool, and I would argue, one of the most important ones given how often I use it that way. Hence my objection to this verbage in the changelog, it's not correct. thanks, greg k-h
Re: [PATCH 3/3] get_maintainer: add patch-only pattern matching type
On Wed, Sep 27, 2023 at 3:14 PM Greg KH wrote: > > On Wed, Sep 27, 2023 at 03:19:16AM +, Justin Stitt wrote: > > Note that folks really shouldn't be using get_maintainer on tree files > > anyways [1]. > > That's not true, Linus and I use it on a daily basis this way, it's part > of our normal workflow, AND the workflow of the kernel security team. > > So please don't take that valid use-case away from us. Fair. I'm on the side of keeping the "K:'' behavior the way it is and that's why I'm proposing adding "D:" to provide a more granular content matching type operating strictly on patches. It's purely opt-in. The patch I linked mentioned steering folks away from using tree files but not necessarily removing the behavior. > > thanks, > > greg k-h Thanks Justin
Re: [PATCH 3/3] get_maintainer: add patch-only pattern matching type
On Wed, Sep 27, 2023 at 03:19:16AM +, Justin Stitt wrote: > Note that folks really shouldn't be using get_maintainer on tree files > anyways [1]. That's not true, Linus and I use it on a daily basis this way, it's part of our normal workflow, AND the workflow of the kernel security team. So please don't take that valid use-case away from us. thanks, greg k-h
