RE: [PATCH] selftests/bpf: use !E instead of comparing with NULL
> -Original Message-
> From: Alexei Starovoitov
>
> On Tue, Apr 13, 2021 at 9:32 AM wrote:
> >
> > > -Original Message-
> > > From: Alexei Starovoitov
> > >
> > > On Tue, Apr 13, 2021 at 9:19 AM wrote:
> > > >
> > > > > -Original Message-
> > > > > From: Alexei Starovoitov
> > > > >
> > > > > On Tue, Apr 13, 2021 at 9:10 AM wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -Original Message-
> > > > > > > From: Alexei Starovoitov
> > > > > > >
> > > > > > > On Tue, Apr 13, 2021 at 2:52 AM Yang Li
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Fix the following coccicheck warnings:
> > > > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:189:7-11:
> > > > > > > > WARNING
> > > > > > > > comparing pointer to 0, suggest !E
> > > > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:361:7-11:
> > > > > > > > WARNING
> > > > > > > > comparing pointer to 0, suggest !E
> > > > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:386:14-18:
> > > > > > > > WARNING
> > > > > > > > comparing pointer to 0, suggest !E
> > > > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:402:14-18:
> > > > > > > > WARNING
> > > > > > > > comparing pointer to 0, suggest !E
> > > > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:433:7-11:
> > > > > > > > WARNING
> > > > > > > > comparing pointer to 0, suggest !E
> > > > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:534:14-18:
> > > > > > > > WARNING
> > > > > > > > comparing pointer to 0, suggest !E
> > > > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:625:7-11:
> > > > > > > > WARNING
> > > > > > > > comparing pointer to 0, suggest !E
> > > > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:767:7-11:
> > > > > > > > WARNING
> > > > > > > > comparing pointer to 0, suggest !E
> > > > > > > >
> > > > > > > > Reported-by: Abaci Robot
> > > > > > > > Signed-off-by: Yang Li
> > > > > > > > ---
> > > > > > > > tools/testing/selftests/bpf/progs/profiler.inc.h | 22
> > > > > > > > +++---
> > > > > > > > 1 file changed, 11 insertions(+), 11 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > > > > > b/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > > > > > index 4896fdf8..a33066c 100644
> > > > > > > > --- a/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > > > > > +++ b/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > > > > > @@ -189,7 +189,7 @@ static INLINE void
> > > > > > > > populate_ancestors(struct task_struct* task,
> > > > > > > > #endif
> > > > > > > > for (num_ancestors = 0; num_ancestors < MAX_ANCESTORS;
> > > > > > > > num_ancestors++) {
> > > > > > > > parent = BPF_CORE_READ(parent, real_parent);
> > > > > > > > - if (parent == NULL)
> > > > > > > > + if (!parent)
> > > > > > >
> > > > > > > Sorry, but I'd like the progs to stay as close as possible to the
> > > > > > > way
> > > > > > > they were written.
> > > > > > Why?
> > > > > >
> > > > > > > They might not adhere to kernel coding style in some cases.
> > > > > > > The code could be grossly inefficient and even buggy.
> > > > > > There would have to be a really good reason to accept
> > > > > > grossly inefficient and even buggy code into the kernel.
> > > > > >
> > > > > > Can you please explain what that reason is?
> > > > >
> > > > > It's not the kernel. It's a test of bpf program.
> > > > That doesn't answer the question of why you don't want any changes.
> > > >
> > > > Why would we not use kernel coding style guidelines and quality
> > > > thresholds for
> > > > testing code? This *is* going into the kernel source tree, where it
> > > > will be
> > > > maintained and used by other developers.
> > >
> > > because the way the C code is written makes llvm generate a particular
> > > code pattern that may not be seen otherwise.
> > > Like removing 'if' because it's useless to humans, but not to the compiler
> > > will change generated code which may or may not trigger the behavior
> > > the prog intends to cover.
> > > In particular this profiler.inc.h test is compiled three different ways to
> > > maximize code generation differences.
> > > It may not be checking error paths in some cases which can be considered
> > > a bug, but that's the intended behavior of the C code as it was written.
> > > So it has nothing to do with "quality of kernel code".
> > > and it should not be used by developers. It's neither sample nor example.
> >
> > Ok - in this case it looks like a program, but it is essentially test data
> > (for testing
> > the compiler). Thanks for the explanation.
>
> yes. That's a good way of saying it.
> Of course not all tests are like this.
> Majority of bpf progs in selftests/bpf/progs/ are carefully written,
> short and designed
> as a unit test. While few are "test data" for llvm.
Thanks. It
RE: [PATCH] selftests/bpf: use !E instead of comparing with NULL
> -Original Message-
> From: Alexei Starovoitov
>
> On Tue, Apr 13, 2021 at 9:19 AM wrote:
> >
> > > -Original Message-
> > > From: Alexei Starovoitov
> > >
> > > On Tue, Apr 13, 2021 at 9:10 AM wrote:
> > > >
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Alexei Starovoitov
> > > > >
> > > > > On Tue, Apr 13, 2021 at 2:52 AM Yang Li
> > > > > wrote:
> > > > > >
> > > > > > Fix the following coccicheck warnings:
> > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:189:7-11: WARNING
> > > > > > comparing pointer to 0, suggest !E
> > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:361:7-11: WARNING
> > > > > > comparing pointer to 0, suggest !E
> > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:386:14-18:
> > > > > > WARNING
> > > > > > comparing pointer to 0, suggest !E
> > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:402:14-18:
> > > > > > WARNING
> > > > > > comparing pointer to 0, suggest !E
> > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:433:7-11: WARNING
> > > > > > comparing pointer to 0, suggest !E
> > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:534:14-18:
> > > > > > WARNING
> > > > > > comparing pointer to 0, suggest !E
> > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:625:7-11: WARNING
> > > > > > comparing pointer to 0, suggest !E
> > > > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:767:7-11: WARNING
> > > > > > comparing pointer to 0, suggest !E
> > > > > >
> > > > > > Reported-by: Abaci Robot
> > > > > > Signed-off-by: Yang Li
> > > > > > ---
> > > > > > tools/testing/selftests/bpf/progs/profiler.inc.h | 22
> > > > > > +++---
> > > > > > 1 file changed, 11 insertions(+), 11 deletions(-)
> > > > > >
> > > > > > diff --git a/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > > > b/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > > > index 4896fdf8..a33066c 100644
> > > > > > --- a/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > > > +++ b/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > > > @@ -189,7 +189,7 @@ static INLINE void populate_ancestors(struct
> > > > > > task_struct* task,
> > > > > > #endif
> > > > > > for (num_ancestors = 0; num_ancestors < MAX_ANCESTORS;
> > > > > > num_ancestors++) {
> > > > > > parent = BPF_CORE_READ(parent, real_parent);
> > > > > > - if (parent == NULL)
> > > > > > + if (!parent)
> > > > >
> > > > > Sorry, but I'd like the progs to stay as close as possible to the way
> > > > > they were written.
> > > > Why?
> > > >
> > > > > They might not adhere to kernel coding style in some cases.
> > > > > The code could be grossly inefficient and even buggy.
> > > > There would have to be a really good reason to accept
> > > > grossly inefficient and even buggy code into the kernel.
> > > >
> > > > Can you please explain what that reason is?
> > >
> > > It's not the kernel. It's a test of bpf program.
> > That doesn't answer the question of why you don't want any changes.
> >
> > Why would we not use kernel coding style guidelines and quality thresholds
> > for
> > testing code? This *is* going into the kernel source tree, where it will be
> > maintained and used by other developers.
>
> because the way the C code is written makes llvm generate a particular
> code pattern that may not be seen otherwise.
> Like removing 'if' because it's useless to humans, but not to the compiler
> will change generated code which may or may not trigger the behavior
> the prog intends to cover.
> In particular this profiler.inc.h test is compiled three different ways to
> maximize code generation differences.
> It may not be checking error paths in some cases which can be considered
> a bug, but that's the intended behavior of the C code as it was written.
> So it has nothing to do with "quality of kernel code".
> and it should not be used by developers. It's neither sample nor example.
Ok - in this case it looks like a program, but it is essentially test data (for
testing
the compiler). Thanks for the explanation.
-- Tim
RE: [PATCH] selftests/bpf: use !E instead of comparing with NULL
> -Original Message-
> From: Alexei Starovoitov
>
> On Tue, Apr 13, 2021 at 2:52 AM Yang Li wrote:
> >
> > Fix the following coccicheck warnings:
> > ./tools/testing/selftests/bpf/progs/profiler.inc.h:189:7-11: WARNING
> > comparing pointer to 0, suggest !E
> > ./tools/testing/selftests/bpf/progs/profiler.inc.h:361:7-11: WARNING
> > comparing pointer to 0, suggest !E
> > ./tools/testing/selftests/bpf/progs/profiler.inc.h:386:14-18: WARNING
> > comparing pointer to 0, suggest !E
> > ./tools/testing/selftests/bpf/progs/profiler.inc.h:402:14-18: WARNING
> > comparing pointer to 0, suggest !E
> > ./tools/testing/selftests/bpf/progs/profiler.inc.h:433:7-11: WARNING
> > comparing pointer to 0, suggest !E
> > ./tools/testing/selftests/bpf/progs/profiler.inc.h:534:14-18: WARNING
> > comparing pointer to 0, suggest !E
> > ./tools/testing/selftests/bpf/progs/profiler.inc.h:625:7-11: WARNING
> > comparing pointer to 0, suggest !E
> > ./tools/testing/selftests/bpf/progs/profiler.inc.h:767:7-11: WARNING
> > comparing pointer to 0, suggest !E
> >
> > Reported-by: Abaci Robot
> > Signed-off-by: Yang Li
> > ---
> > tools/testing/selftests/bpf/progs/profiler.inc.h | 22
> > +++---
> > 1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/progs/profiler.inc.h
> > b/tools/testing/selftests/bpf/progs/profiler.inc.h
> > index 4896fdf8..a33066c 100644
> > --- a/tools/testing/selftests/bpf/progs/profiler.inc.h
> > +++ b/tools/testing/selftests/bpf/progs/profiler.inc.h
> > @@ -189,7 +189,7 @@ static INLINE void populate_ancestors(struct
> > task_struct* task,
> > #endif
> > for (num_ancestors = 0; num_ancestors < MAX_ANCESTORS;
> > num_ancestors++) {
> > parent = BPF_CORE_READ(parent, real_parent);
> > - if (parent == NULL)
> > + if (!parent)
>
> Sorry, but I'd like the progs to stay as close as possible to the way
> they were written.
Why?
> They might not adhere to kernel coding style in some cases.
> The code could be grossly inefficient and even buggy.
There would have to be a really good reason to accept
grossly inefficient and even buggy code into the kernel.
Can you please explain what that reason is?
> Please don't run spell checks, coccicheck, checkpatch.pl on them.
-- Tim
RE: [PATCH] selftests/bpf: use !E instead of comparing with NULL
> -Original Message-
> From: Alexei Starovoitov
>
> On Tue, Apr 13, 2021 at 9:10 AM wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Alexei Starovoitov
> > >
> > > On Tue, Apr 13, 2021 at 2:52 AM Yang Li
> > > wrote:
> > > >
> > > > Fix the following coccicheck warnings:
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:189:7-11: WARNING
> > > > comparing pointer to 0, suggest !E
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:361:7-11: WARNING
> > > > comparing pointer to 0, suggest !E
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:386:14-18: WARNING
> > > > comparing pointer to 0, suggest !E
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:402:14-18: WARNING
> > > > comparing pointer to 0, suggest !E
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:433:7-11: WARNING
> > > > comparing pointer to 0, suggest !E
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:534:14-18: WARNING
> > > > comparing pointer to 0, suggest !E
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:625:7-11: WARNING
> > > > comparing pointer to 0, suggest !E
> > > > ./tools/testing/selftests/bpf/progs/profiler.inc.h:767:7-11: WARNING
> > > > comparing pointer to 0, suggest !E
> > > >
> > > > Reported-by: Abaci Robot
> > > > Signed-off-by: Yang Li
> > > > ---
> > > > tools/testing/selftests/bpf/progs/profiler.inc.h | 22
> > > > +++---
> > > > 1 file changed, 11 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > b/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > index 4896fdf8..a33066c 100644
> > > > --- a/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > +++ b/tools/testing/selftests/bpf/progs/profiler.inc.h
> > > > @@ -189,7 +189,7 @@ static INLINE void populate_ancestors(struct
> > > > task_struct* task,
> > > > #endif
> > > > for (num_ancestors = 0; num_ancestors < MAX_ANCESTORS;
> > > > num_ancestors++) {
> > > > parent = BPF_CORE_READ(parent, real_parent);
> > > > - if (parent == NULL)
> > > > + if (!parent)
> > >
> > > Sorry, but I'd like the progs to stay as close as possible to the way
> > > they were written.
> > Why?
> >
> > > They might not adhere to kernel coding style in some cases.
> > > The code could be grossly inefficient and even buggy.
> > There would have to be a really good reason to accept
> > grossly inefficient and even buggy code into the kernel.
> >
> > Can you please explain what that reason is?
>
> It's not the kernel. It's a test of bpf program.
That doesn't answer the question of why you don't want any changes.
Why would we not use kernel coding style guidelines and quality thresholds for
testing code? This *is* going into the kernel source tree, where it will be
maintained and used by other developers.
-- Tim
RE: [PATCH] selftests: Fix O= and KBUILD_OUTPUT handling for relative paths
> -Original Message- > From: Shuah Khan on Monday, October 14, 2019 3:45 PM > > Fix O= and KBUILD_OUTPUT handling for relative paths. > > export KBUILD_OUTPUT=../kselftest_size > make TARGETS=size kselftest-all > > or > > make O=../kselftest_size TARGETS=size kselftest-all > > In both of these cases, targets get built in ../kselftest_size which is > a one level up from the size test directory. > > make[1]: Entering directory '/mnt/data/lkml/kselftest_size' > make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \ > ARCH=x86 -C ../../.. headers_install > INSTALL ../kselftest_size/usr/include > gcc -static -ffreestanding -nostartfiles -sget_size.c -o > ../kselftest_size/size/get_size > /usr/bin/ld: cannot open output file ../kselftest_size/size/get_size: No such > file or directory > collect2: error: ld returned 1 exit status > make[3]: *** [../lib.mk:138: ../kselftest_size/size/get_size] Error 1 > make[2]: *** [Makefile:143: all] Error 2 > make[1]: *** [/mnt/data/lkml/linux_5.4/Makefile:1221: kselftest-all] Error 2 > make[1]: Leaving directory '/mnt/data/lkml/kselftest_size' > make: *** [Makefile:179: sub-make] Error 2 > > Use abs_objtree exported by the main Makefile. > > Reported-by: Tim Bird > Signed-off-by: Shuah Khan > --- > tools/testing/selftests/Makefile | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/Makefile > b/tools/testing/selftests/Makefile > index 4cdbae6f4e61..3405aa26a655 100644 > --- a/tools/testing/selftests/Makefile > +++ b/tools/testing/selftests/Makefile > @@ -86,10 +86,10 @@ override LDFLAGS = > endif > > ifneq ($(O),) > - BUILD := $(O) > + BUILD := $(abs_objtree) > else > ifneq ($(KBUILD_OUTPUT),) > - BUILD := $(KBUILD_OUTPUT)/kselftest > + BUILD := $(abs_objtree)/kselftest > else > BUILD := $(shell pwd) > DEFAULT_INSTALL_HDR_PATH := 1 > @@ -102,6 +102,7 @@ include $(top_srcdir)/scripts/subarch.include > ARCH ?= $(SUBARCH) > export KSFT_KHDR_INSTALL_DONE := 1 > export BUILD > +#$(info abd_objtree = $(abs_objtree) BUILD = $(BUILD)) > > # build and run gpio when output directory is the src dir. > # gpio has dependency on tools/gpio and builds tools/gpio > -- This works great. Thanks very much. Tested-by: Tim Bird Acked-by: Tim Bird -- Tim
RE: [BUGFIX PATCH] selftests: Use real temporary working directory for archiving
> -Original Message-
> From: Masami Hiramatsu on Thursday, October 03, 2019 7:13 PM
>
> Use real temporary working directory for generating kselftest
> archive.
>
> tools/testing/selftests/kselftest directory has been used for
> the temporary working directory for making a tar archive from
> gen_kselftest_tar.sh, and it removes the directory for cleanup.
>
> However, since the kselftest directory became a part of the
> repository, it must not be used as a working dir.
I'm not objecting to the test, but I would like to understand this
point better. Under normal circumstances (i.e. when not using O= or
KBUILD_OUTPUT)
the rest of the kernel directory structure holds generated files.
What is the issue with using kselftest to hold generated files?
-- Tim
>
> Introduce mktemp to prepare a temporary working directory
> for archiving kselftests.
>
> Signed-off-by: Masami Hiramatsu
> ---
> tools/testing/selftests/gen_kselftest_tar.sh |8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/gen_kselftest_tar.sh
> b/tools/testing/selftests/gen_kselftest_tar.sh
> index a27e2eec3586..eba1e9987ffc 100755
> --- a/tools/testing/selftests/gen_kselftest_tar.sh
> +++ b/tools/testing/selftests/gen_kselftest_tar.sh
> @@ -38,16 +38,16 @@ main()
> esac
> fi
>
> - install_dir=./kselftest
> + tmpdir=`mktemp -d ./install-XX` || exit 1
>
> # Run install using INSTALL_KSFT_PATH override to generate install
> # directory
> -./kselftest_install.sh
> -tar $copts kselftest${ext} $install_dir
> +./kselftest_install.sh $tmpdir
> +tar $copts kselftest${ext} -C $tmpdir kselftest
> echo "Kselftest archive kselftest${ext} created!"
>
> # clean up install directory
> -rm -rf kselftest
> +rm -rf $tmpdir
> }
>
> main "$@"
bug in KBUILD_OUTPUT handling - for relative paths in kselftest
I found a bug in kselftest KBUILD_OUTPUT handling. The following works: $ cd /home/tbird/work/linux $ export KBUILD_OUTPUT=/home/tbird/work/kbuild $ yes '' | make localmodconfig $ make TARGETS=size kselftest But this doesn't work: $ cd /home/tbird/work/linux $ export KBUILD_OUTPUT=../kbuild $ yes '' | make localmodconfig $ make TARGETS=size kselftest I see the following: make[1]: Entering directory '/home/tbird/work/kbuild' make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \ ARCH=x86 -C ../../.. headers_install INSTALL ../kbuild/kselftest/usr/include gcc -static -ffreestanding -nostartfiles -sget_size.c -o ../kbuild/kselftest/size/get_size /usr/bin/ld: cannot open output file ../kbuild/kselftest/size/get_size: No such file or directory collect2: error: ld returned 1 exit status ../lib.mk:138: recipe for target '../kbuild/kselftest/size/get_size' failed make[3]: *** [../kbuild/kselftest/size/get_size] Error 1 Makefile:136: recipe for target 'all' failed make[2]: *** [all] Error 2 /home/tbird/work/linux/Makefile:1240: recipe for target 'kselftest' failed make[1]: *** [kselftest] Error 2 make[1]: Leaving directory '/home/tbird/work/kbuild' Makefile:179: recipe for target 'sub-make' failed make: *** [sub-make] Error 2 This is due to the relative path for KBUILD_OUTPUT being handled incorrectly (I believe) in tools/testing/selftests/Makefile. There are these lines in the Makefile, which are responsible for creating the output directory: BUILD_TARGET=$$BUILD/$$TARGET mkdir $$BUILD_TARGET -p But these are executed from working directory tools/testing/selftests, so the 'size' directory gets created at tools/testing/kbuild/kselftest/size, instead of /home/tbird/work/kbuild/kselftest/size. I can add some code to the Makefile to change the assignment of the variable BUILD, so that if it is a relative path it is relative to $(top_srcdir) instead of the current directory. But I wanted to check and make sure that I'm not breaking anyone else's workflow. I'm not sure what the expectation would be for someone who did this: $ export KBUILD_OUTPUT=../kbuild ; make -C tools/testing/selftests run_tests But I assume if someone is running the kernel's 'make' from the top-level kernel source directory, and they have a relative KBUILD_OUTPUT directory, then they want that output directory to be relative to the top-level directory and not somewhere else. Should I just code up something for review? I use relative KBUILD_OUTPUT paths for a number of my kernel build scripts, and right now these are incompatible with kselftests. Thanks, -- Tim
RE: [GIT PULL] Kselftest update for Linux 5.4-rc1
> -Original Message- > From: Ingo Molnar on Sunday, September 22, 2019 1:26 AM > > * Linus Torvalds wrote: > > > On Fri, Sep 20, 2019 at 9:35 AM Brendan Higgins > > wrote: > > > > > > Sorry about that. I am surprised that none of the other reviewers > > > brought this up. > > > > I think I'm "special". > > > > There was some other similar change a few years ago, which I > > absolutely hated because of how it broke autocomplete for me. Very few > > other people seemed to react to it. > > FWIW, I am obsessively sensitive to autocomplete and overall source code > file hieararchy and nomenclature details as well, so it's not just you. > > Beyond the muscle memory aspect, nonsensical naming and inanely flat file > hierarchies annoy kernel developers and makes it harder for newbies to > understand the kernel source as well. > > The less clutter, the more organization, the better - and there's very > few valid technical reasons to add any new files or directories to the > top level directory - we should probably *remove* quite a few. > > For example 'firmware/' was recently moved to drivers/firmware/, and in a > similar fashion about a third of the remaining 22 directories should > probably be moved too: > > drwxr-xr-xarch > drwxr-xr-xblock > drwxr-xr-xcerts # move to build/certs/ dir > drwxr-xr-xcrypto # move to kernel/crypto/ or security/crypto/ > drwxr-xr-xDocumentation > drwxr-xr-xdrivers > drwxr-xr-xfs > drwxr-xr-xinclude > drwxr-xr-xinit > drwxr-xr-xipc # move to kernel/ipc/ > drwxr-xr-xkernel > drwxr-xr-xlib > drwxr-xr-xLICENSES > drwxr-xr-xmm > drwxr-xr-xnet > drwxr-xr-xsamples # move to Documentation/samples/ > drwxr-xr-xscripts # move to build/scripts/ This one seems like it would break a lot of workflows, and contributor muscle memory and scripts. get_maintainer.pl and checkpatch.pl are probably in quite a few people's scripts. Also, I'm not sure '/build' is the right destination for this. There are a lot more things in there than just build scripts. If you really want to remove the top level 'scripts', it might be best to put the scripts from top-level '/scripts' into '/tools/scripts', which is mostly empty now. -- Tim ... rest snipped ...
RE: [PATCH] selftests/kselftest/runner.sh: Add 45 second timeout per test
> -Original Message-
> From tim.b...@sony.com
>
> > -Original Message-
> > From: Kees Cook
> >
> > Commit a745f7af3cbd ("selftests/harness: Add 30 second timeout per
> > test") solves the problem of kselftest_harness.h-using binary tests
> > possibly hanging forever. However, scripts and other binaries can still
> > hang forever. This adds a global timeout to each test script run.
> >
> > To make this configurable (e.g. as needed in the "rtc" test case),
> > include a new per-test-directory "settings" file (similar to "config")
> > that can contain kselftest-specific settings. The first recognized field
> > is "timeout".
>
> OK - this is quite interesting. I have had on my to-do list an action
> item to propose the creation of a file (or a standard kerneldoc string)
> to hold CI-related meta-data (of which timeout is one example).
>
> What other meta-data did you have in mind?
>
> I would like (that Fuego, and probably other CI systems would like) to have
> access to data like test dependencies, descriptions, and results
> interpretation
> that would be beneficial for both CI systems (using them to control test
> invocations and scheduling), as
> well as users who are trying to interpret and handle the test results.
> So this concept is a very welcome addition to kselftest.
>
> LTP is in the process of adopting a new system for expressing and handling
> their test meta-data.
> See the discussion at:
> https://lists.yoctoproject.org/pipermail/automated-testing/2019-
> August/000471.html
> and the prototype implementation at:
> https://github.com/metan-ucw/ltp/tree/master/docparse
>
> I realize that that system is coupled pretty tightly to LTP, but conceptually
> some of the same type of information would be valuable for kselftest tests.
> One example of a specific field that would be handy is 'need_root'.
>
> It would be nice to avoid proliferation of such meta-data schemas (that is
> field names), so maybe we can have a discussion about this before adopting
> something?
>
> Just FYI, I'm OK with the name 'timeout'. I think that's pretty much
> universally
> used by all CI runners I'm aware of to indicate the test timeout value. But
> before adopting other fields it would be good to start comparing notes
> and not invent a bunch of new field names for concepts that are already in
> other systems.
>
> >
> > Additionally, this splits the reporting for timeouts into a specific
> > "TIMEOUT" not-ok (and adds exit code reporting in the remaining case).
> >
> > Signed-off-by: Kees Cook
> > ---
> > tools/testing/selftests/kselftest/runner.sh | 36 +++--
> > tools/testing/selftests/rtc/settings| 1 +
> > 2 files changed, 34 insertions(+), 3 deletions(-)
> > create mode 100644 tools/testing/selftests/rtc/settings
> >
> > diff --git a/tools/testing/selftests/kselftest/runner.sh
> > b/tools/testing/selftests/kselftest/runner.sh
> > index 00c9020bdda8..84de7bc74f2c 100644
> > --- a/tools/testing/selftests/kselftest/runner.sh
> > +++ b/tools/testing/selftests/kselftest/runner.sh
> > @@ -3,9 +3,14 @@
> > #
> > # Runs a set of tests in a given subdirectory.
> > export skip_rc=4
> > +export timeout_rc=124
> what are the units here? I presume seconds?
Nevermind. I misread this. This is the return code from the 'timeout'
program, right?
-- Tim
RE: [PATCH] selftests/kselftest/runner.sh: Add 45 second timeout per test
> -Original Message-
> From: shuah
>
> On 9/19/19 12:55 PM, Alexandre Belloni wrote:
> > On 19/09/2019 11:06:44-0700, Kees Cook wrote:
> >> Commit a745f7af3cbd ("selftests/harness: Add 30 second timeout per
> >> test") solves the problem of kselftest_harness.h-using binary tests
> >> possibly hanging forever. However, scripts and other binaries can still
> >> hang forever. This adds a global timeout to each test script run.
> >>
>
> Timeout is good, but really tests should not hang. So we have to somehow
> indicate that the test needs to be fixed.
Well, a test hanging is something that might indicate either a failing
in the test, or the detection of an actual problem.
>
> This timeout is a band-aid and not real solution for the problem. This
> arbitrary value doesn't take into account that the test(s) in that
> particular directory (TARGET) could be running normally and working
> through all the tests.
Yes. Also, having a single timeout value doesn't reflect the need
to test on different hardware with potentially huge differences in
CPU speed (or other resource performance differences).
Since kselftest has a policy of having each test run quickly, this is somewhat
mitigated. But still, some embedded boards are running magnitudes
slower than your "average" enterprise Linux machine. Finding a timeout
that can handle all hardware cases is difficult.
-- Tim
>
> We need some way to differentiate the two cases.
Agreed.
>
> >> To make this configurable (e.g. as needed in the "rtc" test case),
> >> include a new per-test-directory "settings" file (similar to "config")
> >> that can contain kselftest-specific settings. The first recognized field
> >> is "timeout".
> >>
> >
> > Seems good to me. I was also wondering whether this is actually
> > reasonable to have tests running for so long. I wanted to discuss that
> > at LPC but I missed the session.
> >
>
> There is the individual test times and overall kselftest run time. We
> have lots of tests now and it does take long.
>
> >> Additionally, this splits the reporting for timeouts into a specific
> >> "TIMEOUT" not-ok (and adds exit code reporting in the remaining case).
> >>
> >> Signed-off-by: Kees Cook
> >> ---
> >> tools/testing/selftests/kselftest/runner.sh | 36
> +++--
> >> tools/testing/selftests/rtc/settings| 1 +
> >> 2 files changed, 34 insertions(+), 3 deletions(-)
> >> create mode 100644 tools/testing/selftests/rtc/settings
> >>
> >> diff --git a/tools/testing/selftests/kselftest/runner.sh
> b/tools/testing/selftests/kselftest/runner.sh
> >> index 00c9020bdda8..84de7bc74f2c 100644
> >> --- a/tools/testing/selftests/kselftest/runner.sh
> >> +++ b/tools/testing/selftests/kselftest/runner.sh
> >> @@ -3,9 +3,14 @@
> >> #
> >> # Runs a set of tests in a given subdirectory.
> >> export skip_rc=4
> >> +export timeout_rc=124
> >> export logfile=/dev/stdout
> >> export per_test_logging=
> >>
> >> +# Defaults for "settings" file fields:
> >> +# "timeout" how many seconds to let each test run before failing.
> >> +export kselftest_default_timeout=45
> >> +
> >> # There isn't a shell-agnostic way to find the path of a sourced file,
> >> # so we must rely on BASE_DIR being set to find other tools.
> >> if [ -z "$BASE_DIR" ]; then
> >> @@ -24,6 +29,16 @@ tap_prefix()
> >>fi
> >> }
> >>
> >> +tap_timeout()
> >> +{
> >> + # Make sure tests will time out if utility is available.
> >> + if [ -x /usr/bin/timeout ] ; then
> >> + /usr/bin/timeout "$kselftest_timeout" "$1"
> >> + else
> >> + "$1"
> >> + fi
> >> +}
> >> +
> >> run_one()
> >> {
> >>DIR="$1"
> >> @@ -32,6 +47,18 @@ run_one()
> >>
> >>BASENAME_TEST=$(basename $TEST)
> >>
> >> + # Reset any "settings"-file variables.
> >> + export kselftest_timeout="$kselftest_default_timeout"
> >> + # Load per-test-directory kselftest "settings" file.
> >> + settings="$BASE_DIR/$DIR/settings"
> >> + if [ -r "$settings" ] ; then
> >> + while read line ; do
> >> + field=$(echo "$line" | cut -d= -f1)
> >> + value=$(echo "$line" | cut -d= -f2-)
> >> + eval "kselftest_$field"="$value"
> >> + done < "$settings"
> >> + fi
> >> +
> >>TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST"
> >>echo "# $TEST_HDR_MSG"
> >>if [ ! -x "$TEST" ]; then
> >> @@ -44,14 +71,17 @@ run_one()
> >>echo "not ok $test_num $TEST_HDR_MSG"
> >>else
> >>cd `dirname $TEST` > /dev/null
> >> - (./$BASENAME_TEST 2>&1; echo $? >&3) |
> >> + ( tap_timeout ./$BASENAME_TEST 2>&1; echo $? >&3) |
> >>tap_prefix >&4) 3>&1) |
> >>(read xs; exit $xs)) 4>>"$logfile" &&
> >>echo "ok $test_num $TEST_HDR_MSG") ||
> >> - (if [ $? -eq $skip_rc ]; then \
> >> + (rc=$?; \
> >> + if [ $rc -eq $skip_rc ]; then \
> >>echo "not ok
RE: [PATCH] selftests/kselftest/runner.sh: Add 45 second timeout per test
> -Original Message-
> From: Kees Cook
>
> Commit a745f7af3cbd ("selftests/harness: Add 30 second timeout per
> test") solves the problem of kselftest_harness.h-using binary tests
> possibly hanging forever. However, scripts and other binaries can still
> hang forever. This adds a global timeout to each test script run.
>
> To make this configurable (e.g. as needed in the "rtc" test case),
> include a new per-test-directory "settings" file (similar to "config")
> that can contain kselftest-specific settings. The first recognized field
> is "timeout".
OK - this is quite interesting. I have had on my to-do list an action
item to propose the creation of a file (or a standard kerneldoc string)
to hold CI-related meta-data (of which timeout is one example).
What other meta-data did you have in mind?
I would like (that Fuego, and probably other CI systems would like) to have
access to data like test dependencies, descriptions, and results interpretation
that would be beneficial for both CI systems (using them to control test
invocations and scheduling), as
well as users who are trying to interpret and handle the test results.
So this concept is a very welcome addition to kselftest.
LTP is in the process of adopting a new system for expressing and handling
their test meta-data.
See the discussion at:
https://lists.yoctoproject.org/pipermail/automated-testing/2019-August/000471.html
and the prototype implementation at:
https://github.com/metan-ucw/ltp/tree/master/docparse
I realize that that system is coupled pretty tightly to LTP, but conceptually
some of the same type of information would be valuable for kselftest tests.
One example of a specific field that would be handy is 'need_root'.
It would be nice to avoid proliferation of such meta-data schemas (that is
field names), so maybe we can have a discussion about this before adopting
something?
Just FYI, I'm OK with the name 'timeout'. I think that's pretty much
universally
used by all CI runners I'm aware of to indicate the test timeout value. But
before adopting other fields it would be good to start comparing notes
and not invent a bunch of new field names for concepts that are already in
other systems.
>
> Additionally, this splits the reporting for timeouts into a specific
> "TIMEOUT" not-ok (and adds exit code reporting in the remaining case).
>
> Signed-off-by: Kees Cook
> ---
> tools/testing/selftests/kselftest/runner.sh | 36 +++--
> tools/testing/selftests/rtc/settings| 1 +
> 2 files changed, 34 insertions(+), 3 deletions(-)
> create mode 100644 tools/testing/selftests/rtc/settings
>
> diff --git a/tools/testing/selftests/kselftest/runner.sh
> b/tools/testing/selftests/kselftest/runner.sh
> index 00c9020bdda8..84de7bc74f2c 100644
> --- a/tools/testing/selftests/kselftest/runner.sh
> +++ b/tools/testing/selftests/kselftest/runner.sh
> @@ -3,9 +3,14 @@
> #
> # Runs a set of tests in a given subdirectory.
> export skip_rc=4
> +export timeout_rc=124
what are the units here? I presume seconds?
> export logfile=/dev/stdout
> export per_test_logging=
>
> +# Defaults for "settings" file fields:
> +# "timeout" how many seconds to let each test run before failing.
> +export kselftest_default_timeout=45
> +
> # There isn't a shell-agnostic way to find the path of a sourced file,
> # so we must rely on BASE_DIR being set to find other tools.
> if [ -z "$BASE_DIR" ]; then
> @@ -24,6 +29,16 @@ tap_prefix()
> fi
> }
>
> +tap_timeout()
> +{
> + # Make sure tests will time out if utility is available.
> + if [ -x /usr/bin/timeout ] ; then
> + /usr/bin/timeout "$kselftest_timeout" "$1"
> + else
> + "$1"
> + fi
> +}
> +
> run_one()
> {
> DIR="$1"
> @@ -32,6 +47,18 @@ run_one()
>
> BASENAME_TEST=$(basename $TEST)
>
> + # Reset any "settings"-file variables.
> + export kselftest_timeout="$kselftest_default_timeout"
> + # Load per-test-directory kselftest "settings" file.
> + settings="$BASE_DIR/$DIR/settings"
> + if [ -r "$settings" ] ; then
> + while read line ; do
> + field=$(echo "$line" | cut -d= -f1)
> + value=$(echo "$line" | cut -d= -f2-)
> + eval "kselftest_$field"="$value"
> + done < "$settings"
> + fi
> +
> TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST"
> echo "# $TEST_HDR_MSG"
> if [ ! -x "$TEST" ]; then
> @@ -44,14 +71,17 @@ run_one()
> echo "not ok $test_num $TEST_HDR_MSG"
> else
> cd `dirname $TEST` > /dev/null
> - (./$BASENAME_TEST 2>&1; echo $? >&3) |
> + ( tap_timeout ./$BASENAME_TEST 2>&1; echo $? >&3) |
> tap_prefix >&4) 3>&1) |
> (read xs; exit $xs)) 4>>"$logfile" &&
> echo "ok $test_num $TEST_HDR_MSG") ||
> - (if [ $? -eq $skip_rc ]; then \
> +
RE: [PATCH] kunit: add PRINTK dependency
Minor spelling nit.. > -Original Message- > From: Arnd Bergmann > > The vprintk_emit() function is not available when CONFIG_PRINTK > is disabled: > > kunit/test.c:22:9: error: implicit declaration of function 'vprintk_emit' [- > Werror,-Wimplicit-function-declaration] > > I suppose without printk(), there is not much use in kunit > either, so add a Kconfig depenedency here. depenedency -> dependency > > Signed-off-by: Arnd Bergmann > --- > kunit/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kunit/Kconfig b/kunit/Kconfig > index 8541ef95b65a..e80d8af00454 100644 > --- a/kunit/Kconfig > +++ b/kunit/Kconfig > @@ -6,6 +6,7 @@ menu "KUnit support" > > config KUNIT > bool "Enable support for unit tests (KUnit)" > + depends on PRINTK > help > Enables support for kernel unit tests (KUnit), a lightweight unit > testing and mocking framework for the Linux kernel. These tests are > -- > 2.20.0
RE: [PATCH v2] kunit: fix failure to build without printk
> -Original Message-
> From: Brendan Higgins
>
> On Fri, Aug 30, 2019 at 11:22:43PM +, tim.b...@sony.com wrote:
> > > -Original Message-
> > > From: Brendan Higgins
> > >
> > > On Fri, Aug 30, 2019 at 3:46 PM Joe Perches wrote:
> > > >
> > > > On Fri, 2019-08-30 at 21:58 +, tim.b...@sony.com wrote:
> > > > > > From: Joe Perches
> > > > []
> > > > > IMHO %pV should be avoided if possible. Just because people are
> > > > > doing it doesn't mean it should be used when it is not necessary.
> > > >
> > > > Well, as the guy that created %pV, I of course
> > > > have a different opinion.
> > > >
> > > > > > then wouldn't it be easier to pass in the
> > > > > > > kernel level as a separate parameter and then strip off all printk
> > > > > > > headers like this:
> > > > > >
> > > > > > Depends on whether or not you care for overall
> > > > > > object size. Consolidated formats with the
> > > > > > embedded KERN_ like suggested are smaller
> > > > > > overall object size.
> > > > >
> > > > > This is an argument I can agree with. I'm generally in favor of
> > > > > things that lessen kernel size creep. :-)
> > > >
> > > > As am I.
> > >
> > > Sorry, to be clear, we are talking about the object size penalty due
> > > to adding a single parameter to a function. Is that right?
> >
> > Not exactly. The argument is that pre-pending the different KERN_LEVEL
> > strings onto format strings can result in several versions of nearly
> > identical
> strings
> > being compiled into the object file. By parameterizing this (that is,
> > adding
> > '%s' into the format string, and putting the level into the string as an
> argument),
> > it prevents this duplication of format strings.
> >
> > I haven't seen the data on duplication of format strings, and how much this
> > affects it, but little things can add up. Whether it matters in this case
> depends
> > on whether the format strings that kunit uses are also used elsewhere in
> the kernel,
> > and whether these same format strings are used with multiple kernel
> message levels.
> > -- Tim
>
> I thought this portion of the discussion was about whether Joe's version
> of kunit_printk was better or my critique of his version of kunit_printk:
>
> Joe's:
> > > > > -void kunit_printk(const char *level,
> > > > > - const struct kunit *test,
> > > > > - const char *fmt, ...)
> > > > > +void kunit_printk(const struct kunit *test, const char *fmt, ...)
> > > > > {
> > > > > + char lvl[PRINTK_MAX_SINGLE_HEADER_LEN + 1] = "\0";
> > > > > struct va_format vaf;
> > > > > va_list args;
> > > > > + int kern_level;
> > > > >
> > > > > va_start(args, fmt);
> > > > >
> > > > > + while ((kern_level = printk_get_level(fmt)) != 0) {
> > > > > + size_t size = printk_skip_level(fmt) - fmt;
> > > > > +
> > > > > + if (kern_level >= '0' && kern_level <= '7') {
> > > > > + memcpy(lvl, fmt, size);
> > > > > + lvl[size] = '\0';
> > > > > + }
> > > > > + fmt += size;
> > > > > + }
> > > > > +
> > > > > vaf.fmt = fmt;
> > > > > vaf.va =
> > > > >
> > > > > - kunit_vprintk(test, level, );
> > > > > + printk("%s\t# %s %pV\n", lvl, test->name, );
> > > > >
> > > > > va_end(args);
> > > > > }
>
> Mine:
> > void kunit_printk(const char *level,
> > const struct kunit *test,
> > const char *fmt, ...)
> > {
> > struct va_format vaf;
> > va_list args;
> >
> > va_start(args, fmt);
> >
> > + fmt = printk_skip_headers(fmt);
> > +
> > vaf.fmt = fmt;
> > vaf.va =
> >
> > - kunit_vprintk(test, level, );
> > + printk("%s\t# %s %pV\n", level, test->name, );
> >
> > va_end(args);
> > }
>
> I thought you and Joe were arguing that "Joe's" resulted in a smaller
> object size than "Mine" (not to be confused with the actual patch I
> presented here, which is what Sergey suggested I do on a different
> thread).
>
> I really don't feel strongly about what Sergey suggested I do (which is
> what this patch originally introduced), versus, what Joe suggested,
> versus what I suggested in response to Joe (or any of the things
> suggested on other threads). I just want to pick one, fix the breakage
> in linux-next, and move on with my life.
When in doubt, do what the sub-system maintainer says. I'd go
with Sergey's suggestion. Maintainers often are juggling a host
of issues, and weighing new features and usages of their system
against their long-term plans for their sub-system. Sometimes
they have time to communicate all the intricacies of their
counter-proposals, and sometimes not.
But they know their system best, and much more often than not
provide sound advice.
If you don't have a strong feeling about it, just do what they
say.
-- Tim
RE: [PATCH v2] kunit: fix failure to build without printk
> -Original Message- > From: Joe Perches > > On Fri, 2019-08-30 at 21:58 +, tim.b...@sony.com wrote: > > > From: Joe Perches > [] > > IMHO %pV should be avoided if possible. Just because people are > > doing it doesn't mean it should be used when it is not necessary. > > Well, as the guy that created %pV, I of course > have a different opinion. LOL. Well I stepped in that one. I don't have any data to support my position on this particular printk feature, but having worked for a while on stack size reduction for a few Sony products, I'm always a bit leery of recursive routines in the kernel. I vaguely recall some recursive printk routines giving me problems on a product that used a sub-4K stack configuration I did many years ago. I don't recall if it was specifically %pV or not. Anyway YMMV. -- Tim
RE: [PATCH v2] kunit: fix failure to build without printk
> -Original Message- > From: Brendan Higgins > > On Fri, Aug 30, 2019 at 3:46 PM Joe Perches wrote: > > > > On Fri, 2019-08-30 at 21:58 +, tim.b...@sony.com wrote: > > > > From: Joe Perches > > [] > > > IMHO %pV should be avoided if possible. Just because people are > > > doing it doesn't mean it should be used when it is not necessary. > > > > Well, as the guy that created %pV, I of course > > have a different opinion. > > > > > > then wouldn't it be easier to pass in the > > > > > kernel level as a separate parameter and then strip off all printk > > > > > headers like this: > > > > > > > > Depends on whether or not you care for overall > > > > object size. Consolidated formats with the > > > > embedded KERN_ like suggested are smaller > > > > overall object size. > > > > > > This is an argument I can agree with. I'm generally in favor of > > > things that lessen kernel size creep. :-) > > > > As am I. > > Sorry, to be clear, we are talking about the object size penalty due > to adding a single parameter to a function. Is that right? Not exactly. The argument is that pre-pending the different KERN_LEVEL strings onto format strings can result in several versions of nearly identical strings being compiled into the object file. By parameterizing this (that is, adding '%s' into the format string, and putting the level into the string as an argument), it prevents this duplication of format strings. I haven't seen the data on duplication of format strings, and how much this affects it, but little things can add up. Whether it matters in this case depends on whether the format strings that kunit uses are also used elsewhere in the kernel, and whether these same format strings are used with multiple kernel message levels. -- Tim
RE: [PATCH v2] kunit: fix failure to build without printk
> -Original Message-
> From: Joe Perches
>
> On Fri, 2019-08-30 at 11:38 -0700, Brendan Higgins wrote:
> > On Thu, Aug 29, 2019 at 09:44:58PM -0700, Joe Perches wrote:
> > > On Thu, 2019-08-29 at 11:01 -0600, shuah wrote:
> > > > On 8/28/19 3:49 AM, Sergey Senozhatsky wrote:
> > > > > On (08/28/19 02:31), Brendan Higgins wrote:
> > > > > [..]
> > > > > > Previously KUnit assumed that printk would always be present,
> which is
> > > > > > not a valid assumption to make. Fix that by removing call to
> > > > > > vprintk_emit, and calling printk directly.
> > > > > >
> > > > > > Reported-by: Randy Dunlap
> > > > > > Link: https://lore.kernel.org/linux-kselftest/0352fae9-564f-4a97-
> 715a-fabe01625...@kernel.org/T/#t
> > > > > > Cc: Stephen Rothwell
> > > > > > Cc: Sergey Senozhatsky
> > > > > > Signed-off-by: Brendan Higgins
> > > > >
> > > > > [..]
> > > > >
> > > > > > -static void kunit_vprintk(const struct kunit *test,
> > > > > > - const char *level,
> > > > > > - struct va_format *vaf)
> > > > > > -{
> > > > > > - kunit_printk_emit(level[1] - '0', "\t# %s: %pV", test->name,
> vaf);
> > > > > > -}
> > > > >
> > > > > This patch looks good to me. I like the removal of recursive
> > > > > vsprintf() (%pV).
> > > > >
> > > > > -ss
> > > > >
> > > >
> > > > Hi Sergey,
> > > >
> > > > What are the guidelines for using printk(). I recall some discussion
> > > > about not using printk(). I am seeing the following from checkpatch
> > > > script:
> > > >
> > > >
> > > > WARNING: Prefer [subsystem eg: netdev]_level([subsystem]dev, ...
> then
> > > > dev_level(dev, ... then pr_level(... to printk(KERN_LEVEL ...
> > > > #105: FILE: include/kunit/test.h:343:
> > > > + printk(KERN_LEVEL "\t# %s: " fmt, (test)->name, ##__VA_ARGS__)
> > > >
> > > >
> > > > Is there supposed to be pr_level() - I can find dev_level()
> > > >
> > > > cc'ing Joe Perches for his feedback on this message recommending
> > > > pr_level() which isn't in 5.3.
> > >
> > > I don't care for pr_level or KERN_LEVEL in a printk.
> >
> > I don't think I follow, how does your version fix this?
> >
> > > I think this is somewhat overly complicated.
> > >
> > > I think I'd write it like:
> > > ---
> > > include/kunit/test.h | 11 -
> > > kunit/test.c | 69
> > >
> > > 2 files changed, 26 insertions(+), 54 deletions(-)
> > >
> > > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > > index 8b7eb03d4971..aa4abf0a22a5 100644
> > > --- a/include/kunit/test.h
> > > +++ b/include/kunit/test.h
> > > @@ -339,9 +339,8 @@ static inline void *kunit_kzalloc(struct kunit *test,
> size_t size, gfp_t gfp)
> > >
> > > void kunit_cleanup(struct kunit *test);
> > >
> > > -void __printf(3, 4) kunit_printk(const char *level,
> > > - const struct kunit *test,
> > > - const char *fmt, ...);
> > > +__printf(2, 3)
> > > +void kunit_printk(const struct kunit *test, const char *fmt, ...);
> > >
> > > /**
> > > * kunit_info() - Prints an INFO level message associated with @test.
> > > @@ -353,7 +352,7 @@ void __printf(3, 4) kunit_printk(const char *level,
> > > * Takes a variable number of format parameters just like printk().
> > > */
> > > #define kunit_info(test, fmt, ...) \
> > > - kunit_printk(KERN_INFO, test, fmt, ##__VA_ARGS__)
> > > + kunit_printk(test, KERN_INFO fmt, ##__VA_ARGS__)
> > >
> > > /**
> > > * kunit_warn() - Prints a WARN level message associated with @test.
> > > @@ -364,7 +363,7 @@ void __printf(3, 4) kunit_printk(const char *level,
> > > * Prints a warning level message.
> > > */
> > > #define kunit_warn(test, fmt, ...) \
> > > - kunit_printk(KERN_WARNING, test, fmt, ##__VA_ARGS__)
> > > + kunit_printk(test, KERN_WARNING fmt, ##__VA_ARGS__)
> > >
> > > /**
> > > * kunit_err() - Prints an ERROR level message associated with @test.
> > > @@ -375,7 +374,7 @@ void __printf(3, 4) kunit_printk(const char *level,
> > > * Prints an error level message.
> > > */
> > > #define kunit_err(test, fmt, ...) \
> > > - kunit_printk(KERN_ERR, test, fmt, ##__VA_ARGS__)
> > > + kunit_printk(test, KERN_ERR fmt, ##__VA_ARGS__)
> > >
> > > /**
> > > * KUNIT_SUCCEED() - A no-op expectation. Only exists for code clarity.
> > > diff --git a/kunit/test.c b/kunit/test.c
> > > index b2ca9b94c353..ddb9bffb5a5d 100644
> > > --- a/kunit/test.c
> > > +++ b/kunit/test.c
> > > @@ -16,40 +16,6 @@ static void kunit_set_failure(struct kunit *test)
> > > WRITE_ONCE(test->success, false);
> > > }
> > >
> > > -static int kunit_vprintk_emit(int level, const char *fmt, va_list args)
> > > -{
> > > - return vprintk_emit(0, level, NULL, 0, fmt, args);
> > > -}
> > > -
> > > -static int kunit_printk_emit(int level, const char *fmt, ...)
> > > -{
> > > - va_list args;
> > > - int ret;
> > > -
> > > - va_start(args, fmt);
> > > - ret = kunit_vprintk_emit(level, fmt, args);
> > >
RE: [PATCH v1] kunit: fix failure to build without printk
> -Original Message-
> From: Brendan Higgins
>
> On Tue, Aug 27, 2019 at 3:00 PM shuah wrote:
> >
> > On 8/27/19 3:36 PM, Brendan Higgins wrote:
> > > On Tue, Aug 27, 2019 at 2:09 PM Brendan Higgins
> > > wrote:
> > >>
> > >> On Tue, Aug 27, 2019 at 2:03 PM Brendan Higgins
> > >> wrote:
> > >>>
> > >>> On Tue, Aug 27, 2019 at 1:21 PM shuah wrote:
> >
> > On 8/27/19 11:49 AM, Brendan Higgins wrote:
> > > Previously KUnit assumed that printk would always be present,
> which is
> > > not a valid assumption to make. Fix that by ifdefing out functions
> which
> > > directly depend on printk core functions similar to what dev_printk
> > > does.
> > >
> > > Reported-by: Randy Dunlap
> > > Link: https://lore.kernel.org/linux-kselftest/0352fae9-564f-4a97-
> 715a-fabe01625...@kernel.org/T/#t
> > > Cc: Stephen Rothwell
> > > Signed-off-by: Brendan Higgins
> > > ---
> > >include/kunit/test.h | 7 +++
> > >kunit/test.c | 41 -
> > >2 files changed, 31 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > > index 8b7eb03d4971..339af5f95c4a 100644
> > > --- a/include/kunit/test.h
> > > +++ b/include/kunit/test.h
> > > @@ -339,9 +339,16 @@ static inline void *kunit_kzalloc(struct kunit
> *test, size_t size, gfp_t gfp)
> > >>> [...]
> > Okay after reviewing this, I am not sure why you need to do all
> > this.
> >
> > Why can't you just change the root function that throws the warn:
> >
> > static int kunit_vprintk_emit(int level, const char *fmt, va_list
> > args)
> > {
> > return vprintk_emit(0, level, NULL, 0, fmt, args);
> > }
> >
> > You aren'r really doing anything extra here, other than calling
> > vprintk_emit()
> > >>>
> > >>> Yeah, I did that a while ago. I think it was a combination of trying
> > >>> to avoid an extra layer of adding and then removing the log level, and
> > >>> that's what dev_printk and friends did.
> > >>>
> > >>> But I think you are probably right. It's a lot of maintenance overhead
> > >>> to get rid of that. Probably best to just use what the printk people
> > >>> have.
> > >>>
> > Unless I am missing something, can't you solve this problem by
> including
> > printk.h and let it handle the !CONFIG_PRINTK case?
> > >>>
> > >>> Randy, I hope you don't mind, but I am going to ask you to re-ack my
> > >>> next revision since it basically addresses the problem in a totally
> > >>> different way.
> > >>
> > >> Actually, scratch that. Checkpatch doesn't like me calling printk
> > >> without using a KERN_.
> > >>
> > >> Now that I am thinking back to when I wrote this. I think it also
> > >> might not like using a dynamic KERN_ either (printk("%s my
> > >> message", KERN_INFO)).
> > >>
> > >> I am going to have to do some more investigation.
> > >
> > > Alright, I am pretty sure it is safe to do printk("%smessage",
> KERN_);
> > >
> > > Looking at the printk implementation, it appears to do the format
> > > before it checks the log level:
> > >
> > >
> https://elixir.bootlin.com/linux/v5.2.10/source/kernel/printk/printk.c#L1907
> > >
> > > So I am pretty sure we can do it either with the vprintk_emit or with
> printk.
> >
> > Let me see if we are on the same page first. I am asking if you can
> > just include printk.h for vprintk_emit() define for both CONFIG_PRINTK
> > and !CONFIG_PRINTK cases.
>
> Ah sorry, I misunderstood you.
>
> No, that doesn't work. I tried including linux/printk.h, and I get the
> same error.
>
> The reason for this is that vprintk_emit() is only defined when
> CONFIG_PRINTK=y:
>
> https://elixir.bootlin.com/linux/latest/ident/vprintk_emit
Ugh. That's just a bug in include/linux/printk.h
There should be a stub definition for vprintk_emit() in the #else part
of #ifdef CONFIG_PRINTK.
You shouldn't be dealing with whether printk is present or not
in the kunit code. All the printk-related routines are supposed
to evaporate themselves, based on the conditional in
include/linux/printk.h
That should be fixed there instead of in your code.
Let me know if you'd like me to submit a patch for that. I only hesitate
because your patch depends on it, and IMHO it makes more sense to
include it in your batch than separately. Otherwise there's a patch race
condition.
-- Tim
RE: [PATCH] selftests: net: tcp_fastopen_backup_key.sh: fix shellcheck issue
> -Original Message-
> From: Anders Roxell
>
> When running tcp_fastopen_backup_key.sh the following issue was seen in
> a busybox environment.
> ./tcp_fastopen_backup_key.sh: line 33: [: -ne: unary operator expected
>
> Shellcheck showed the following issue.
> $ shellcheck tools/testing/selftests/net/tcp_fastopen_backup_key.sh
>
> In tools/testing/selftests/net/tcp_fastopen_backup_key.sh line 33:
> if [ $val -ne 0 ]; then
> ^-- SC2086: Double quote to prevent globbing and word splitting.
>
> Rework to add double quotes around the variable 'val' that shellcheck
> recommends.
>
> Signed-off-by: Anders Roxell
> ---
> tools/testing/selftests/net/tcp_fastopen_backup_key.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/net/tcp_fastopen_backup_key.sh
> b/tools/testing/selftests/net/tcp_fastopen_backup_key.sh
> index 41476399e184..ba5ec3eb314e 100755
> --- a/tools/testing/selftests/net/tcp_fastopen_backup_key.sh
> +++ b/tools/testing/selftests/net/tcp_fastopen_backup_key.sh
> @@ -30,7 +30,7 @@ do_test() {
> ip netns exec "${NETNS}" ./tcp_fastopen_backup_key "$1"
> val=$(ip netns exec "${NETNS}" nstat -az | \
> grep TcpExtTCPFastOpenPassiveFail | awk '{print $2}')
> - if [ $val -ne 0 ]; then
> + if [ "$val" -ne 0 ]; then
Did you test this in the failing environment?
With a busybox shell, I get:
$ [ "" -ne 0 ]
sh: bad number
You might need to explicitly check for empty string here, or switch to a string
comparison instead:
if [ "$val" != 0 ]; then
-- Tim
> echo "FAIL: TcpExtTCPFastOpenPassiveFail non-zero"
> return 1
> fi
> --
> 2.20.1
RE: Linux Testing Microconference at LPC
> -Original Message- > From: Dmitry Vyukov > On Fri, Apr 26, 2019 at 11:03 PM Tim Bird wrote: > > > > I'm in the process now of planning Automated Testing Summit 2019, > > which is tentatively planned for Lyon, France on October 31. This is > > This is _November_ 1, right? No. Thursday, October 31, 2019. Is there some conflict on Thursday? -- Tim
RE: Linux Testing Microconference at LPC
> -Original Message- > From: Sasha Levin > > On Fri, Apr 26, 2019 at 02:02:53PM -0700, Tim Bird wrote: ... > > > >With regards to the Testing microconference at Plumbers, I would like > >to do a presentation on the current status of test standards and test > >framework interoperability. We recently had some good meetings > >between the LAVA and Fuego people at Linaro Connect > >on this topic. > > Hi Tim, > > Sorry for the delayed response, this mail got marked as read as a result > of fat fingers :( > > I'd want to avoid having an 'overview' talk as part of the MC. We have > quite a few discussion topics this year and in the spirit of LPC I'd > prefer to avoid presentations. OK. Sounds good. > Maybe it's more appropriate for the refereed track? I'll consider submitting it there, but there's a certain "fun" aspect to attending a conference that I don't have to prepare a talk for. :-) Thanks for getting back to me. I'm already registered for Plumbers, so I'll see you there. -- Tim
RE: [Ksummit-discuss] The linux devs can rescind their license grant.
> -Original Message- > From: Al Viro > > On Sat, Oct 27, 2018 at 06:52:44AM +, visionsofal...@redchan.it wrote: > > Al: the FSF was so insistent on the adoption of the GPL version 3 > > because the GPL version 2 is not operative against the grantor. > > Anonymous wankstain: sod off and learn to troll properly. It *is* an art > form, and the one you are clearly not up to. > > D-. For the effort and successful use of spellchecker. The style and > contents are about F to E-, unfortunately, so take that in the spirit > in which it is offered. As a participation award, that is. Al, Can you please, even in the face of comments you find irritating, keep your responses more civil? Calling someone a "wankstain" is unprofessional, and we're trying to raise the level of discourse here. > > *plonk* I think this part of the response was sufficient to communicate that you do not take the suggestions of the other party seriously. And it communicates to others the right approach. If someone thinks that another person is acting in bad faith, I think it's better to just stop listening to that person, and let that person know it, and to let other community members know it. De-escalation is preferable to engagement when working with someone who is acting in bad faith. Although I disagree with the approach used in the top portion of this message, I am grateful that you are committed to protecting Linux and our development community. Regards, -- Tim
RE: [Ksummit-discuss] The linux devs can rescind their license grant.
> -Original Message- > From: Al Viro > > On Sat, Oct 27, 2018 at 06:52:44AM +, visionsofal...@redchan.it wrote: > > Al: the FSF was so insistent on the adoption of the GPL version 3 > > because the GPL version 2 is not operative against the grantor. > > Anonymous wankstain: sod off and learn to troll properly. It *is* an art > form, and the one you are clearly not up to. > > D-. For the effort and successful use of spellchecker. The style and > contents are about F to E-, unfortunately, so take that in the spirit > in which it is offered. As a participation award, that is. Al, Can you please, even in the face of comments you find irritating, keep your responses more civil? Calling someone a "wankstain" is unprofessional, and we're trying to raise the level of discourse here. > > *plonk* I think this part of the response was sufficient to communicate that you do not take the suggestions of the other party seriously. And it communicates to others the right approach. If someone thinks that another person is acting in bad faith, I think it's better to just stop listening to that person, and let that person know it, and to let other community members know it. De-escalation is preferable to engagement when working with someone who is acting in bad faith. Although I disagree with the approach used in the top portion of this message, I am grateful that you are committed to protecting Linux and our development community. Regards, -- Tim
RE: [Ksummit-discuss] [PATCH 6/7] Code of Conduct: Change the contact email address
> -Original Message- > From: Alan Cox > > > +to the circumstances. The Code of Conduct Committee is obligated to > > +maintain confidentiality with regard to the reporter of an incident. > > +Further details of specific enforcement policies may be posted > > +separately. > > Unfortunately by ignoring the other suggestions on this you've left this > bit broken. > > The committee can't keep most stuff confidential so it's misleading and > wrong to imply they can. I disagree with this assessment. We have managed to keep most stuff confidential to the general public in the past, to the point where it has been argued that the TAB have not been transparent enough. We're trying to address that issue with the section about quarterly anonymized reports. > Data protection law, reporting laws in some > countries and the like mean that anyone expecting an incident to remain > confidential from the person it was reported against is living in > dreamland and are going to get a nasty shock. OK - you seem to be talking about keeping the incident and reporter confidential from the person reported against. Certainly the person reported against has to have the incident identified to them, so that part is not confidential. Many legal jurisdictions require that the accused can know their accuser. But these things come into play mostly when items have veered into legal territory. Most violation reports are not in the territory. There's no legal requirement that the Code of Conduct committee tell someone who it was that said they were rude on the mailing list. > At the very least it should say '(except where required by law)'. That might be a good to add. It would be helpful, IMHO, to re-visit the patch that's been floating around and see if it can be added on top of this. > There is a separate issue that serious things should always go to law > enforcement - you are setting up a policy akin to the one that got the > catholic church and many others in trouble. > > You should also reserving the right to report serious incidents directly > to law enforcement. Unless of course you want to be forced to sit on > multiple reports of physical abuse from different people about > someone - unable to tell them about each others report, unable to prove > anything, and in twenty years time having to explain to the media why > nothing was done. The scope of the code of conduct basically means that it covers online interactions (communication via mailing list, git commits and Bugzilla). Not to be flippant, but those are hardly mediums that are susceptible to executing physical abuse. Also, they are all mediums that leave a persistent, public trail. So I don't think the comparison is very apt here. -- Tim
RE: [Ksummit-discuss] [PATCH 6/7] Code of Conduct: Change the contact email address
> -Original Message- > From: Alan Cox > > > +to the circumstances. The Code of Conduct Committee is obligated to > > +maintain confidentiality with regard to the reporter of an incident. > > +Further details of specific enforcement policies may be posted > > +separately. > > Unfortunately by ignoring the other suggestions on this you've left this > bit broken. > > The committee can't keep most stuff confidential so it's misleading and > wrong to imply they can. I disagree with this assessment. We have managed to keep most stuff confidential to the general public in the past, to the point where it has been argued that the TAB have not been transparent enough. We're trying to address that issue with the section about quarterly anonymized reports. > Data protection law, reporting laws in some > countries and the like mean that anyone expecting an incident to remain > confidential from the person it was reported against is living in > dreamland and are going to get a nasty shock. OK - you seem to be talking about keeping the incident and reporter confidential from the person reported against. Certainly the person reported against has to have the incident identified to them, so that part is not confidential. Many legal jurisdictions require that the accused can know their accuser. But these things come into play mostly when items have veered into legal territory. Most violation reports are not in the territory. There's no legal requirement that the Code of Conduct committee tell someone who it was that said they were rude on the mailing list. > At the very least it should say '(except where required by law)'. That might be a good to add. It would be helpful, IMHO, to re-visit the patch that's been floating around and see if it can be added on top of this. > There is a separate issue that serious things should always go to law > enforcement - you are setting up a policy akin to the one that got the > catholic church and many others in trouble. > > You should also reserving the right to report serious incidents directly > to law enforcement. Unless of course you want to be forced to sit on > multiple reports of physical abuse from different people about > someone - unable to tell them about each others report, unable to prove > anything, and in twenty years time having to explain to the media why > nothing was done. The scope of the code of conduct basically means that it covers online interactions (communication via mailing list, git commits and Bugzilla). Not to be flippant, but those are hardly mediums that are susceptible to executing physical abuse. Also, they are all mediums that leave a persistent, public trail. So I don't think the comparison is very apt here. -- Tim
RE: [Ksummit-discuss] [PATCH v3 1/3] code-of-conduct: Fix the ambiguity about collecting email addresses
> -Original Message- > From: Frank Rowand > > On 10/18/18 07:56, James Bottomley wrote: > > On Wed, 2018-10-17 at 12:53 -0700, Frank Rowand wrote: > >> On 10/17/18 12:08, James Bottomley wrote: > > [...] > Trying to understand how you are understanding my comment vs what > I intended to communicate, it seems to me that you are focused on > the "where allowed" and I am focused on the "which email > addresses". > > More clear? Or am I still not communicating well enough? > >>> > >>> I think the crux of the disagreement is that you think the carve > >>> out equates to a permission which is not specific enough and I > >>> think it > >> > >> Nope. That is a big place where I was not transferring my thoughts > >> to clear communication. I agree that what I wrote should have been > >> written in terms of carve out instead of permission. > >> > >> > >>> doesn't equate to a permission at all, which is why there's no need > >>> to make it more explicit. Is that a fair characterisation? > >> > >> Nope. My concern is "which email addresses". > > > > The idea here was because it's a carve out that doesn't give permission > > and because the permission is ruled by the project contribution > > documents, the carve out should be broad enough to cover anything they > > might say hence "email addresses not ordinarily collected by the > > project" are still included as unacceptable behaviour. > > > > Perhaps if you propose the wording you'd like to see it would help > > because there still looks to be some subtlety I'm not getting. > > > From the beginning of the thread: > > > @@ -31,7 +31,7 @@ Examples of unacceptable behavior by participants > include: > > * Trolling, insulting/derogatory comments, and personal or political > attacks > > * Public or private harassment > > * Publishing others’ private information, such as a physical or > electronic > > - address, without explicit permission > > + address not ordinarily collected by the project, without explicit > permission > > * Other conduct which could reasonably be considered inappropriate in a > >professional setting > > > Alternative (and I'm sure someone else can probably clean this up a little > bit): > > + address that has been provided in a public space for the project, without > explicit permission This ends up reading like so: Examples of unacceptable behavior by participants include: ... * Publishing others’ private information, such as a physical or electronic address that has been provided in a public space for the project, without explicit permission. I think that in context, you want a 'not' in there. That is: unacceptable behavior includes publishing others' private information... that has *not* been provided in a public space. So, I think the suggested text needs some fixing, IMHO. I looked at this issue upstream, and decided to leave the wording in the CoC itself alone - favoring instead to add a clarifying addition to the upstream CoC FAQ, about some email addresses not being private information. The reason I took that approach, rather than try to change the wording inside the CoC, is that the current wording seems to me to be sufficient. The thing that is unacceptable is publishing private information. The "such as..." clause is intended to convey examples of the types of thing that might usually be considered private information. But it is not exhaustive, nor is it necessarily correct, depending on the circumstances. In particular, email addresses are sometimes private information and sometimes not. In the context of kernel development, many email addresses are not private. I am sympathetic to the argument that we use emails as public information so much in kernel development processes, that it makes sense to omit this or qualify it more. My own views are that: 1) if we change this line at all, we should simply omit the "such as..." part of the phrase, and leave it at: * Publishing others’ private information without explicit permission but also 2) I'm OK with leaving the phrase as is and handling the concerns in an clarifying document. Just my 2 cents. -- Tim
RE: [Ksummit-discuss] [PATCH v3 1/3] code-of-conduct: Fix the ambiguity about collecting email addresses
> -Original Message- > From: Frank Rowand > > On 10/18/18 07:56, James Bottomley wrote: > > On Wed, 2018-10-17 at 12:53 -0700, Frank Rowand wrote: > >> On 10/17/18 12:08, James Bottomley wrote: > > [...] > Trying to understand how you are understanding my comment vs what > I intended to communicate, it seems to me that you are focused on > the "where allowed" and I am focused on the "which email > addresses". > > More clear? Or am I still not communicating well enough? > >>> > >>> I think the crux of the disagreement is that you think the carve > >>> out equates to a permission which is not specific enough and I > >>> think it > >> > >> Nope. That is a big place where I was not transferring my thoughts > >> to clear communication. I agree that what I wrote should have been > >> written in terms of carve out instead of permission. > >> > >> > >>> doesn't equate to a permission at all, which is why there's no need > >>> to make it more explicit. Is that a fair characterisation? > >> > >> Nope. My concern is "which email addresses". > > > > The idea here was because it's a carve out that doesn't give permission > > and because the permission is ruled by the project contribution > > documents, the carve out should be broad enough to cover anything they > > might say hence "email addresses not ordinarily collected by the > > project" are still included as unacceptable behaviour. > > > > Perhaps if you propose the wording you'd like to see it would help > > because there still looks to be some subtlety I'm not getting. > > > From the beginning of the thread: > > > @@ -31,7 +31,7 @@ Examples of unacceptable behavior by participants > include: > > * Trolling, insulting/derogatory comments, and personal or political > attacks > > * Public or private harassment > > * Publishing others’ private information, such as a physical or > electronic > > - address, without explicit permission > > + address not ordinarily collected by the project, without explicit > permission > > * Other conduct which could reasonably be considered inappropriate in a > >professional setting > > > Alternative (and I'm sure someone else can probably clean this up a little > bit): > > + address that has been provided in a public space for the project, without > explicit permission This ends up reading like so: Examples of unacceptable behavior by participants include: ... * Publishing others’ private information, such as a physical or electronic address that has been provided in a public space for the project, without explicit permission. I think that in context, you want a 'not' in there. That is: unacceptable behavior includes publishing others' private information... that has *not* been provided in a public space. So, I think the suggested text needs some fixing, IMHO. I looked at this issue upstream, and decided to leave the wording in the CoC itself alone - favoring instead to add a clarifying addition to the upstream CoC FAQ, about some email addresses not being private information. The reason I took that approach, rather than try to change the wording inside the CoC, is that the current wording seems to me to be sufficient. The thing that is unacceptable is publishing private information. The "such as..." clause is intended to convey examples of the types of thing that might usually be considered private information. But it is not exhaustive, nor is it necessarily correct, depending on the circumstances. In particular, email addresses are sometimes private information and sometimes not. In the context of kernel development, many email addresses are not private. I am sympathetic to the argument that we use emails as public information so much in kernel development processes, that it makes sense to omit this or qualify it more. My own views are that: 1) if we change this line at all, we should simply omit the "such as..." part of the phrase, and leave it at: * Publishing others’ private information without explicit permission but also 2) I'm OK with leaving the phrase as is and handling the concerns in an clarifying document. Just my 2 cents. -- Tim
RE: [RFC v1 06/31] arch: um: enabled running kunit from User Mode Linux
> -Original Message- > From: Brendan Higgins > > On Wed, Oct 17, 2018 at 10:52 AM wrote: > > > > > > It might be of interest to the automated testing mailing list too ? (Tim?) > > > > I think this is interesting to groups doing automated testing of the kernel > > (including myself) as another set of tests to run. Right now I don't see it > > as having any special attributes related to automation. But I could be > wrong. > > Pardon my ignorance, but by automated testing you mean a CI server > with presubmits, nightlys, and things of the sort? Yes. > > If that's the case, KUnit could be helpful because of the low resource > cost in running them and the speed at which they run. True. > There are some > other features we would like to add which would help with that goal as > well like test isolation. We actually have a presubmit server > internally for running KUnit tests that can usually respond to patches > with test results within a couple minutes. Would something like that > be interesting? I think the code and architecture of the software that handles presubmit, mail-list scanning, notifications, etc. would be of interest. But KUnit features themselves (macro definitions, mocking vs. faking, etc.) would not. I only say that in the context of CC-ing the automated testing list on the patch set. Of course the KUnit features are interesting by themselves for testers doing unit testing. -- Tim
RE: [RFC v1 06/31] arch: um: enabled running kunit from User Mode Linux
> -Original Message- > From: Brendan Higgins > > On Wed, Oct 17, 2018 at 10:52 AM wrote: > > > > > > It might be of interest to the automated testing mailing list too ? (Tim?) > > > > I think this is interesting to groups doing automated testing of the kernel > > (including myself) as another set of tests to run. Right now I don't see it > > as having any special attributes related to automation. But I could be > wrong. > > Pardon my ignorance, but by automated testing you mean a CI server > with presubmits, nightlys, and things of the sort? Yes. > > If that's the case, KUnit could be helpful because of the low resource > cost in running them and the speed at which they run. True. > There are some > other features we would like to add which would help with that goal as > well like test isolation. We actually have a presubmit server > internally for running KUnit tests that can usually respond to patches > with test results within a couple minutes. Would something like that > be interesting? I think the code and architecture of the software that handles presubmit, mail-list scanning, notifications, etc. would be of interest. But KUnit features themselves (macro definitions, mocking vs. faking, etc.) would not. I only say that in the context of CC-ing the automated testing list on the patch set. Of course the KUnit features are interesting by themselves for testers doing unit testing. -- Tim
RE: [RFC v1 06/31] arch: um: enabled running kunit from User Mode Linux
> -Original Message-
> From: Kieran Bingham
>
> Hi Brendan,
>
> I very excitedly jumped on these patches to try them out, as this is
> essentially something I was trying to do a few weeks back.
>
> On 17/10/18 00:50, Brendan Higgins wrote:
> > Makes minimum number of changes outside of the KUnit directories for
> > KUnit to build and run using UML.
> >
> > Signed-off-by: Brendan Higgins
> > ---
> > Makefile | 2 +-
> > arch/um/Kconfig.rest | 3 +++
>
> But this file isn't present on v4.19-rc8
>
> It looks like the file is removed at f163977d21a2 ("um: cleanup Kconfig
> files")
>
> What version have you currently based these patches on?
> Do you expect to keep a branch somewhere that's easy to pull in?
>
> Please add me to the CC list as an interested party on later versions :-)
>
> It might be of interest to the automated testing mailing list too ? (Tim?)
I think this is interesting to groups doing automated testing of the kernel
(including myself) as another set of tests to run. Right now I don't see it
as having any special attributes related to automation. But I could be wrong.
-- Tim
RE: [RFC v1 06/31] arch: um: enabled running kunit from User Mode Linux
> -Original Message-
> From: Kieran Bingham
>
> Hi Brendan,
>
> I very excitedly jumped on these patches to try them out, as this is
> essentially something I was trying to do a few weeks back.
>
> On 17/10/18 00:50, Brendan Higgins wrote:
> > Makes minimum number of changes outside of the KUnit directories for
> > KUnit to build and run using UML.
> >
> > Signed-off-by: Brendan Higgins
> > ---
> > Makefile | 2 +-
> > arch/um/Kconfig.rest | 3 +++
>
> But this file isn't present on v4.19-rc8
>
> It looks like the file is removed at f163977d21a2 ("um: cleanup Kconfig
> files")
>
> What version have you currently based these patches on?
> Do you expect to keep a branch somewhere that's easy to pull in?
>
> Please add me to the CC list as an interested party on later versions :-)
>
> It might be of interest to the automated testing mailing list too ? (Tim?)
I think this is interesting to groups doing automated testing of the kernel
(including myself) as another set of tests to run. Right now I don't see it
as having any special attributes related to automation. But I could be wrong.
-- Tim
RE: [RFC v1 00/31] kunit: Introducing KUnit, the Linux kernel unit testing framework
> -Original Message- > From: Brendan Higgins > > This patch set proposes KUnit, a lightweight unit testing and mocking > framework for the Linux kernel. I'm interested in this, and think the kernel might benefit from this, but I have lots of questions. > Unlike Autotest and kselftest, KUnit is a true unit testing framework; > it does not require installing the kernel on a test machine or in a VM > and does not require tests to be written in userspace running on a host > kernel. This is stated here and a few places in the documentation. Just to clarify, KUnit works by compiling the unit under test, along with the test code itself, and then runs it on the machine where the compilation took place? Is this right? How does cross-compiling enter into the equation? If not what I described, then what exactly is happening? Sorry - I haven't had time to look through the patches in detail. Another issue is, what requirements does this place on the tested code? Is extra instrumentation required? I didn't see any, but I didn't look exhaustively at the code. Are all unit tests stored separately from the unit-under-test, or are they expected to be in the same directory? Who is expected to maintain the unit tests? How often are they expected to change? (Would it be every time the unit-under-test changed?) Does the test code require the same level of expertise to write and maintain as the unit-under-test code? That is, could this be a new opportunity for additional developers (especially relative newcomers) to add value to the kernel by writing and maintaining test code, or does this add to the already large burden of code maintenance for our existing maintainers. Thanks, -- Tim ...
RE: [RFC v1 00/31] kunit: Introducing KUnit, the Linux kernel unit testing framework
> -Original Message- > From: Brendan Higgins > > This patch set proposes KUnit, a lightweight unit testing and mocking > framework for the Linux kernel. I'm interested in this, and think the kernel might benefit from this, but I have lots of questions. > Unlike Autotest and kselftest, KUnit is a true unit testing framework; > it does not require installing the kernel on a test machine or in a VM > and does not require tests to be written in userspace running on a host > kernel. This is stated here and a few places in the documentation. Just to clarify, KUnit works by compiling the unit under test, along with the test code itself, and then runs it on the machine where the compilation took place? Is this right? How does cross-compiling enter into the equation? If not what I described, then what exactly is happening? Sorry - I haven't had time to look through the patches in detail. Another issue is, what requirements does this place on the tested code? Is extra instrumentation required? I didn't see any, but I didn't look exhaustively at the code. Are all unit tests stored separately from the unit-under-test, or are they expected to be in the same directory? Who is expected to maintain the unit tests? How often are they expected to change? (Would it be every time the unit-under-test changed?) Does the test code require the same level of expertise to write and maintain as the unit-under-test code? That is, could this be a new opportunity for additional developers (especially relative newcomers) to add value to the kernel by writing and maintaining test code, or does this add to the already large burden of code maintenance for our existing maintainers. Thanks, -- Tim ...
RE: [Ksummit-discuss] [PATCH 2/2] code-of-conduct: Strip the enforcement paragraph pending community discussion
> -Original Message- > From: James Bottomley > > On Mon, 2018-10-08 at 17:58 +, tim.b...@sony.com wrote: > > > -Original Message- > > > From: James Bottomley > > > > > > On Mon, 2018-10-08 at 13:51 +, tim.b...@sony.com wrote: > > > > > -Original Message- > > > > > From: James Bottomley > > > > > On Sat, 2018-10-06 at 21:43 +, tim.b...@sony.com wrote: > > > > > > > -Original Message- > > > > > > > From: James Bottomley > > > > > > > > > > > > > > Significant concern has been expressed about the > > > > > > > responsibilities outlined in the enforcement clause of the > > > > > > > new > > > > > > > code of conduct. Since there is concern that this becomes > > > > > > > binding on the release of the 4.19 kernel, strip the > > > > > > > enforcement clauses to give the community time to consider > > > > > > > and > > > > > > > debate how this should be handled. > > > > > > > > > > > > > > Signed-off-by: James Bottomley > > > > > > > > > > > > > > --- > > > > > > > Documentation/process/code-of-conduct.rst | 15 - > > > > > > > -- > > > > > > > 1 file changed, 15 deletions(-) > > > > > > > > > > > > > > diff --git a/Documentation/process/code-of-conduct.rst > > > > > > > b/Documentation/process/code-of-conduct.rst > > > > > > > index aa40e34e7785..4dd90987305b 100644 > > > > > > > --- a/Documentation/process/code-of-conduct.rst > > > > > > > +++ b/Documentation/process/code-of-conduct.rst > > > > > > > @@ -59,21 +59,6 @@ address, posting via an official social > > > > > > > media account, or acting as an appointed representative at > > > > > > > an > > > > > > > online or offline event. Representation of a project may be > > > > > > > further defined and clarified by project maintainers. > > > > > > > > > > > > > > -Enforcement > > > > > > > -=== > > > > > > > - > > > > > > > -Instances of abusive, harassing, or otherwise unacceptable > > > > > > > behavior may be > > > > > > > -reported by contacting the Technical Advisory Board (TAB) > > > > > > > at > > > > > > > -. All complaints will be > > > > > > > reviewed and > > > > > > > -investigated and will result in a response that is deemed > > > > > > > necessary and > > > > > > > -appropriate to the circumstances. The TAB is obligated to > > > > > > > maintain > > > > > > > -confidentiality with regard to the reporter of an > > > > > > > incident. Further details of > > > > > > > -specific enforcement policies may be posted separately. > > > > > > > > > > > > I think it's OK to leave the above section, as it doesn't > > > > > > speak > > > > > > to enforcement, but rather is just a set of reporting > > > > > > instructions, with an assurance of confidentiality. This > > > > > > seems > > > > > > to me not to be the objectionable part of this section. > > > > > > (IOW, I would omit this removal from the patch). > > > > > > > > > > So I did think about that, but it struck me that with both > > > > > paragraphs removed, the current CoC is very similar to the > > > > > status quo: namely every subsystem handles their own issues and > > > > > that's formalised by the "Our Responsibilities" section. That > > > > > also makes me think that whether we want a centralised channel > > > > > of reporting or enforcement and what it should be also ought to > > > > > be part of the debate. The TAB was created to channel > > > > > community technical input into the Linux Foundation. That's > > > > > not to say it can't provide the reporting and arbitration > > > > > structure, but if we're going to do it right we should debate > > > > > the expansion of its duties (and powers). > > > > > > > > When the Code of Conflict was adopted 3 years ago, we already > > > > created the central reporting mechanism, so I actually think > > > > leaving (ie including) the above paragraph is closer to the > > > > status quo. I think it's the expanded powers and duties (or > > > > perception thereof) that are causing concern and I think debating > > > > those to clarify intent, and adopting changes as needed to > > > > ameliorate concerns is worthwhile. > > > If we want to go back to the status quo, then a plain revert is the > > > patch series I should submit. > > > > Let me try to be more clear. I don't want to go back to the status > > quo. I was saying that if we keep this document, but omit the central > > reporting mechanism, that is a large departure from the status quo, > > because the Code of Conflict already established that. And I think > > that having an ombudsman-type role somewhere in the community > > is beneficial. > > The purpose of this patch is not to be the final point but to take us > up to a useful starting point for Shuah's CoC debate proposal at the > kernel summit (and beyond). Shuah asked that I clarify this in the > commit message, so I will in v2. > > > I believe parts of the Code of Conduct are an improvement over the > > Code of Conflict, so my personal preference would be to keep
RE: [Ksummit-discuss] [PATCH 2/2] code-of-conduct: Strip the enforcement paragraph pending community discussion
> -Original Message- > From: James Bottomley > > On Mon, 2018-10-08 at 17:58 +, tim.b...@sony.com wrote: > > > -Original Message- > > > From: James Bottomley > > > > > > On Mon, 2018-10-08 at 13:51 +, tim.b...@sony.com wrote: > > > > > -Original Message- > > > > > From: James Bottomley > > > > > On Sat, 2018-10-06 at 21:43 +, tim.b...@sony.com wrote: > > > > > > > -Original Message- > > > > > > > From: James Bottomley > > > > > > > > > > > > > > Significant concern has been expressed about the > > > > > > > responsibilities outlined in the enforcement clause of the > > > > > > > new > > > > > > > code of conduct. Since there is concern that this becomes > > > > > > > binding on the release of the 4.19 kernel, strip the > > > > > > > enforcement clauses to give the community time to consider > > > > > > > and > > > > > > > debate how this should be handled. > > > > > > > > > > > > > > Signed-off-by: James Bottomley > > > > > > > > > > > > > > --- > > > > > > > Documentation/process/code-of-conduct.rst | 15 - > > > > > > > -- > > > > > > > 1 file changed, 15 deletions(-) > > > > > > > > > > > > > > diff --git a/Documentation/process/code-of-conduct.rst > > > > > > > b/Documentation/process/code-of-conduct.rst > > > > > > > index aa40e34e7785..4dd90987305b 100644 > > > > > > > --- a/Documentation/process/code-of-conduct.rst > > > > > > > +++ b/Documentation/process/code-of-conduct.rst > > > > > > > @@ -59,21 +59,6 @@ address, posting via an official social > > > > > > > media account, or acting as an appointed representative at > > > > > > > an > > > > > > > online or offline event. Representation of a project may be > > > > > > > further defined and clarified by project maintainers. > > > > > > > > > > > > > > -Enforcement > > > > > > > -=== > > > > > > > - > > > > > > > -Instances of abusive, harassing, or otherwise unacceptable > > > > > > > behavior may be > > > > > > > -reported by contacting the Technical Advisory Board (TAB) > > > > > > > at > > > > > > > -. All complaints will be > > > > > > > reviewed and > > > > > > > -investigated and will result in a response that is deemed > > > > > > > necessary and > > > > > > > -appropriate to the circumstances. The TAB is obligated to > > > > > > > maintain > > > > > > > -confidentiality with regard to the reporter of an > > > > > > > incident. Further details of > > > > > > > -specific enforcement policies may be posted separately. > > > > > > > > > > > > I think it's OK to leave the above section, as it doesn't > > > > > > speak > > > > > > to enforcement, but rather is just a set of reporting > > > > > > instructions, with an assurance of confidentiality. This > > > > > > seems > > > > > > to me not to be the objectionable part of this section. > > > > > > (IOW, I would omit this removal from the patch). > > > > > > > > > > So I did think about that, but it struck me that with both > > > > > paragraphs removed, the current CoC is very similar to the > > > > > status quo: namely every subsystem handles their own issues and > > > > > that's formalised by the "Our Responsibilities" section. That > > > > > also makes me think that whether we want a centralised channel > > > > > of reporting or enforcement and what it should be also ought to > > > > > be part of the debate. The TAB was created to channel > > > > > community technical input into the Linux Foundation. That's > > > > > not to say it can't provide the reporting and arbitration > > > > > structure, but if we're going to do it right we should debate > > > > > the expansion of its duties (and powers). > > > > > > > > When the Code of Conflict was adopted 3 years ago, we already > > > > created the central reporting mechanism, so I actually think > > > > leaving (ie including) the above paragraph is closer to the > > > > status quo. I think it's the expanded powers and duties (or > > > > perception thereof) that are causing concern and I think debating > > > > those to clarify intent, and adopting changes as needed to > > > > ameliorate concerns is worthwhile. > > > If we want to go back to the status quo, then a plain revert is the > > > patch series I should submit. > > > > Let me try to be more clear. I don't want to go back to the status > > quo. I was saying that if we keep this document, but omit the central > > reporting mechanism, that is a large departure from the status quo, > > because the Code of Conflict already established that. And I think > > that having an ombudsman-type role somewhere in the community > > is beneficial. > > The purpose of this patch is not to be the final point but to take us > up to a useful starting point for Shuah's CoC debate proposal at the > kernel summit (and beyond). Shuah asked that I clarify this in the > commit message, so I will in v2. > > > I believe parts of the Code of Conduct are an improvement over the > > Code of Conflict, so my personal preference would be to keep
RE: [Ksummit-discuss] [PATCH 2/2] code-of-conduct: Strip the enforcement paragraph pending community discussion
> -Original Message- > From: James Bottomley > > On Mon, 2018-10-08 at 13:51 +, tim.b...@sony.com wrote: > > > -Original Message- > > > From: James Bottomley > > > On Sat, 2018-10-06 at 21:43 +, tim.b...@sony.com wrote: > > > > > -Original Message- > > > > > From: James Bottomley > > > > > > > > > > Significant concern has been expressed about the > > > > > responsibilities outlined in the enforcement clause of the new > > > > > code of conduct. Since there is concern that this becomes > > > > > binding on the release of the 4.19 kernel, strip the > > > > > enforcement clauses to give the community time to consider and > > > > > debate how this should be handled. > > > > > > > > > > Signed-off-by: James Bottomley > > > > > > > > > > --- > > > > > Documentation/process/code-of-conduct.rst | 15 --- > > > > > 1 file changed, 15 deletions(-) > > > > > > > > > > diff --git a/Documentation/process/code-of-conduct.rst > > > > > b/Documentation/process/code-of-conduct.rst > > > > > index aa40e34e7785..4dd90987305b 100644 > > > > > --- a/Documentation/process/code-of-conduct.rst > > > > > +++ b/Documentation/process/code-of-conduct.rst > > > > > @@ -59,21 +59,6 @@ address, posting via an official social > > > > > media account, or acting as an appointed representative at an > > > > > online or offline event. Representation of a project may be > > > > > further defined and clarified by project maintainers. > > > > > > > > > > -Enforcement > > > > > -=== > > > > > - > > > > > -Instances of abusive, harassing, or otherwise unacceptable > > > > > behavior may be > > > > > -reported by contacting the Technical Advisory Board (TAB) at > > > > > -. All complaints will be > > > > > reviewed and > > > > > -investigated and will result in a response that is deemed > > > > > necessary and > > > > > -appropriate to the circumstances. The TAB is obligated to > > > > > maintain > > > > > -confidentiality with regard to the reporter of an > > > > > incident. Further details of > > > > > -specific enforcement policies may be posted separately. > > > > > > > > I think it's OK to leave the above section, as it doesn't speak > > > > to enforcement, but rather is just a set of reporting > > > > instructions, with an assurance of confidentiality. This seems > > > > to me not to be the objectionable part of this section. > > > > (IOW, I would omit this removal from the patch). > > > > > > So I did think about that, but it struck me that with both > > > paragraphs removed, the current CoC is very similar to the status > > > quo: namely every subsystem handles their own issues and that's > > > formalised by the "Our Responsibilities" section. That also makes > > > me think that whether we want a centralised channel of reporting or > > > enforcement and what it should be also ought to be part of the > > > debate. The TAB was created to channel community technical input > > > into the Linux Foundation. That's not to say it can't provide the > > > reporting and arbitration structure, but if we're going to do it > > > right we should debate the expansion of its duties (and powers). > > > > When the Code of Conflict was adopted 3 years ago, we already created > > the central reporting mechanism, so I actually think leaving (ie > > including) the above paragraph is closer to the status quo. I think > > it's the expanded powers and duties (or perception thereof) that are > > causing concern and I think debating those to clarify intent, and > > adopting changes as needed to ameliorate concerns is worthwhile. > > If we want to go back to the status quo, then a plain revert is the > patch series I should submit. Let me try to be more clear. I don't want to go back to the status quo. I was saying that if we keep this document, but omit the central reporting mechanism, that is a large departure from the status quo, because the Code of Conflict already established that. And I think that having an ombudsman-type role somewhere in the community is beneficial. I believe parts of the Code of Conduct are an improvement over the Code of Conflict, so my personal preference would be to keep it and try to adjust it moving forward. I think your patches, with clear suggestions for improvements (or for deletions in the case where we want more debate on particular sections before adopting them) is a good approach, and I like that process as opposed to starting over from scratch. > > > I believe that in the vast majority of cases, the TAB will end up > > performing a mediator role to smooth hurt feelings and remind and > > encourage improved communication - very similar to what we've done in > > the past. I really believe that bans will continue to be very few > > and far between, as they have been historically (I can only think of > > 3 in the past decade.) > > That might very well be the position the discussion arrives at; > however, I really think making the process fully transparent
RE: [Ksummit-discuss] [PATCH 2/2] code-of-conduct: Strip the enforcement paragraph pending community discussion
> -Original Message- > From: James Bottomley > > On Mon, 2018-10-08 at 13:51 +, tim.b...@sony.com wrote: > > > -Original Message- > > > From: James Bottomley > > > On Sat, 2018-10-06 at 21:43 +, tim.b...@sony.com wrote: > > > > > -Original Message- > > > > > From: James Bottomley > > > > > > > > > > Significant concern has been expressed about the > > > > > responsibilities outlined in the enforcement clause of the new > > > > > code of conduct. Since there is concern that this becomes > > > > > binding on the release of the 4.19 kernel, strip the > > > > > enforcement clauses to give the community time to consider and > > > > > debate how this should be handled. > > > > > > > > > > Signed-off-by: James Bottomley > > > > > > > > > > --- > > > > > Documentation/process/code-of-conduct.rst | 15 --- > > > > > 1 file changed, 15 deletions(-) > > > > > > > > > > diff --git a/Documentation/process/code-of-conduct.rst > > > > > b/Documentation/process/code-of-conduct.rst > > > > > index aa40e34e7785..4dd90987305b 100644 > > > > > --- a/Documentation/process/code-of-conduct.rst > > > > > +++ b/Documentation/process/code-of-conduct.rst > > > > > @@ -59,21 +59,6 @@ address, posting via an official social > > > > > media account, or acting as an appointed representative at an > > > > > online or offline event. Representation of a project may be > > > > > further defined and clarified by project maintainers. > > > > > > > > > > -Enforcement > > > > > -=== > > > > > - > > > > > -Instances of abusive, harassing, or otherwise unacceptable > > > > > behavior may be > > > > > -reported by contacting the Technical Advisory Board (TAB) at > > > > > -. All complaints will be > > > > > reviewed and > > > > > -investigated and will result in a response that is deemed > > > > > necessary and > > > > > -appropriate to the circumstances. The TAB is obligated to > > > > > maintain > > > > > -confidentiality with regard to the reporter of an > > > > > incident. Further details of > > > > > -specific enforcement policies may be posted separately. > > > > > > > > I think it's OK to leave the above section, as it doesn't speak > > > > to enforcement, but rather is just a set of reporting > > > > instructions, with an assurance of confidentiality. This seems > > > > to me not to be the objectionable part of this section. > > > > (IOW, I would omit this removal from the patch). > > > > > > So I did think about that, but it struck me that with both > > > paragraphs removed, the current CoC is very similar to the status > > > quo: namely every subsystem handles their own issues and that's > > > formalised by the "Our Responsibilities" section. That also makes > > > me think that whether we want a centralised channel of reporting or > > > enforcement and what it should be also ought to be part of the > > > debate. The TAB was created to channel community technical input > > > into the Linux Foundation. That's not to say it can't provide the > > > reporting and arbitration structure, but if we're going to do it > > > right we should debate the expansion of its duties (and powers). > > > > When the Code of Conflict was adopted 3 years ago, we already created > > the central reporting mechanism, so I actually think leaving (ie > > including) the above paragraph is closer to the status quo. I think > > it's the expanded powers and duties (or perception thereof) that are > > causing concern and I think debating those to clarify intent, and > > adopting changes as needed to ameliorate concerns is worthwhile. > > If we want to go back to the status quo, then a plain revert is the > patch series I should submit. Let me try to be more clear. I don't want to go back to the status quo. I was saying that if we keep this document, but omit the central reporting mechanism, that is a large departure from the status quo, because the Code of Conflict already established that. And I think that having an ombudsman-type role somewhere in the community is beneficial. I believe parts of the Code of Conduct are an improvement over the Code of Conflict, so my personal preference would be to keep it and try to adjust it moving forward. I think your patches, with clear suggestions for improvements (or for deletions in the case where we want more debate on particular sections before adopting them) is a good approach, and I like that process as opposed to starting over from scratch. > > > I believe that in the vast majority of cases, the TAB will end up > > performing a mediator role to smooth hurt feelings and remind and > > encourage improved communication - very similar to what we've done in > > the past. I really believe that bans will continue to be very few > > and far between, as they have been historically (I can only think of > > 3 in the past decade.) > > That might very well be the position the discussion arrives at; > however, I really think making the process fully transparent
RE: [Ksummit-discuss] [PATCH] code-of-conduct: Remove explicit list of discrimination factors
> -Original Message- > From: Laurent Pinchart > > Hi Tim, > > On Monday, 8 October 2018 17:12:05 EEST tim.b...@sony.com wrote: > > > -Original Message- > > > From: Josh Triplett > > > On Sun, Oct 07, 2018 at 08:18:26PM +0300, Laurent Pinchart wrote: > > >> On Sunday, 7 October 2018 14:35:14 EEST Josh Triplett wrote: > > >>> On Sun, Oct 07, 2018 at 10:51:02AM +0200, Geert Uytterhoeven wrote: > > Providing an explicit list of discrimination factors may give the > > false impression that discrimination based on other unlisted factors > > would be allowed. > > > > Avoid any ambiguity by removing the list, to ensure "a harassment- > > free experience for everyone", period. > > >>> > > >>> I would suggest reading the commit message that added this in the > > >>> first place. "Explicit guidelines have demonstrated success in other > > >>> projects and other areas of the kernel." See also various comparisons > > >>> of codes of conduct, which make the same point. The point of this list > > >>> is precisely to serve as one such explicit guideline; removing it > > >>> would rather defeat the purpose. > > >>> > > >>> In any case, this is not the appropriate place for such patches, any > > >>> more than it's the place for patches to the GPL. > > >> > > >> So what's an appropriate place to discuss the changes that we would > > >> like, *together*, to make to the current document and propose > upstream ? > > > > > > I didn't say "not the appropriate place to discuss" (ksummit-discuss is > > > not ideal but we don't currently have somewhere better), I said "not the > > > appropriate place for such patches". > > > > > > The Linux kernel is by no means the only project using the Contributor > > > Covenant. In general, we don't encourage people working on significant > > > changes to the Linux kernel to work in private for an extended period > > > and only pop up when "done"; rather, we encourage people to start > > > conversations early and include others in the design. Along the same > > > lines, I'd suggest that patches or ideas for patches belong upstream. > > > For instance, the idea of clarifying that email addresses already used > > > on a public mailing list don't count as "private information" seems like > > > a perfectly reasonable suggestion, and one that other projects would > > > benefit from as well. > > > > So I raised this issue with upstream about 2 weeks ago, and here is my > > experience: > > 1) I suggested that the email clarification could be put into the covenant > > itself, or in a supporting FAQ. > > 2) The project maintainer (Coraline Ada Ehmke) was pleasant and > supportive > > of changes to enhance the document, and said either approach would be > fine. > > 3) I noticed that there was a FAQ in progress of being created. > > 4) After thinking about it, I decided that I didn't want to alter the > > language of the covenant, because I didn't want to dilute the expression of > > a need to get permission when revealing private information. > > > > My own opinion is that putting clarifying language in a FAQ is sufficient. > > So I made the following recommendation for the (not yet included > upstream) > > FAQ: > > > > Q: Does the prohibition on publishing private information include email > > addresses sent to a public list? A: No. Information that has voluntarily > > been published to a public location does not fall under the category of > > private information. Such public information may be used within the > context > > of the project according to project norms (such as in commit meta-data in > > code repositories), without that constituting a breach of the CoC. > > > > You can see the history of discussion in these two issues, online: > > https://urldefense.proofpoint.com/v2/url?u=https- > 3A__github.com_ContributorCovenant_contributor- > 5Fcovenant_issues_590=DwICAg=fP4tf--1dS0biCFlB0saz0I0kjO5v7- > GLPtvShAo4cc=rUvFawR4KzgZu1gSN5tuozUn7iTTP0Y- > INWqfY8MsF0=b6Q42NB0w9BZPta7p9Iyr2Lw91cD5dszFL52DzV3FL0=17 > HaUjlX7xwXIvGmJLYhuclrQ1ze-ySl5xLrWIKUDbU= > > https://urldefense.proofpoint.com/v2/url?u=https- > 3A__github.com_ContributorCovenant_contributor- > 5Fcovenant_issues_575=DwICAg=fP4tf--1dS0biCFlB0saz0I0kjO5v7- > GLPtvShAo4cc=rUvFawR4KzgZu1gSN5tuozUn7iTTP0Y- > INWqfY8MsF0=b6Q42NB0w9BZPta7p9Iyr2Lw91cD5dszFL52DzV3FL0=p > MGMapO3n9KVVxipezDC8cn2BvpY2xmJKq0T7p-Gt1E= > > > > I hesitated to post these, because a formatting error in one of the posts > > makes me look a bit dumb. :-) > > > > I don't know what progress is being made adopting the FAQ, but Coraline > > seems very supportive, and I've told here that I will come back and help > > with it if it stalls. > > > > Honestly, I believe Linux will adopt its own FAQ or some similar document, > > so with the Contributor Covenant adopting the clarification as a separate > > document, I don't know if Linux would inherit it (ie include the Covenant > > FAQ in our source tree). However, I think that the
RE: [Ksummit-discuss] [PATCH] code-of-conduct: Remove explicit list of discrimination factors
> -Original Message- > From: Laurent Pinchart > > Hi Tim, > > On Monday, 8 October 2018 17:12:05 EEST tim.b...@sony.com wrote: > > > -Original Message- > > > From: Josh Triplett > > > On Sun, Oct 07, 2018 at 08:18:26PM +0300, Laurent Pinchart wrote: > > >> On Sunday, 7 October 2018 14:35:14 EEST Josh Triplett wrote: > > >>> On Sun, Oct 07, 2018 at 10:51:02AM +0200, Geert Uytterhoeven wrote: > > Providing an explicit list of discrimination factors may give the > > false impression that discrimination based on other unlisted factors > > would be allowed. > > > > Avoid any ambiguity by removing the list, to ensure "a harassment- > > free experience for everyone", period. > > >>> > > >>> I would suggest reading the commit message that added this in the > > >>> first place. "Explicit guidelines have demonstrated success in other > > >>> projects and other areas of the kernel." See also various comparisons > > >>> of codes of conduct, which make the same point. The point of this list > > >>> is precisely to serve as one such explicit guideline; removing it > > >>> would rather defeat the purpose. > > >>> > > >>> In any case, this is not the appropriate place for such patches, any > > >>> more than it's the place for patches to the GPL. > > >> > > >> So what's an appropriate place to discuss the changes that we would > > >> like, *together*, to make to the current document and propose > upstream ? > > > > > > I didn't say "not the appropriate place to discuss" (ksummit-discuss is > > > not ideal but we don't currently have somewhere better), I said "not the > > > appropriate place for such patches". > > > > > > The Linux kernel is by no means the only project using the Contributor > > > Covenant. In general, we don't encourage people working on significant > > > changes to the Linux kernel to work in private for an extended period > > > and only pop up when "done"; rather, we encourage people to start > > > conversations early and include others in the design. Along the same > > > lines, I'd suggest that patches or ideas for patches belong upstream. > > > For instance, the idea of clarifying that email addresses already used > > > on a public mailing list don't count as "private information" seems like > > > a perfectly reasonable suggestion, and one that other projects would > > > benefit from as well. > > > > So I raised this issue with upstream about 2 weeks ago, and here is my > > experience: > > 1) I suggested that the email clarification could be put into the covenant > > itself, or in a supporting FAQ. > > 2) The project maintainer (Coraline Ada Ehmke) was pleasant and > supportive > > of changes to enhance the document, and said either approach would be > fine. > > 3) I noticed that there was a FAQ in progress of being created. > > 4) After thinking about it, I decided that I didn't want to alter the > > language of the covenant, because I didn't want to dilute the expression of > > a need to get permission when revealing private information. > > > > My own opinion is that putting clarifying language in a FAQ is sufficient. > > So I made the following recommendation for the (not yet included > upstream) > > FAQ: > > > > Q: Does the prohibition on publishing private information include email > > addresses sent to a public list? A: No. Information that has voluntarily > > been published to a public location does not fall under the category of > > private information. Such public information may be used within the > context > > of the project according to project norms (such as in commit meta-data in > > code repositories), without that constituting a breach of the CoC. > > > > You can see the history of discussion in these two issues, online: > > https://urldefense.proofpoint.com/v2/url?u=https- > 3A__github.com_ContributorCovenant_contributor- > 5Fcovenant_issues_590=DwICAg=fP4tf--1dS0biCFlB0saz0I0kjO5v7- > GLPtvShAo4cc=rUvFawR4KzgZu1gSN5tuozUn7iTTP0Y- > INWqfY8MsF0=b6Q42NB0w9BZPta7p9Iyr2Lw91cD5dszFL52DzV3FL0=17 > HaUjlX7xwXIvGmJLYhuclrQ1ze-ySl5xLrWIKUDbU= > > https://urldefense.proofpoint.com/v2/url?u=https- > 3A__github.com_ContributorCovenant_contributor- > 5Fcovenant_issues_575=DwICAg=fP4tf--1dS0biCFlB0saz0I0kjO5v7- > GLPtvShAo4cc=rUvFawR4KzgZu1gSN5tuozUn7iTTP0Y- > INWqfY8MsF0=b6Q42NB0w9BZPta7p9Iyr2Lw91cD5dszFL52DzV3FL0=p > MGMapO3n9KVVxipezDC8cn2BvpY2xmJKq0T7p-Gt1E= > > > > I hesitated to post these, because a formatting error in one of the posts > > makes me look a bit dumb. :-) > > > > I don't know what progress is being made adopting the FAQ, but Coraline > > seems very supportive, and I've told here that I will come back and help > > with it if it stalls. > > > > Honestly, I believe Linux will adopt its own FAQ or some similar document, > > so with the Contributor Covenant adopting the clarification as a separate > > document, I don't know if Linux would inherit it (ie include the Covenant > > FAQ in our source tree). However, I think that the
RE: [Ksummit-discuss] [PATCH] code-of-conduct: Remove explicit list of discrimination factors
> -Original Message- > From: Josh Triplett > > On Sun, Oct 07, 2018 at 08:18:26PM +0300, Laurent Pinchart wrote: > > Hi Josh, > > > > On Sunday, 7 October 2018 14:35:14 EEST Josh Triplett wrote: > > > On Sun, Oct 07, 2018 at 10:51:02AM +0200, Geert Uytterhoeven wrote: > > > > Providing an explicit list of discrimination factors may give the false > > > > impression that discrimination based on other unlisted factors would be > > > > allowed. > > > > > > > > Avoid any ambiguity by removing the list, to ensure "a harassment-free > > > > experience for everyone", period. > > > > > > I would suggest reading the commit message that added this in the first > > > place. "Explicit guidelines have demonstrated success in other projects > > > and other areas of the kernel." See also various comparisons of codes of > > > conduct, which make the same point. The point of this list is precisely > > > to serve as one such explicit guideline; removing it would rather defeat > > > the purpose. > > > > > > In any case, this is not the appropriate place for such patches, any > > > more than it's the place for patches to the GPL. > > > > So what's an appropriate place to discuss the changes that we would like, > > *together*, to make to the current document and propose upstream ? > > I didn't say "not the appropriate place to discuss" (ksummit-discuss is > not ideal but we don't currently have somewhere better), I said "not the > appropriate place for such patches". > > The Linux kernel is by no means the only project using the Contributor > Covenant. In general, we don't encourage people working on significant > changes to the Linux kernel to work in private for an extended period > and only pop up when "done"; rather, we encourage people to start > conversations early and include others in the design. Along the same > lines, I'd suggest that patches or ideas for patches belong upstream. > For instance, the idea of clarifying that email addresses already used > on a public mailing list don't count as "private information" seems like > a perfectly reasonable suggestion, and one that other projects would > benefit from as well. So I raised this issue with upstream about 2 weeks ago, and here is my experience: 1) I suggested that the email clarification could be put into the covenant itself, or in a supporting FAQ. 2) The project maintainer (Coraline Ada Ehmke) was pleasant and supportive of changes to enhance the document, and said either approach would be fine. 3) I noticed that there was a FAQ in progress of being created. 4) After thinking about it, I decided that I didn't want to alter the language of the covenant, because I didn't want to dilute the expression of a need to get permission when revealing private information. My own opinion is that putting clarifying language in a FAQ is sufficient. So I made the following recommendation for the (not yet included upstream) FAQ: Q: Does the prohibition on publishing private information include email addresses sent to a public list? A: No. Information that has voluntarily been published to a public location does not fall under the category of private information. Such public information may be used within the context of the project according to project norms (such as in commit meta-data in code repositories), without that constituting a breach of the CoC. You can see the history of discussion in these two issues, online: https://github.com/ContributorCovenant/contributor_covenant/issues/590 https://github.com/ContributorCovenant/contributor_covenant/issues/575 I hesitated to post these, because a formatting error in one of the posts makes me look a bit dumb. :-) I don't know what progress is being made adopting the FAQ, but Coraline seems very supportive, and I've told here that I will come back and help with it if it stalls. Honestly, I believe Linux will adopt its own FAQ or some similar document, so with the Contributor Covenant adopting the clarification as a separate document, I don't know if Linux would inherit it (ie include the Covenant FAQ in our source tree). However, I think that the existence of this email clarification in the upstream FAQ would still have a beneficial effect for all downstream users of the covenant, so I view this as a useful exercise. -- Tim
RE: [Ksummit-discuss] [PATCH] code-of-conduct: Remove explicit list of discrimination factors
> -Original Message- > From: Josh Triplett > > On Sun, Oct 07, 2018 at 08:18:26PM +0300, Laurent Pinchart wrote: > > Hi Josh, > > > > On Sunday, 7 October 2018 14:35:14 EEST Josh Triplett wrote: > > > On Sun, Oct 07, 2018 at 10:51:02AM +0200, Geert Uytterhoeven wrote: > > > > Providing an explicit list of discrimination factors may give the false > > > > impression that discrimination based on other unlisted factors would be > > > > allowed. > > > > > > > > Avoid any ambiguity by removing the list, to ensure "a harassment-free > > > > experience for everyone", period. > > > > > > I would suggest reading the commit message that added this in the first > > > place. "Explicit guidelines have demonstrated success in other projects > > > and other areas of the kernel." See also various comparisons of codes of > > > conduct, which make the same point. The point of this list is precisely > > > to serve as one such explicit guideline; removing it would rather defeat > > > the purpose. > > > > > > In any case, this is not the appropriate place for such patches, any > > > more than it's the place for patches to the GPL. > > > > So what's an appropriate place to discuss the changes that we would like, > > *together*, to make to the current document and propose upstream ? > > I didn't say "not the appropriate place to discuss" (ksummit-discuss is > not ideal but we don't currently have somewhere better), I said "not the > appropriate place for such patches". > > The Linux kernel is by no means the only project using the Contributor > Covenant. In general, we don't encourage people working on significant > changes to the Linux kernel to work in private for an extended period > and only pop up when "done"; rather, we encourage people to start > conversations early and include others in the design. Along the same > lines, I'd suggest that patches or ideas for patches belong upstream. > For instance, the idea of clarifying that email addresses already used > on a public mailing list don't count as "private information" seems like > a perfectly reasonable suggestion, and one that other projects would > benefit from as well. So I raised this issue with upstream about 2 weeks ago, and here is my experience: 1) I suggested that the email clarification could be put into the covenant itself, or in a supporting FAQ. 2) The project maintainer (Coraline Ada Ehmke) was pleasant and supportive of changes to enhance the document, and said either approach would be fine. 3) I noticed that there was a FAQ in progress of being created. 4) After thinking about it, I decided that I didn't want to alter the language of the covenant, because I didn't want to dilute the expression of a need to get permission when revealing private information. My own opinion is that putting clarifying language in a FAQ is sufficient. So I made the following recommendation for the (not yet included upstream) FAQ: Q: Does the prohibition on publishing private information include email addresses sent to a public list? A: No. Information that has voluntarily been published to a public location does not fall under the category of private information. Such public information may be used within the context of the project according to project norms (such as in commit meta-data in code repositories), without that constituting a breach of the CoC. You can see the history of discussion in these two issues, online: https://github.com/ContributorCovenant/contributor_covenant/issues/590 https://github.com/ContributorCovenant/contributor_covenant/issues/575 I hesitated to post these, because a formatting error in one of the posts makes me look a bit dumb. :-) I don't know what progress is being made adopting the FAQ, but Coraline seems very supportive, and I've told here that I will come back and help with it if it stalls. Honestly, I believe Linux will adopt its own FAQ or some similar document, so with the Contributor Covenant adopting the clarification as a separate document, I don't know if Linux would inherit it (ie include the Covenant FAQ in our source tree). However, I think that the existence of this email clarification in the upstream FAQ would still have a beneficial effect for all downstream users of the covenant, so I view this as a useful exercise. -- Tim
RE: [Ksummit-discuss] [PATCH 2/2] code-of-conduct: Strip the enforcement paragraph pending community discussion
> -Original Message- > From: James Bottomley > On Sat, 2018-10-06 at 21:43 +, tim.b...@sony.com wrote: > > > -Original Message- > > > From: James Bottomley > > > > > > Significant concern has been expressed about the responsibilities > > > outlined in the enforcement clause of the new code of > > > conduct. Since there is concern that this becomes binding on the > > > release of the 4.19 kernel, strip the enforcement clauses to give > > > the community time to consider and debate how this should be > > > handled. > > > > > > Signed-off-by: James Bottomley > > > > > > --- > > > Documentation/process/code-of-conduct.rst | 15 --- > > > 1 file changed, 15 deletions(-) > > > > > > diff --git a/Documentation/process/code-of-conduct.rst > > > b/Documentation/process/code-of-conduct.rst > > > index aa40e34e7785..4dd90987305b 100644 > > > --- a/Documentation/process/code-of-conduct.rst > > > +++ b/Documentation/process/code-of-conduct.rst > > > @@ -59,21 +59,6 @@ address, posting via an official social media > > > account, or > > > acting as an appointed > > > representative at an online or offline event. Representation of a > > > project may > > > be > > > further defined and clarified by project maintainers. > > > > > > -Enforcement > > > -=== > > > - > > > -Instances of abusive, harassing, or otherwise unacceptable > > > behavior may be > > > -reported by contacting the Technical Advisory Board (TAB) at > > > -. All complaints will be reviewed > > > and > > > -investigated and will result in a response that is deemed > > > necessary and > > > -appropriate to the circumstances. The TAB is obligated to maintain > > > -confidentiality with regard to the reporter of an > > > incident. Further details of > > > -specific enforcement policies may be posted separately. > > > > I think it's OK to leave the above section, as it doesn't speak to > > enforcement, but rather is just a set of reporting instructions, > > with an assurance of confidentiality. This seems to me not to be > > the objectionable part of this section. > > (IOW, I would omit this removal from the patch). > > So I did think about that, but it struck me that with both paragraphs > removed, the current CoC is very similar to the status quo: namely > every subsystem handles their own issues and that's formalised by the > "Our Responsibilities" section. That also makes me think that whether > we want a centralised channel of reporting or enforcement and what it > should be also ought to be part of the debate. The TAB was created to > channel community technical input into the Linux Foundation. That's > not to say it can't provide the reporting and arbitration structure, > but if we're going to do it right we should debate the expansion of its > duties (and powers). When the Code of Conflict was adopted 3 years ago, we already created the central reporting mechanism, so I actually think leaving (ie including) the above paragraph is closer to the status quo. I think it's the expanded powers and duties (or perception thereof) that are causing concern and I think debating those to clarify intent, and adopting changes as needed to ameliorate concerns is worthwhile. I believe that in the vast majority of cases, the TAB will end up performing a mediator role to smooth hurt feelings and remind and encourage improved communication - very similar to what we've done in the past. I really believe that bans will continue to be very few and far between, as they have been historically (I can only think of 3 in the past decade.) -- Tim
RE: [Ksummit-discuss] [PATCH 2/2] code-of-conduct: Strip the enforcement paragraph pending community discussion
> -Original Message- > From: James Bottomley > On Sat, 2018-10-06 at 21:43 +, tim.b...@sony.com wrote: > > > -Original Message- > > > From: James Bottomley > > > > > > Significant concern has been expressed about the responsibilities > > > outlined in the enforcement clause of the new code of > > > conduct. Since there is concern that this becomes binding on the > > > release of the 4.19 kernel, strip the enforcement clauses to give > > > the community time to consider and debate how this should be > > > handled. > > > > > > Signed-off-by: James Bottomley > > > > > > --- > > > Documentation/process/code-of-conduct.rst | 15 --- > > > 1 file changed, 15 deletions(-) > > > > > > diff --git a/Documentation/process/code-of-conduct.rst > > > b/Documentation/process/code-of-conduct.rst > > > index aa40e34e7785..4dd90987305b 100644 > > > --- a/Documentation/process/code-of-conduct.rst > > > +++ b/Documentation/process/code-of-conduct.rst > > > @@ -59,21 +59,6 @@ address, posting via an official social media > > > account, or > > > acting as an appointed > > > representative at an online or offline event. Representation of a > > > project may > > > be > > > further defined and clarified by project maintainers. > > > > > > -Enforcement > > > -=== > > > - > > > -Instances of abusive, harassing, or otherwise unacceptable > > > behavior may be > > > -reported by contacting the Technical Advisory Board (TAB) at > > > -. All complaints will be reviewed > > > and > > > -investigated and will result in a response that is deemed > > > necessary and > > > -appropriate to the circumstances. The TAB is obligated to maintain > > > -confidentiality with regard to the reporter of an > > > incident. Further details of > > > -specific enforcement policies may be posted separately. > > > > I think it's OK to leave the above section, as it doesn't speak to > > enforcement, but rather is just a set of reporting instructions, > > with an assurance of confidentiality. This seems to me not to be > > the objectionable part of this section. > > (IOW, I would omit this removal from the patch). > > So I did think about that, but it struck me that with both paragraphs > removed, the current CoC is very similar to the status quo: namely > every subsystem handles their own issues and that's formalised by the > "Our Responsibilities" section. That also makes me think that whether > we want a centralised channel of reporting or enforcement and what it > should be also ought to be part of the debate. The TAB was created to > channel community technical input into the Linux Foundation. That's > not to say it can't provide the reporting and arbitration structure, > but if we're going to do it right we should debate the expansion of its > duties (and powers). When the Code of Conflict was adopted 3 years ago, we already created the central reporting mechanism, so I actually think leaving (ie including) the above paragraph is closer to the status quo. I think it's the expanded powers and duties (or perception thereof) that are causing concern and I think debating those to clarify intent, and adopting changes as needed to ameliorate concerns is worthwhile. I believe that in the vast majority of cases, the TAB will end up performing a mediator role to smooth hurt feelings and remind and encourage improved communication - very similar to what we've done in the past. I really believe that bans will continue to be very few and far between, as they have been historically (I can only think of 3 in the past decade.) -- Tim
RE: [Ksummit-discuss] [PATCH 2/2] code-of-conduct: Strip the enforcement paragraph pending community discussion
> -Original Message- > From: James Bottomley > > Significant concern has been expressed about the responsibilities outlined in > the enforcement clause of the new code of conduct. Since there is concern > that this becomes binding on the release of the 4.19 kernel, strip the > enforcement clauses to give the community time to consider and debate > how this > should be handled. > > Signed-off-by: James Bottomley > > --- > Documentation/process/code-of-conduct.rst | 15 --- > 1 file changed, 15 deletions(-) > > diff --git a/Documentation/process/code-of-conduct.rst > b/Documentation/process/code-of-conduct.rst > index aa40e34e7785..4dd90987305b 100644 > --- a/Documentation/process/code-of-conduct.rst > +++ b/Documentation/process/code-of-conduct.rst > @@ -59,21 +59,6 @@ address, posting via an official social media account, or > acting as an appointed > representative at an online or offline event. Representation of a project may > be > further defined and clarified by project maintainers. > > -Enforcement > -=== > - > -Instances of abusive, harassing, or otherwise unacceptable behavior may be > -reported by contacting the Technical Advisory Board (TAB) at > -. All complaints will be reviewed and > -investigated and will result in a response that is deemed necessary and > -appropriate to the circumstances. The TAB is obligated to maintain > -confidentiality with regard to the reporter of an incident. Further details > of > -specific enforcement policies may be posted separately. I think it's OK to leave the above section, as it doesn't speak to enforcement, but rather is just a set of reporting instructions, with an assurance of confidentiality. This seems to me not to be the objectionable part of this section. (IOW, I would omit this removal from the patch). If the next part is indeed removed, then maybe the section needs to be renamed? -- Tim > - > -Maintainers who do not follow or enforce the Code of Conduct in good faith > may > -face temporary or permanent repercussions as determined by other > members of the > -project’s leadership. > - > Attribution > ===
RE: [Ksummit-discuss] [PATCH 2/2] code-of-conduct: Strip the enforcement paragraph pending community discussion
> -Original Message- > From: James Bottomley > > Significant concern has been expressed about the responsibilities outlined in > the enforcement clause of the new code of conduct. Since there is concern > that this becomes binding on the release of the 4.19 kernel, strip the > enforcement clauses to give the community time to consider and debate > how this > should be handled. > > Signed-off-by: James Bottomley > > --- > Documentation/process/code-of-conduct.rst | 15 --- > 1 file changed, 15 deletions(-) > > diff --git a/Documentation/process/code-of-conduct.rst > b/Documentation/process/code-of-conduct.rst > index aa40e34e7785..4dd90987305b 100644 > --- a/Documentation/process/code-of-conduct.rst > +++ b/Documentation/process/code-of-conduct.rst > @@ -59,21 +59,6 @@ address, posting via an official social media account, or > acting as an appointed > representative at an online or offline event. Representation of a project may > be > further defined and clarified by project maintainers. > > -Enforcement > -=== > - > -Instances of abusive, harassing, or otherwise unacceptable behavior may be > -reported by contacting the Technical Advisory Board (TAB) at > -. All complaints will be reviewed and > -investigated and will result in a response that is deemed necessary and > -appropriate to the circumstances. The TAB is obligated to maintain > -confidentiality with regard to the reporter of an incident. Further details > of > -specific enforcement policies may be posted separately. I think it's OK to leave the above section, as it doesn't speak to enforcement, but rather is just a set of reporting instructions, with an assurance of confidentiality. This seems to me not to be the objectionable part of this section. (IOW, I would omit this removal from the patch). If the next part is indeed removed, then maybe the section needs to be renamed? -- Tim > - > -Maintainers who do not follow or enforce the Code of Conduct in good faith > may > -face temporary or permanent repercussions as determined by other > members of the > -project’s leadership. > - > Attribution > ===
