From: Arnaldo Carvalho de Melo <acme@redhat.com>

Hi,

	Please take a look, as per tools/include/uapi/README we carry
copies of kernel files for various reasons and check when it drifts, in
this case we need another way to accept diffs while checking, its all
spelled out in the individual patches, please ack.

- Arnaldo

Arnaldo Carvalho de Melo (2):
  tools check_headers.sh: Add check variant that excludes some hunks
  perf tools: Cope with differences for lib/list_sort.c copy from the kernel

 .../check-header_ignore_hunks/lib/list_sort.c | 31 +++++++++++++++++++
 tools/perf/check-headers.sh                   | 29 ++++++++++++++++-
 2 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/check-header_ignore_hunks/lib/list_sort.c

-- 
2.46.0

On Mon, Sep 30, 2024 at 05:21:34PM -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> Hi,
> 
> 	Please take a look, as per tools/include/uapi/README we carry
> copies of kernel files for various reasons and check when it drifts, in
> this case we need another way to accept diffs while checking, its all
> spelled out in the individual patches, please ack.
> 
> - Arnaldo
> 
> Arnaldo Carvalho de Melo (2):
>   tools check_headers.sh: Add check variant that excludes some hunks
>   perf tools: Cope with differences for lib/list_sort.c copy from the kernel
>
LGTM. For the series:

Acked-by: Kuan-Wei Chiu <visitorckw@gmail.com>

While reviewing the patches, I noticed that there was already a
difference between lib/list_sort.c and tools/lib/list_sort.c regarding
an additional #include <linux/bug.h>. This prompted me to investigate
the reason for this discrepancy. From what I can see, both files only
seem to require only three headers:

#include <linux/compiler.h> /* for likely() macro */
#include <linux/export.h> /* for EXPORT_SYMBOL() macro */
#include <linux/list_sort.h> /* for list_sort() and linux/types.h */

I'll check the git history and run build tests to confirm. If only
these headers are needed, I'll submit a cleanup patch.

Regards,
Kuan-Wei

>  .../check-header_ignore_hunks/lib/list_sort.c | 31 +++++++++++++++++++
>  tools/perf/check-headers.sh                   | 29 ++++++++++++++++-
>  2 files changed, 59 insertions(+), 1 deletion(-)
>  create mode 100644 tools/perf/check-header_ignore_hunks/lib/list_sort.c
> 
> -- 
> 2.46.0
>

On Thu, Oct 03, 2024 at 01:21:02AM +0800, Kuan-Wei Chiu wrote:
> On Mon, Sep 30, 2024 at 05:21:34PM -0300, Arnaldo Carvalho de Melo wrote:
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 	Please take a look, as per tools/include/uapi/README we carry
> > copies of kernel files for various reasons and check when it drifts, in
> > this case we need another way to accept diffs while checking, its all
> > spelled out in the individual patches, please ack.

> > Arnaldo Carvalho de Melo (2):
> >   tools check_headers.sh: Add check variant that excludes some hunks
> >   perf tools: Cope with differences for lib/list_sort.c copy from the kernel

> LGTM. For the series:
 
> Acked-by: Kuan-Wei Chiu <visitorckw@gmail.com>

Adding it to that cset before pushing to the written in stone
perf-tools/perf-tools branch.
 
> While reviewing the patches, I noticed that there was already a
> difference between lib/list_sort.c and tools/lib/list_sort.c regarding
> an additional #include <linux/bug.h>. This prompted me to investigate
> the reason for this discrepancy. From what I can see, both files only
> seem to require only three headers:

> #include <linux/compiler.h> /* for likely() macro */
> #include <linux/export.h> /* for EXPORT_SYMBOL() macro */
> #include <linux/list_sort.h> /* for list_sort() and linux/types.h */
 
> I'll check the git history and run build tests to confirm. If only
> these headers are needed, I'll submit a cleanup patch.

tools/ is a sidecar or sorts for the kernel, that tries to add value to
kernel developers while not getting in their way.

Sometimes things we discover while using more widely things that were
designed for use in the kernel source may be of help to kernel
developers, if this is one such case, great!

But IIRC that linux/bug.h discrepancy, without further checking, was
something already somehow accepted via:

+check lib/list_sort.c                '-I "^#include <linux/bug.h>"'

in the cset that introduced the copy:

  92ec3cc94c2cb60d ("tools lib: Adopt list_sort() from the kernel sources")

- Arnaldo

Hi Arnaldo,

On Wed, Oct 02, 2024 at 02:29:52PM -0300, Arnaldo Carvalho de Melo wrote:
> On Thu, Oct 03, 2024 at 01:21:02AM +0800, Kuan-Wei Chiu wrote:
> > On Mon, Sep 30, 2024 at 05:21:34PM -0300, Arnaldo Carvalho de Melo wrote:
> > > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > 	Please take a look, as per tools/include/uapi/README we carry
> > > copies of kernel files for various reasons and check when it drifts, in
> > > this case we need another way to accept diffs while checking, its all
> > > spelled out in the individual patches, please ack.
> 
> > > Arnaldo Carvalho de Melo (2):
> > >   tools check_headers.sh: Add check variant that excludes some hunks
> > >   perf tools: Cope with differences for lib/list_sort.c copy from the kernel
> 
> > LGTM. For the series:
>  
> > Acked-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> 
> Adding it to that cset before pushing to the written in stone
> perf-tools/perf-tools branch.

Oh, I thought you wanted to have it in the perf-tools-next branch.
But it's ok to go through perf-tools.  I'll drop this patchset from
tmp.perf-tools-next.

Thanks,
Namhyung

>  
> > While reviewing the patches, I noticed that there was already a
> > difference between lib/list_sort.c and tools/lib/list_sort.c regarding
> > an additional #include <linux/bug.h>. This prompted me to investigate
> > the reason for this discrepancy. From what I can see, both files only
> > seem to require only three headers:
> 
> > #include <linux/compiler.h> /* for likely() macro */
> > #include <linux/export.h> /* for EXPORT_SYMBOL() macro */
> > #include <linux/list_sort.h> /* for list_sort() and linux/types.h */
>  
> > I'll check the git history and run build tests to confirm. If only
> > these headers are needed, I'll submit a cleanup patch.
> 
> tools/ is a sidecar or sorts for the kernel, that tries to add value to
> kernel developers while not getting in their way.
> 
> Sometimes things we discover while using more widely things that were
> designed for use in the kernel source may be of help to kernel
> developers, if this is one such case, great!
> 
> But IIRC that linux/bug.h discrepancy, without further checking, was
> something already somehow accepted via:
> 
> +check lib/list_sort.c                '-I "^#include <linux/bug.h>"'
> 
> in the cset that introduced the copy:
> 
>   92ec3cc94c2cb60d ("tools lib: Adopt list_sort() from the kernel sources")
> 
> - Arnaldo