Re: [PATCH 2/2] perf tools: Cope with differences for lib/list_sort.c copy from the kernel

[Namhyung Kim]

On Wed, Oct 02, 2024 at 02:54:53PM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, Sep 30, 2024 at 06:56:16PM -0700, Namhyung Kim wrote:
> > On Mon, Sep 30, 2024 at 05:21:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > > From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> > > 
> > > 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@xxxxxxxxx>
> > > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > > Cc: Ian Rogers <irogers@xxxxxxxxxx>
> > > Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> > > Cc: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
> > > Cc: Kuan-Wei Chiu <visitorckw@xxxxxxxxx>
> > > Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> > > Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> > 
> > 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
> 
> What kind of update you thinks its nice for later changes?

I meant it's good. :)  It'd be convenient if we have some changes that
need to be handled separately like this in the future.

> 
> > file with the same name under check-header_ignore_hunks directory.
> 
> Isn't it like that already? Was the intention:
> 
> .../check-header_ignore_hunks/lib/list_sort.c | 31 +++++++++++++++++++
> 
> It is the same name as tools/lib/list_sort.c and lib/list_sort.c
> 
> Or did you mean something else?

No, I just said about possible future work and glad to have this.

> 
> > Good job!
> 
> Assuming all is right, thanks! 8-)

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
> > >