diff mbox series

Fix: rseq uapi: Adapt header includes to follow glibc header changes

Message ID 20231025214811.2066376-1-mathieu.desnoyers@efficios.com
State New
Headers
Series Fix: rseq uapi: Adapt header includes to follow glibc header changes |

Commit Message

Mathieu Desnoyers Oct. 25, 2023, 9:48 p.m. UTC 
  With "recent" glibc headers, using <sys/types.h> with __GNU_SOURCE fails
to have __u32 and others types needed by the rseq.h uapi header file.
Include ctype.h and asm/types.h to fix this. Add a __KERNEL__ #ifdef to
select the kernel vs userspace header includes.

Also, remove the now unneeded asm/byteorder.h include, since it also
causes its own build issues with "recent" glibc headers.

I'm cautiously using the term "recent" glibc here because I don't know
exactly in which glibc versions those changes happened. Steven
reproduced this issue with glibc 2.37 on Debian.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Reported-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Cc: Steven Rostedt (Google) <rostedt@goodmis.org>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/uapi/linux/rseq.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Rik van Riel Oct. 26, 2023, 1:05 a.m. UTC | #1 
On Wed, 2023-10-25 at 17:48 -0400, Mathieu Desnoyers wrote:
> With "recent" glibc headers, using <sys/types.h> with __GNU_SOURCE
> fails
> to have __u32 and others types needed by the rseq.h uapi header file.
> Include ctype.h and asm/types.h to fix this. Add a __KERNEL__ #ifdef
> to
> select the kernel vs userspace header includes.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Reported-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> Cc: Steven Rostedt (Google) <rostedt@goodmis.org>
> Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
> 
Acked-by: Rik van Riel <riel@surriel.com>
kernel test robot Oct. 27, 2023, 7:53 a.m. UTC | #2 
Hi Mathieu,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.6-rc7 next-20231026]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mathieu-Desnoyers/Fix-rseq-uapi-Adapt-header-includes-to-follow-glibc-header-changes/20231026-054939
base:   linus/master
patch link:    https://lore.kernel.org/r/20231025214811.2066376-1-mathieu.desnoyers%40efficios.com
patch subject: [PATCH] Fix: rseq uapi: Adapt header includes to follow glibc header changes
config: i386-randconfig-001-20231026 (https://download.01.org/0day-ci/archive/20231027/202310271556.LunB8KLv-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231027/202310271556.LunB8KLv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310271556.LunB8KLv-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> usr/include/linux/rseq.h:14: include of <linux/types.h> is preferred over <asm/types.h>
>> usr/include/linux/rseq.h:47: found __[us]{8,16,32,64} type without #include <linux/types.h>
Mathieu Desnoyers Oct. 27, 2023, 1:37 p.m. UTC | #3 
On 2023-10-27 03:53, kernel test robot wrote:
> Hi Mathieu,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on linus/master]
> [also build test WARNING on v6.6-rc7 next-20231026]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]

The test robot complains about using <asm/types.h> in uapi headers for 
!__KERNEL__ case.

Steven, was there something wrong with including linux/types.h in uapi 
headers ?

Thanks,

Mathieu

> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Mathieu-Desnoyers/Fix-rseq-uapi-Adapt-header-includes-to-follow-glibc-header-changes/20231026-054939
> base:   linus/master
> patch link:    https://lore.kernel.org/r/20231025214811.2066376-1-mathieu.desnoyers%40efficios.com
> patch subject: [PATCH] Fix: rseq uapi: Adapt header includes to follow glibc header changes
> config: i386-randconfig-001-20231026 (https://download.01.org/0day-ci/archive/20231027/202310271556.LunB8KLv-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231027/202310271556.LunB8KLv-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202310271556.LunB8KLv-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>>> usr/include/linux/rseq.h:14: include of <linux/types.h> is preferred over <asm/types.h>
>>> usr/include/linux/rseq.h:47: found __[us]{8,16,32,64} type without #include <linux/types.h>
>
Steven Rostedt Oct. 27, 2023, 2:06 p.m. UTC | #4 
On Fri, 27 Oct 2023 09:37:26 -0400
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

> On 2023-10-27 03:53, kernel test robot wrote:
> > Hi Mathieu,
> > 
> > kernel test robot noticed the following build warnings:
> > 
> > [auto build test WARNING on linus/master]
> > [also build test WARNING on v6.6-rc7 next-20231026]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]  
> 
> The test robot complains about using <asm/types.h> in uapi headers for 
> !__KERNEL__ case.
> 
> Steven, was there something wrong with including linux/types.h in uapi 
> headers ?
> 

Actually, linux/types.h includes asm/types.h so I don't think that was the
issue. I think the issue was mostly with:

 #include <asm/byteorder.h>

Replacing linux/types.h with asm/types.h worked, but may have been
unnecessary.

