| Message ID | cover.1693575451.git.maciej.wieczor-retman@intel.com |
|---|---|
| Headers | show |
| Series | selftests/resctrl: Bug fix and optimization | expand |
Hi Maciej, On 9/11/2023 11:32 PM, Maciej Wieczór-Retman wrote: > On 2023-09-11 at 09:59:06 -0700, Reinette Chatre wrote: >> Hi Maciej, >> When I build the tests with this applied I encounter the following: >> >> resctrlfs.c: In function ‘write_schemata’: >> resctrlfs.c:475:14: warning: implicit declaration of function ‘open’; did you mean ‘popen’? [-Wimplicit-function-declaration] >> 475 | fd = open(controlgroup, O_WRONLY); >> | ^~~~ >> | popen >> resctrlfs.c:475:33: error: ‘O_WRONLY’ undeclared (first use in this function) >> 475 | fd = open(controlgroup, O_WRONLY); >> | ^~~~~~~~ >> resctrlfs.c:475:33: note: each undeclared identifier is reported only once for each function it appears in > > Hmm, that's odd. How do you build the tests? I applied this series on top of kselftest repo's "next" branch. I use a separate build directory and first ran "make headers". After that, $ make O=<build dir> -C tools/testing/selftests/resctrl > I use "make -C tools/testing/selftests/resctrl" while in the root kernel > source directory. I tried to get the same error you experienced by > compiling some dummy test program with "open" and "O_WRONLY". From the > experiment I found that the "resctrl.h" header provides the declarations > that are causing your errors.
On 2023-09-12 at 09:00:28 -0700, Reinette Chatre wrote: >Hi Maciej, > >On 9/11/2023 11:32 PM, Maciej Wieczór-Retman wrote: >> On 2023-09-11 at 09:59:06 -0700, Reinette Chatre wrote: >>> Hi Maciej, >>> When I build the tests with this applied I encounter the following: >>> >>> resctrlfs.c: In function ‘write_schemata’: >>> resctrlfs.c:475:14: warning: implicit declaration of function ‘open’; did you mean ‘popen’? [-Wimplicit-function-declaration] >>> 475 | fd = open(controlgroup, O_WRONLY); >>> | ^~~~ >>> | popen >>> resctrlfs.c:475:33: error: ‘O_WRONLY’ undeclared (first use in this function) >>> 475 | fd = open(controlgroup, O_WRONLY); >>> | ^~~~~~~~ >>> resctrlfs.c:475:33: note: each undeclared identifier is reported only once for each function it appears in >> >> Hmm, that's odd. How do you build the tests? > >I applied this series on top of kselftest repo's "next" branch. > >I use a separate build directory and first ran "make headers". After that, >$ make O=<build dir> -C tools/testing/selftests/resctrl I do the same, just without the build directory, but that shouldn't matter here I guess. >> I use "make -C tools/testing/selftests/resctrl" while in the root kernel >> source directory. I tried to get the same error you experienced by >> compiling some dummy test program with "open" and "O_WRONLY". From the >> experiment I found that the "resctrl.h" header provides the declarations >> that are causing your errors. > >From what I can tell resctrl.h does not include fcntl.h that provides >what is needed. I found out you can run "gcc -M <file>" and it will recursively tell you what headers are including other headers. Using this I found that "resctrl.h" includes <sys/mount.h> which in turn includes <fcntl.h> out of /usr/include/sys directory. Is that also the case on your system?
Hi Maciej, On 9/12/2023 10:59 PM, Maciej Wieczór-Retman wrote: > On 2023-09-12 at 09:00:28 -0700, Reinette Chatre wrote: >> Hi Maciej, >> >> On 9/11/2023 11:32 PM, Maciej Wieczór-Retman wrote: >>> On 2023-09-11 at 09:59:06 -0700, Reinette Chatre wrote: >>>> Hi Maciej, >>>> When I build the tests with this applied I encounter the following: >>>> >>>> resctrlfs.c: In function ‘write_schemata’: >>>> resctrlfs.c:475:14: warning: implicit declaration of function ‘open’; did you mean ‘popen’? [-Wimplicit-function-declaration] >>>> 475 | fd = open(controlgroup, O_WRONLY); >>>> | ^~~~ >>>> | popen >>>> resctrlfs.c:475:33: error: ‘O_WRONLY’ undeclared (first use in this function) >>>> 475 | fd = open(controlgroup, O_WRONLY); >>>> | ^~~~~~~~ >>>> resctrlfs.c:475:33: note: each undeclared identifier is reported only once for each function it appears in >>> >>> Hmm, that's odd. How do you build the tests? >> >> I applied this series on top of kselftest repo's "next" branch. >> >> I use a separate build directory and first ran "make headers". After that, >> $ make O=<build dir> -C tools/testing/selftests/resctrl > > I do the same, just without the build directory, but that shouldn't > matter here I guess. > >>> I use "make -C tools/testing/selftests/resctrl" while in the root kernel >>> source directory. I tried to get the same error you experienced by >>> compiling some dummy test program with "open" and "O_WRONLY". From the >>> experiment I found that the "resctrl.h" header provides the declarations >>> that are causing your errors. >> >>From what I can tell resctrl.h does not include fcntl.h that provides >> what is needed. > > I found out you can run "gcc -M <file>" and it will recursively tell you > what headers are including other headers. > > Using this I found that "resctrl.h" includes <sys/mount.h> which in turn > includes <fcntl.h> out of /usr/include/sys directory. Is that also the > case on your system? > No. The test system I used is running glibc 2.35 and it seems that including fcntl.h was added to sys/mount.h in 2.36. See glibc commit 78a408ee7ba0 ("linux: Add open_tree") Generally we should avoid indirect inclusions and here I think certainly so since it cannot be guaranteed that fcntl.h would be available via sys/mount.h. Reinette
Hi, On 2023-09-13 at 11:49:19 -0700, Reinette Chatre wrote: >Hi Maciej, > >On 9/12/2023 10:59 PM, Maciej Wieczór-Retman wrote: >> On 2023-09-12 at 09:00:28 -0700, Reinette Chatre wrote: >>> Hi Maciej, >>> >>> On 9/11/2023 11:32 PM, Maciej Wieczór-Retman wrote: >>>> On 2023-09-11 at 09:59:06 -0700, Reinette Chatre wrote: >>>>> Hi Maciej, >>>>> When I build the tests with this applied I encounter the following: >>>>> >>>>> resctrlfs.c: In function ‘write_schemata’: >>>>> resctrlfs.c:475:14: warning: implicit declaration of function ‘open’; did you mean ‘popen’? [-Wimplicit-function-declaration] >>>>> 475 | fd = open(controlgroup, O_WRONLY); >>>>> | ^~~~ >>>>> | popen >>>>> resctrlfs.c:475:33: error: ‘O_WRONLY’ undeclared (first use in this function) >>>>> 475 | fd = open(controlgroup, O_WRONLY); >>>>> | ^~~~~~~~ >>>>> resctrlfs.c:475:33: note: each undeclared identifier is reported only once for each function it appears in >>>> >>>> Hmm, that's odd. How do you build the tests? >>> >>> I applied this series on top of kselftest repo's "next" branch. >>> >>> I use a separate build directory and first ran "make headers". After that, >>> $ make O=<build dir> -C tools/testing/selftests/resctrl >> >> I do the same, just without the build directory, but that shouldn't >> matter here I guess. >> >>>> I use "make -C tools/testing/selftests/resctrl" while in the root kernel >>>> source directory. I tried to get the same error you experienced by >>>> compiling some dummy test program with "open" and "O_WRONLY". From the >>>> experiment I found that the "resctrl.h" header provides the declarations >>>> that are causing your errors. >>> >>>From what I can tell resctrl.h does not include fcntl.h that provides >>> what is needed. >> >> I found out you can run "gcc -M <file>" and it will recursively tell you >> what headers are including other headers. >> >> Using this I found that "resctrl.h" includes <sys/mount.h> which in turn >> includes <fcntl.h> out of /usr/include/sys directory. Is that also the >> case on your system? >> > >No. The test system I used is running glibc 2.35 and it seems that including >fcntl.h was added to sys/mount.h in 2.36. See glibc commit >78a408ee7ba0 ("linux: Add open_tree") > >Generally we should avoid indirect inclusions and here I think certainly so >since it cannot be guaranteed that fcntl.h would be available via >sys/mount.h. Okay, would including the fcntl.h header to resctrl.h be okay in this case? Or is there some other more sophisticated way of doing that (some include guard or checking glibc version for example)?
Hi Maciej, On 9/13/2023 11:01 PM, Maciej Wieczór-Retman wrote: > On 2023-09-13 at 11:49:19 -0700, Reinette Chatre wrote: >> On 9/12/2023 10:59 PM, Maciej Wieczór-Retman wrote: >>> On 2023-09-12 at 09:00:28 -0700, Reinette Chatre wrote: >>>> On 9/11/2023 11:32 PM, Maciej Wieczór-Retman wrote: >>>>> On 2023-09-11 at 09:59:06 -0700, Reinette Chatre wrote: >>>>>> Hi Maciej, >>>>>> When I build the tests with this applied I encounter the following: >>>>>> >>>>>> resctrlfs.c: In function ‘write_schemata’: >>>>>> resctrlfs.c:475:14: warning: implicit declaration of function ‘open’; did you mean ‘popen’? [-Wimplicit-function-declaration] >>>>>> 475 | fd = open(controlgroup, O_WRONLY); >>>>>> | ^~~~ >>>>>> | popen >>>>>> resctrlfs.c:475:33: error: ‘O_WRONLY’ undeclared (first use in this function) >>>>>> 475 | fd = open(controlgroup, O_WRONLY); >>>>>> | ^~~~~~~~ >>>>>> resctrlfs.c:475:33: note: each undeclared identifier is reported only once for each function it appears in >>>>> >>>>> Hmm, that's odd. How do you build the tests? >>>> >>>> I applied this series on top of kselftest repo's "next" branch. >>>> >>>> I use a separate build directory and first ran "make headers". After that, >>>> $ make O=<build dir> -C tools/testing/selftests/resctrl >>> >>> I do the same, just without the build directory, but that shouldn't >>> matter here I guess. >>> >>>>> I use "make -C tools/testing/selftests/resctrl" while in the root kernel >>>>> source directory. I tried to get the same error you experienced by >>>>> compiling some dummy test program with "open" and "O_WRONLY". From the >>>>> experiment I found that the "resctrl.h" header provides the declarations >>>>> that are causing your errors. >>>> >>> >From what I can tell resctrl.h does not include fcntl.h that provides >>>> what is needed. >>> >>> I found out you can run "gcc -M <file>" and it will recursively tell you >>> what headers are including other headers. >>> >>> Using this I found that "resctrl.h" includes <sys/mount.h> which in turn >>> includes <fcntl.h> out of /usr/include/sys directory. Is that also the >>> case on your system? >>> >> >> No. The test system I used is running glibc 2.35 and it seems that including >> fcntl.h was added to sys/mount.h in 2.36. See glibc commit >> 78a408ee7ba0 ("linux: Add open_tree") >> >> Generally we should avoid indirect inclusions and here I think certainly so >> since it cannot be guaranteed that fcntl.h would be available via >> sys/mount.h. > > Okay, would including the fcntl.h header to resctrl.h be okay in this > case? Or is there some other more sophisticated way of doing that (some > include guard or checking glibc version for example)? Ideally fcntl.h would be included in the file it is used. Doing so you may encounter the same problems as Ilpo in [1]. If that is the case and that fix works for you then you may want to have this series depend on Ilpo's work. Reinette [1] https://lore.kernel.org/lkml/dfc53e-3f92-82e4-6af-d1a28e8c199a@linux.intel.com/
On 2023-09-14 at 08:14:25 -0700, Reinette Chatre wrote: >Hi Maciej, > >On 9/13/2023 11:01 PM, Maciej Wieczór-Retman wrote: >> On 2023-09-13 at 11:49:19 -0700, Reinette Chatre wrote: >>> On 9/12/2023 10:59 PM, Maciej Wieczór-Retman wrote: >>>> On 2023-09-12 at 09:00:28 -0700, Reinette Chatre wrote: >>>>> On 9/11/2023 11:32 PM, Maciej Wieczór-Retman wrote: >>>>>> On 2023-09-11 at 09:59:06 -0700, Reinette Chatre wrote: >>>>>>> Hi Maciej, >>>>>>> When I build the tests with this applied I encounter the following: >>>>>>> >>>>>>> resctrlfs.c: In function ‘write_schemata’: >>>>>>> resctrlfs.c:475:14: warning: implicit declaration of function ‘open’; did you mean ‘popen’? [-Wimplicit-function-declaration] >>>>>>> 475 | fd = open(controlgroup, O_WRONLY); >>>>>>> | ^~~~ >>>>>>> | popen >>>>>>> resctrlfs.c:475:33: error: ‘O_WRONLY’ undeclared (first use in this function) >>>>>>> 475 | fd = open(controlgroup, O_WRONLY); >>>>>>> | ^~~~~~~~ >>>>>>> resctrlfs.c:475:33: note: each undeclared identifier is reported only once for each function it appears in >>>>>> >>>>>> Hmm, that's odd. How do you build the tests? >>>>> >>>>> I applied this series on top of kselftest repo's "next" branch. >>>>> >>>>> I use a separate build directory and first ran "make headers". After that, >>>>> $ make O=<build dir> -C tools/testing/selftests/resctrl >>>> >>>> I do the same, just without the build directory, but that shouldn't >>>> matter here I guess. >>>> >>>>>> I use "make -C tools/testing/selftests/resctrl" while in the root kernel >>>>>> source directory. I tried to get the same error you experienced by >>>>>> compiling some dummy test program with "open" and "O_WRONLY". From the >>>>>> experiment I found that the "resctrl.h" header provides the declarations >>>>>> that are causing your errors. >>>>> >>>> >From what I can tell resctrl.h does not include fcntl.h that provides >>>>> what is needed. >>>> >>>> I found out you can run "gcc -M <file>" and it will recursively tell you >>>> what headers are including other headers. >>>> >>>> Using this I found that "resctrl.h" includes <sys/mount.h> which in turn >>>> includes <fcntl.h> out of /usr/include/sys directory. Is that also the >>>> case on your system? >>>> >>> >>> No. The test system I used is running glibc 2.35 and it seems that including >>> fcntl.h was added to sys/mount.h in 2.36. See glibc commit >>> 78a408ee7ba0 ("linux: Add open_tree") >>> >>> Generally we should avoid indirect inclusions and here I think certainly so >>> since it cannot be guaranteed that fcntl.h would be available via >>> sys/mount.h. >> >> Okay, would including the fcntl.h header to resctrl.h be okay in this >> case? Or is there some other more sophisticated way of doing that (some >> include guard or checking glibc version for example)? > >Ideally fcntl.h would be included in the file it is used. Doing so you may >encounter the same problems as Ilpo in [1]. If that is the case and that fix works >for you then you may want to have this series depend on Ilpo's work. Thanks a lot for finding this, and yes, I get the same errors by adding the header. I'll send the next version of this series with the added header rebased on top of Ilpo's series you mentioned. >Reinette > >[1] https://lore.kernel.org/lkml/dfc53e-3f92-82e4-6af-d1a28e8c199a@linux.intel.com/ >