linux-kernel - Re: [PATCH 2/2] perf tools: Cope with differences for lib/list

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZvtWwJ87xzViwZqI@google.com>
Date: Mon, 30 Sep 2024 18:56:16 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Kuan-Wei Chiu <visitorckw@...il.com>, Ingo Molnar <mingo@...nel.org>,
	Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>, linux-kernel@...r.kernel.org,
	linux-perf-users@...r.kernel.org,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 2/2] perf tools: Cope with differences for
 lib/list_sort.c copy from the kernel

On Mon, Sep 30, 2024 at 05:21:36PM -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@...hat.com>
> 
> With 6d74e1e371d43a7b ("tools/lib/list_sort: remove redundant code for
> cond_resched handling") we need to use the newly added hunk based
> exceptions when comparing the copy we carry in tools/lib/ to the
> original file, do it by adding the hunks that we know will be the
> expected diff.
> 
> If at some point the original file is updated in other parts, then we
> should flag and check the file for update.
> 
> Cc: Adrian Hunter <adrian.hunter@...el.com>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: Ian Rogers <irogers@...gle.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Kan Liang <kan.liang@...ux.intel.com>
> Cc: Kuan-Wei Chiu <visitorckw@...il.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>

I'm seeing this but I think it's ok since we're adding a patch file as a
content.

  Applying: perf tools: Cope with differences for lib/list_sort.c copy from the kernel
  .git/rebase-apply/patch:21: space before tab in indent.
   			struct list_head *a, struct list_head *b)
  .git/rebase-apply/patch:23: space before tab in indent.
   	struct list_head *tail = head;
  .git/rebase-apply/patch:25: trailing whitespace.
   
  .git/rebase-apply/patch:26: space before tab in indent.
   	for (;;) {
  .git/rebase-apply/patch:27: space before tab in indent.
   		/* if equal, take 'a' -- important for sort stability */
  warning: squelched 6 whitespace errors
  warning: 11 lines add whitespace errors.

And build doesn't show the message for lib/list_sort.c anymore. :)

I think it's nice to update for later changes, just put the diff in a
file with the same name under check-header_ignore_hunks directory.
Good job!

Thanks,
Namhyung

> ---
>  .../check-header_ignore_hunks/lib/list_sort.c | 31 +++++++++++++++++++
>  tools/perf/check-headers.sh                   |  5 ++-
>  2 files changed, 35 insertions(+), 1 deletion(-)
>  create mode 100644 tools/perf/check-header_ignore_hunks/lib/list_sort.c
> 
> diff --git a/tools/perf/check-header_ignore_hunks/lib/list_sort.c b/tools/perf/check-header_ignore_hunks/lib/list_sort.c
> new file mode 100644
> index 0000000000000000..32d98cb34f80a987
> --- /dev/null
> +++ b/tools/perf/check-header_ignore_hunks/lib/list_sort.c
> @@ -0,0 +1,31 @@
> +@@ -1,5 +1,6 @@
> + // SPDX-License-Identifier: GPL-2.0
> + #include <linux/kernel.h>
> ++#include <linux/bug.h>
> + #include <linux/compiler.h>
> + #include <linux/export.h>
> + #include <linux/string.h>
> +@@ -52,6 +53,7 @@
> + 			struct list_head *a, struct list_head *b)
> + {
> + 	struct list_head *tail = head;
> ++	u8 count = 0;
> + 
> + 	for (;;) {
> + 		/* if equal, take 'a' -- important for sort stability */
> +@@ -77,6 +79,15 @@
> + 	/* Finish linking remainder of list b on to tail */
> + 	tail->next = b;
> + 	do {
> ++		/*
> ++		 * If the merge is highly unbalanced (e.g. the input is
> ++		 * already sorted), this loop may run many iterations.
> ++		 * Continue callbacks to the client even though no
> ++		 * element comparison is needed, so the client's cmp()
> ++		 * routine can invoke cond_resched() periodically.
> ++		 */
> ++		if (unlikely(!++count))
> ++			cmp(priv, b, b);
> + 		b->prev = tail;
> + 		tail = b;
> + 		b = b->next;
> diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
> index 55aba47e5aec9292..f1080d4096663ba1 100755
> --- a/tools/perf/check-headers.sh
> +++ b/tools/perf/check-headers.sh
> @@ -193,7 +193,6 @@ check include/uapi/linux/mman.h       '-I "^#include <\(uapi/\)*asm/mman.h>"'
>  check include/linux/build_bug.h       '-I "^#\(ifndef\|endif\)\( \/\/\)* static_assert$"'
>  check include/linux/ctype.h	      '-I "isdigit("'
>  check lib/ctype.c		      '-I "^EXPORT_SYMBOL" -I "^#include <linux/export.h>" -B'
> -check lib/list_sort.c		      '-I "^#include <linux/bug.h>"'
>  
>  # diff non-symmetric files
>  check_2 tools/perf/arch/x86/entry/syscalls/syscall_32.tbl arch/x86/entry/syscalls/syscall_32.tbl
> @@ -211,6 +210,10 @@ done
>  check_2 tools/perf/util/hashmap.h tools/lib/bpf/hashmap.h
>  check_2 tools/perf/util/hashmap.c tools/lib/bpf/hashmap.c
>  
> +# Files with larger differences
> +
> +check_ignore_some_hunks lib/list_sort.c
> +
>  cd tools/perf || exit
>  
>  if [ ${#FAILURES[@]} -gt 0 ]
> -- 
> 2.46.0
>