-- Steve
Mathieu Desnoyers Nov. 1, 2023, 8:10 p.m. UTC | #5 
On 2023-10-27 10:06, Steven Rostedt wrote:
> On Fri, 27 Oct 2023 09:37:26 -0400
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> 
>> On 2023-10-27 03:53, kernel test robot wrote:
>>> Hi Mathieu,
>>>
>>> kernel test robot noticed the following build warnings:
>>>
>>> [auto build test WARNING on linus/master]
>>> [also build test WARNING on v6.6-rc7 next-20231026]
>>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>>> And when submitting patch, we suggest to use '--base' as documented in
>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>
>> The test robot complains about using <asm/types.h> in uapi headers for
>> !__KERNEL__ case.
>>
>> Steven, was there something wrong with including linux/types.h in uapi
>> headers ?
>>
> 
> Actually, linux/types.h includes asm/types.h so I don't think that was the
> issue. I think the issue was mostly with:
> 
>   #include <asm/byteorder.h>
> 
> Replacing linux/types.h with asm/types.h worked, but may have been
> unnecessary.

Hi Steven,

So what is the minimal change required to make things work on your 
setup? I just tested with a Debian "testing" chroot (with libc 2.37-12) 
and I cannot reproduce your issue.

Should I just submit a patch that removes "#include <asm/byteorder.h>" ? 
I am really unsure which environments are affected though.

Thanks,

Mathieu
Steven Rostedt Nov. 1, 2023, 8:44 p.m. UTC | #6 
On Wed, 1 Nov 2023 16:10:04 -0400
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

> So what is the minimal change required to make things work on your 
> setup? I just tested with a Debian "testing" chroot (with libc 2.37-12) 
> and I cannot reproduce your issue.
> 
> Should I just submit a patch that removes "#include <asm/byteorder.h>" ? 
> I am really unsure which environments are affected though.
> 

I guess you can drop it :-p

When I tried to reproduce it with hand writing 'gcc', I couldn't. But when
I did:

 $ make foo

It gave me the error. I was confused for a bit. Then I looked at what my
Makefile was doing and what I was doing. The only difference was that the
make included:

   -I.

Removing that from the Makefile worked!

My Makefile added to the CFLAGS "-I." and I forgot that this directory has
a "linux/"  directory in it that I used years ago to test kernel functions.
The git history shows it was last touched in 2016 (when I was still at Red Hat)

Removing -I. now makes everything work.

I have no idea why it suddenly stopped working just a few months ago. Maybe
something was moved out of the gcc headers so my local headers no longer
see it. That is, perhaps the glibc headers moved something out and added a
#include to it, where my local headers did not have that change. I don't
know and I don't care.

Well, at least now I know why I was getting errors on my build, but
couldn't find anything on the internet showing why others were not!

Sorry for the noise. :-/

-- Steve
Mathieu Desnoyers Nov. 1, 2023, 8:51 p.m. UTC | #7 
On 2023-11-01 16:44, Steven Rostedt wrote:
> On Wed, 1 Nov 2023 16:10:04 -0400
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> 
>> So what is the minimal change required to make things work on your
>> setup? I just tested with a Debian "testing" chroot (with libc 2.37-12)
>> and I cannot reproduce your issue.
>>
>> Should I just submit a patch that removes "#include <asm/byteorder.h>" ?
>> I am really unsure which environments are affected though.
>>
> 
> I guess you can drop it :-p

OK, let's drop this patch then. I may respin the removal of asm/byteorder.h
include as a cleanup in the future, but there is really no hurry.

Thanks,

Mathieu

> 
> When I tried to reproduce it with hand writing 'gcc', I couldn't. But when
> I did:
> 
>   $ make foo
> 
> It gave me the error. I was confused for a bit. Then I looked at what my
> Makefile was doing and what I was doing. The only difference was that the
> make included:
> 
>     -I.
> 
> Removing that from the Makefile worked!
> 
> My Makefile added to the CFLAGS "-I." and I forgot that this directory has
> a "linux/"  directory in it that I used years ago to test kernel functions.
> The git history shows it was last touched in 2016 (when I was still at Red Hat)
> 
> Removing -I. now makes everything work.
> 
> I have no idea why it suddenly stopped working just a few months ago. Maybe
> something was moved out of the gcc headers so my local headers no longer
> see it. That is, perhaps the glibc headers moved something out and added a
> #include to it, where my local headers did not have that change. I don't
> know and I don't care.
> 
> Well, at least now I know why I was getting errors on my build, but
> couldn't find anything on the internet showing why others were not!
> 
> Sorry for the noise. :-/
> 
> -- Steve
>
diff mbox series

Patch

diff --git a/include/uapi/linux/rseq.h b/include/uapi/linux/rseq.h
index c233aae5eac9..0f9cd8211ff0 100644
--- a/include/uapi/linux/rseq.h
+++ b/include/uapi/linux/rseq.h
@@ -10,8 +10,12 @@ 
  * Copyright (c) 2015-2018 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
  */
 
-#include <linux/types.h>
-#include <asm/byteorder.h>
+#ifdef __KERNEL__
+# include <linux/types.h>
+#else
+# include <ctype.h>
+# include <asm/types.h>
+#endif
 
 enum rseq_cpu_id_state {
 	RSEQ_CPU_ID_UNINITIALIZED		= -1,