Re: [PATCH] vfio_pci: Add local source directory as include
Alex Williamson writes: ... > > Numbering options for clarity: > > 1) >> ccflags-y += -I$(src) >> would add the header search path for all files in drivers/vfio/pci/ >> whereas only the drivers/vfio/pci/vfio_pci_nvlink2.c needs it. >> > > 2) >> CFLAGS_vfio_pci_nvlink2.o += -I$(src) >> is a bit better. >> However, it is not obvious why this extra header search path is needed >> until you find vfio_pci_nvlink2.c including trace.h >> > > 3) >> #define TRACE_INCLUDE_PATH ../../drivers/vfio/pci >> clarifies the intention because the related code is all placed in trace.h Good summary. >> From the comment in include/trace/define_trace.h >> TRACE_INCLUDE_PATH is relative to include/trace/define_trace.h > > In my scan of the tree, the most common solution seems to be 2) as this > is essentially recommended in the sample file. 3) is well represented, > with much fewer examples of 1), though it might depend how liberally > we grep out or examine the use cases. It seems to me that 1 and 2 is overwhelmingly used: $ git grep -F "#define TRACE_INCLUDE_PATH" | wc -l 133 That counts all definitions of TRACE_INCLUDE_PATH. $ git grep -F "#define TRACE_INCLUDE_PATH ." | wc -l 122 That's all files using '.', so only 11 locations use the relative path method (3). Which is unsurprising given that the sample uses '.'. And people often look at existing code for an example, so they're also going to tend to use '.'. I agree with Masahiro that adding include paths to the Makefile for this is a bit gross, and method 3 is much more preferable. Fixing all the existing code to use method 3 would be a good beginner project :) cheers
Re: [PATCH] vfio_pci: Add local source directory as include
On Tue, Jan 8, 2019 at 11:59 AM Alexey Kardashevskiy wrote: > > > > On 08/01/2019 13:38, Masahiro Yamada wrote: > > On Tue, Jan 8, 2019 at 11:22 AM Alexey Kardashevskiy wrote: > >> > >> > >> > >> On 08/01/2019 11:24, Alex Williamson wrote: > >>> On Tue, 8 Jan 2019 10:52:43 +1100 > >>> Alexey Kardashevskiy wrote: > >>> > On 08/01/2019 07:13, Alex Williamson wrote: > > On Mon, 7 Jan 2019 20:39:19 +0900 > > Masahiro Yamada wrote: > > > >> On Mon, Jan 7, 2019 at 8:09 PM Cornelia Huck wrote: > >>> > >>> On Mon, 7 Jan 2019 19:12:10 +0900 > >>> Masahiro Yamada wrote: > >>> > On Mon, Jan 7, 2019 at 6:18 PM Michael Ellerman > wrote: > > > > Laura Abbott writes: > >> Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 > >> SXM2] > >> subdriver") introduced a trace.h file in the local directory but > >> missed adding the local include path, resulting in compilation > >> failures with tracepoints: > >> > >> In file included from drivers/vfio/pci/trace.h:102, > >> from drivers/vfio/pci/vfio_pci_nvlink2.c:29: > >> ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No > >> such file or directory > >> #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) > >> > >> Fix this by adjusting the include path. > >> > >> Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 > >> SXM2] subdriver") > >> Signed-off-by: Laura Abbott > >>> > >>> (...) > >>> > > Alex I assume you'll merge this fix via the vfio tree? > > > > cheers > > > >> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile > >> index 9662c063a6b1..08d4676a8495 100644 > >> --- a/drivers/vfio/pci/Makefile > >> +++ b/drivers/vfio/pci/Makefile > >> @@ -1,3 +1,4 @@ > >> +ccflags-y += -I$(src) > >> > >> vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o > >> vfio_pci_config.o > >> vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o > >> -- > >> 2.20.1 > > > Hi. > > If I correctly understand the usage of TRACE_INCLUDE_PATH, > the correct fix should be like follows: > > > diff --git a/drivers/vfio/pci/trace.h b/drivers/vfio/pci/trace.h > index 228ccdb..4d13e51 100644 > --- a/drivers/vfio/pci/trace.h > +++ b/drivers/vfio/pci/trace.h > @@ -94,7 +94,7 @@ TRACE_EVENT(vfio_pci_npu2_mmap, > #endif /* _TRACE_VFIO_PCI_H */ > > #undef TRACE_INCLUDE_PATH > -#define TRACE_INCLUDE_PATH . > +#define TRACE_INCLUDE_PATH ../../drivers/vfio/pci > #undef TRACE_INCLUDE_FILE > #define TRACE_INCLUDE_FILE trace > >>> > >>> Going from the comments in samples/trace_events/trace-events-sample.h, > >>> I think both approaches are possible, and I see both used in various > >>> places. > >>> > >>> Personally, I'd prefer Laura's patch, as it doesn't involve hardcoding > >>> a path. > > > > Numbering options for clarity: > > > > 1) > >> ccflags-y += -I$(src) > >> would add the header search path for all files in drivers/vfio/pci/ > >> whereas only the drivers/vfio/pci/vfio_pci_nvlink2.c needs it. > >> > > > > 2) > >> CFLAGS_vfio_pci_nvlink2.o += -I$(src) > >> is a bit better. > >> However, it is not obvious why this extra header search path is needed > >> until you find vfio_pci_nvlink2.c including trace.h > >> > > > > 3) > >> #define TRACE_INCLUDE_PATH ../../drivers/vfio/pci > >> clarifies the intention because the related code is all placed in > >> trace.h > >> > >> > >> > >> From the comment in include/trace/define_trace.h > >> TRACE_INCLUDE_PATH is relative to include/trace/define_trace.h > > > > In my scan of the tree, the most common solution seems to be 2) as this > > is essentially recommended in the sample file. 3) is well represented, > > with much fewer examples of 1), though it might depend how liberally > > we grep out or examine the use cases. Choice 1) also seems to be the > > most shotgun approach, adding to the search path for all files. > > > The shotgun approach is always used when compiling out of tree which is > what many people do anyway without realizing that there are additional > -I. Why do not we make in-tree behave the same way? Thanks, > >>> > >>> Are you suggesting bandaging this individual leaf directory, as in > >>> choice 1), because it's no worse than what happens for an out-of-tree > >>> build anyway, > >> > >> > >> I suggest making in-tree and out-of-tree behavior equal -
Re: [PATCH] vfio_pci: Add local source directory as include
On 08/01/2019 13:38, Masahiro Yamada wrote: > On Tue, Jan 8, 2019 at 11:22 AM Alexey Kardashevskiy wrote: >> >> >> >> On 08/01/2019 11:24, Alex Williamson wrote: >>> On Tue, 8 Jan 2019 10:52:43 +1100 >>> Alexey Kardashevskiy wrote: >>> On 08/01/2019 07:13, Alex Williamson wrote: > On Mon, 7 Jan 2019 20:39:19 +0900 > Masahiro Yamada wrote: > >> On Mon, Jan 7, 2019 at 8:09 PM Cornelia Huck wrote: >>> >>> On Mon, 7 Jan 2019 19:12:10 +0900 >>> Masahiro Yamada wrote: >>> On Mon, Jan 7, 2019 at 6:18 PM Michael Ellerman wrote: > > Laura Abbott writes: >> Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] >> subdriver") introduced a trace.h file in the local directory but >> missed adding the local include path, resulting in compilation >> failures with tracepoints: >> >> In file included from drivers/vfio/pci/trace.h:102, >> from drivers/vfio/pci/vfio_pci_nvlink2.c:29: >> ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No >> such file or directory >> #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) >> >> Fix this by adjusting the include path. >> >> Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] >> subdriver") >> Signed-off-by: Laura Abbott >>> >>> (...) >>> > Alex I assume you'll merge this fix via the vfio tree? > > cheers > >> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile >> index 9662c063a6b1..08d4676a8495 100644 >> --- a/drivers/vfio/pci/Makefile >> +++ b/drivers/vfio/pci/Makefile >> @@ -1,3 +1,4 @@ >> +ccflags-y += -I$(src) >> >> vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o >> vfio_pci_config.o >> vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o >> -- >> 2.20.1 Hi. If I correctly understand the usage of TRACE_INCLUDE_PATH, the correct fix should be like follows: diff --git a/drivers/vfio/pci/trace.h b/drivers/vfio/pci/trace.h index 228ccdb..4d13e51 100644 --- a/drivers/vfio/pci/trace.h +++ b/drivers/vfio/pci/trace.h @@ -94,7 +94,7 @@ TRACE_EVENT(vfio_pci_npu2_mmap, #endif /* _TRACE_VFIO_PCI_H */ #undef TRACE_INCLUDE_PATH -#define TRACE_INCLUDE_PATH . +#define TRACE_INCLUDE_PATH ../../drivers/vfio/pci #undef TRACE_INCLUDE_FILE #define TRACE_INCLUDE_FILE trace >>> >>> Going from the comments in samples/trace_events/trace-events-sample.h, >>> I think both approaches are possible, and I see both used in various >>> places. >>> >>> Personally, I'd prefer Laura's patch, as it doesn't involve hardcoding >>> a path. > > Numbering options for clarity: > > 1) >> ccflags-y += -I$(src) >> would add the header search path for all files in drivers/vfio/pci/ >> whereas only the drivers/vfio/pci/vfio_pci_nvlink2.c needs it. >> > > 2) >> CFLAGS_vfio_pci_nvlink2.o += -I$(src) >> is a bit better. >> However, it is not obvious why this extra header search path is needed >> until you find vfio_pci_nvlink2.c including trace.h >> > > 3) >> #define TRACE_INCLUDE_PATH ../../drivers/vfio/pci >> clarifies the intention because the related code is all placed in trace.h >> >> >> >> From the comment in include/trace/define_trace.h >> TRACE_INCLUDE_PATH is relative to include/trace/define_trace.h > > In my scan of the tree, the most common solution seems to be 2) as this > is essentially recommended in the sample file. 3) is well represented, > with much fewer examples of 1), though it might depend how liberally > we grep out or examine the use cases. Choice 1) also seems to be the > most shotgun approach, adding to the search path for all files. The shotgun approach is always used when compiling out of tree which is what many people do anyway without realizing that there are additional -I. Why do not we make in-tree behave the same way? Thanks, >>> >>> Are you suggesting bandaging this individual leaf directory, as in >>> choice 1), because it's no worse than what happens for an out-of-tree >>> build anyway, >> >> >> I suggest making in-tree and out-of-tree behavior equal - both either >> fail or compile. Just makes sense to me. >> >> Since out-of-tree adds extra -I, then there should have been a reason >> for that at the time (before 2005 though). Unfortunately git blame does >> not say why it was done this way for out-of-tree so imho the safest >> thing is to add -I for in-tree as well. Or ditch extra -I
Re: [PATCH] vfio_pci: Add local source directory as include
On Tue, Jan 8, 2019 at 11:22 AM Alexey Kardashevskiy wrote: > > > > On 08/01/2019 11:24, Alex Williamson wrote: > > On Tue, 8 Jan 2019 10:52:43 +1100 > > Alexey Kardashevskiy wrote: > > > >> On 08/01/2019 07:13, Alex Williamson wrote: > >>> On Mon, 7 Jan 2019 20:39:19 +0900 > >>> Masahiro Yamada wrote: > >>> > On Mon, Jan 7, 2019 at 8:09 PM Cornelia Huck wrote: > > > > On Mon, 7 Jan 2019 19:12:10 +0900 > > Masahiro Yamada wrote: > > > >> On Mon, Jan 7, 2019 at 6:18 PM Michael Ellerman > >> wrote: > >>> > >>> Laura Abbott writes: > Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > subdriver") introduced a trace.h file in the local directory but > missed adding the local include path, resulting in compilation > failures with tracepoints: > > In file included from drivers/vfio/pci/trace.h:102, > from drivers/vfio/pci/vfio_pci_nvlink2.c:29: > ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No > such file or directory > #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) > > Fix this by adjusting the include path. > > Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > subdriver") > Signed-off-by: Laura Abbott > > > > (...) > > > >>> Alex I assume you'll merge this fix via the vfio tree? > >>> > >>> cheers > >>> > diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile > index 9662c063a6b1..08d4676a8495 100644 > --- a/drivers/vfio/pci/Makefile > +++ b/drivers/vfio/pci/Makefile > @@ -1,3 +1,4 @@ > +ccflags-y += -I$(src) > > vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o > vfio_pci_config.o > vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o > -- > 2.20.1 > >> > >> > >> Hi. > >> > >> If I correctly understand the usage of TRACE_INCLUDE_PATH, > >> the correct fix should be like follows: > >> > >> > >> diff --git a/drivers/vfio/pci/trace.h b/drivers/vfio/pci/trace.h > >> index 228ccdb..4d13e51 100644 > >> --- a/drivers/vfio/pci/trace.h > >> +++ b/drivers/vfio/pci/trace.h > >> @@ -94,7 +94,7 @@ TRACE_EVENT(vfio_pci_npu2_mmap, > >> #endif /* _TRACE_VFIO_PCI_H */ > >> > >> #undef TRACE_INCLUDE_PATH > >> -#define TRACE_INCLUDE_PATH . > >> +#define TRACE_INCLUDE_PATH ../../drivers/vfio/pci > >> #undef TRACE_INCLUDE_FILE > >> #define TRACE_INCLUDE_FILE trace > > > > Going from the comments in samples/trace_events/trace-events-sample.h, > > I think both approaches are possible, and I see both used in various > > places. > > > > Personally, I'd prefer Laura's patch, as it doesn't involve hardcoding > > a path. > >>> > >>> Numbering options for clarity: > >>> > >>> 1) > ccflags-y += -I$(src) > would add the header search path for all files in drivers/vfio/pci/ > whereas only the drivers/vfio/pci/vfio_pci_nvlink2.c needs it. > > >>> > >>> 2) > CFLAGS_vfio_pci_nvlink2.o += -I$(src) > is a bit better. > However, it is not obvious why this extra header search path is needed > until you find vfio_pci_nvlink2.c including trace.h > > >>> > >>> 3) > #define TRACE_INCLUDE_PATH ../../drivers/vfio/pci > clarifies the intention because the related code is all placed in trace.h > > > > From the comment in include/trace/define_trace.h > TRACE_INCLUDE_PATH is relative to include/trace/define_trace.h > >>> > >>> In my scan of the tree, the most common solution seems to be 2) as this > >>> is essentially recommended in the sample file. 3) is well represented, > >>> with much fewer examples of 1), though it might depend how liberally > >>> we grep out or examine the use cases. Choice 1) also seems to be the > >>> most shotgun approach, adding to the search path for all files. > >> > >> > >> The shotgun approach is always used when compiling out of tree which is > >> what many people do anyway without realizing that there are additional > >> -I. Why do not we make in-tree behave the same way? Thanks, > > > > Are you suggesting bandaging this individual leaf directory, as in > > choice 1), because it's no worse than what happens for an out-of-tree > > build anyway, > > > I suggest making in-tree and out-of-tree behavior equal - both either > fail or compile. Just makes sense to me. > > Since out-of-tree adds extra -I, then there should have been a reason > for that at the time (before 2005 though). Unfortunately git blame does > not say why it was done this way for out-of-tree so imho the safest > thing is to add -I for in-tree as well. Or ditch extra -I and do 2) or > 3) - this is fine too and can count as an impre
Re: [PATCH] vfio_pci: Add local source directory as include
On 08/01/2019 11:24, Alex Williamson wrote: > On Tue, 8 Jan 2019 10:52:43 +1100 > Alexey Kardashevskiy wrote: > >> On 08/01/2019 07:13, Alex Williamson wrote: >>> On Mon, 7 Jan 2019 20:39:19 +0900 >>> Masahiro Yamada wrote: >>> On Mon, Jan 7, 2019 at 8:09 PM Cornelia Huck wrote: > > On Mon, 7 Jan 2019 19:12:10 +0900 > Masahiro Yamada wrote: > >> On Mon, Jan 7, 2019 at 6:18 PM Michael Ellerman >> wrote: >>> >>> Laura Abbott writes: Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] subdriver") introduced a trace.h file in the local directory but missed adding the local include path, resulting in compilation failures with tracepoints: In file included from drivers/vfio/pci/trace.h:102, from drivers/vfio/pci/vfio_pci_nvlink2.c:29: ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No such file or directory #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) Fix this by adjusting the include path. Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] subdriver") Signed-off-by: Laura Abbott > > (...) > >>> Alex I assume you'll merge this fix via the vfio tree? >>> >>> cheers >>> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile index 9662c063a6b1..08d4676a8495 100644 --- a/drivers/vfio/pci/Makefile +++ b/drivers/vfio/pci/Makefile @@ -1,3 +1,4 @@ +ccflags-y += -I$(src) vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o -- 2.20.1 >> >> >> Hi. >> >> If I correctly understand the usage of TRACE_INCLUDE_PATH, >> the correct fix should be like follows: >> >> >> diff --git a/drivers/vfio/pci/trace.h b/drivers/vfio/pci/trace.h >> index 228ccdb..4d13e51 100644 >> --- a/drivers/vfio/pci/trace.h >> +++ b/drivers/vfio/pci/trace.h >> @@ -94,7 +94,7 @@ TRACE_EVENT(vfio_pci_npu2_mmap, >> #endif /* _TRACE_VFIO_PCI_H */ >> >> #undef TRACE_INCLUDE_PATH >> -#define TRACE_INCLUDE_PATH . >> +#define TRACE_INCLUDE_PATH ../../drivers/vfio/pci >> #undef TRACE_INCLUDE_FILE >> #define TRACE_INCLUDE_FILE trace > > Going from the comments in samples/trace_events/trace-events-sample.h, > I think both approaches are possible, and I see both used in various > places. > > Personally, I'd prefer Laura's patch, as it doesn't involve hardcoding > a path. >>> >>> Numbering options for clarity: >>> >>> 1) ccflags-y += -I$(src) would add the header search path for all files in drivers/vfio/pci/ whereas only the drivers/vfio/pci/vfio_pci_nvlink2.c needs it. >>> >>> 2) CFLAGS_vfio_pci_nvlink2.o += -I$(src) is a bit better. However, it is not obvious why this extra header search path is needed until you find vfio_pci_nvlink2.c including trace.h >>> >>> 3) #define TRACE_INCLUDE_PATH ../../drivers/vfio/pci clarifies the intention because the related code is all placed in trace.h From the comment in include/trace/define_trace.h TRACE_INCLUDE_PATH is relative to include/trace/define_trace.h >>> >>> In my scan of the tree, the most common solution seems to be 2) as this >>> is essentially recommended in the sample file. 3) is well represented, >>> with much fewer examples of 1), though it might depend how liberally >>> we grep out or examine the use cases. Choice 1) also seems to be the >>> most shotgun approach, adding to the search path for all files. >> >> >> The shotgun approach is always used when compiling out of tree which is >> what many people do anyway without realizing that there are additional >> -I. Why do not we make in-tree behave the same way? Thanks, > > Are you suggesting bandaging this individual leaf directory, as in > choice 1), because it's no worse than what happens for an out-of-tree > build anyway, I suggest making in-tree and out-of-tree behavior equal - both either fail or compile. Just makes sense to me. Since out-of-tree adds extra -I, then there should have been a reason for that at the time (before 2005 though). Unfortunately git blame does not say why it was done this way for out-of-tree so imho the safest thing is to add -I for in-tree as well. Or ditch extra -I and do 2) or 3) - this is fine too and can count as an impressive compile optimization, although... look below :) > or are you suggesting to fix the in-tree build behavior to > be as inefficient as the out-of-tree behavior in general? Inefficient you say. Hm. I tried this: --- scripts/Makefile.lib.old2019-01-0
Re: [PATCH] vfio_pci: Add local source directory as include
On Tue, 8 Jan 2019 10:52:43 +1100 Alexey Kardashevskiy wrote: > On 08/01/2019 07:13, Alex Williamson wrote: > > On Mon, 7 Jan 2019 20:39:19 +0900 > > Masahiro Yamada wrote: > > > >> On Mon, Jan 7, 2019 at 8:09 PM Cornelia Huck wrote: > >>> > >>> On Mon, 7 Jan 2019 19:12:10 +0900 > >>> Masahiro Yamada wrote: > >>> > On Mon, Jan 7, 2019 at 6:18 PM Michael Ellerman > wrote: > > > > Laura Abbott writes: > >> Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > >> subdriver") introduced a trace.h file in the local directory but > >> missed adding the local include path, resulting in compilation > >> failures with tracepoints: > >> > >> In file included from drivers/vfio/pci/trace.h:102, > >> from drivers/vfio/pci/vfio_pci_nvlink2.c:29: > >> ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No such > >> file or directory > >> #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) > >> > >> Fix this by adjusting the include path. > >> > >> Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > >> subdriver") > >> Signed-off-by: Laura Abbott > >>> > >>> (...) > >>> > > Alex I assume you'll merge this fix via the vfio tree? > > > > cheers > > > >> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile > >> index 9662c063a6b1..08d4676a8495 100644 > >> --- a/drivers/vfio/pci/Makefile > >> +++ b/drivers/vfio/pci/Makefile > >> @@ -1,3 +1,4 @@ > >> +ccflags-y += -I$(src) > >> > >> vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o > >> vfio_pci_config.o > >> vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o > >> -- > >> 2.20.1 > > > Hi. > > If I correctly understand the usage of TRACE_INCLUDE_PATH, > the correct fix should be like follows: > > > diff --git a/drivers/vfio/pci/trace.h b/drivers/vfio/pci/trace.h > index 228ccdb..4d13e51 100644 > --- a/drivers/vfio/pci/trace.h > +++ b/drivers/vfio/pci/trace.h > @@ -94,7 +94,7 @@ TRACE_EVENT(vfio_pci_npu2_mmap, > #endif /* _TRACE_VFIO_PCI_H */ > > #undef TRACE_INCLUDE_PATH > -#define TRACE_INCLUDE_PATH . > +#define TRACE_INCLUDE_PATH ../../drivers/vfio/pci > #undef TRACE_INCLUDE_FILE > #define TRACE_INCLUDE_FILE trace > >>> > >>> Going from the comments in samples/trace_events/trace-events-sample.h, > >>> I think both approaches are possible, and I see both used in various > >>> places. > >>> > >>> Personally, I'd prefer Laura's patch, as it doesn't involve hardcoding > >>> a path. > > > > Numbering options for clarity: > > > > 1) > >> ccflags-y += -I$(src) > >> would add the header search path for all files in drivers/vfio/pci/ > >> whereas only the drivers/vfio/pci/vfio_pci_nvlink2.c needs it. > >> > > > > 2) > >> CFLAGS_vfio_pci_nvlink2.o += -I$(src) > >> is a bit better. > >> However, it is not obvious why this extra header search path is needed > >> until you find vfio_pci_nvlink2.c including trace.h > >> > > > > 3) > >> #define TRACE_INCLUDE_PATH ../../drivers/vfio/pci > >> clarifies the intention because the related code is all placed in trace.h > >> > >> > >> > >> From the comment in include/trace/define_trace.h > >> TRACE_INCLUDE_PATH is relative to include/trace/define_trace.h > > > > In my scan of the tree, the most common solution seems to be 2) as this > > is essentially recommended in the sample file. 3) is well represented, > > with much fewer examples of 1), though it might depend how liberally > > we grep out or examine the use cases. Choice 1) also seems to be the > > most shotgun approach, adding to the search path for all files. > > > The shotgun approach is always used when compiling out of tree which is > what many people do anyway without realizing that there are additional > -I. Why do not we make in-tree behave the same way? Thanks, Are you suggesting bandaging this individual leaf directory, as in choice 1), because it's no worse than what happens for an out-of-tree build anyway, or are you suggesting to fix the in-tree build behavior to be as inefficient as the out-of-tree behavior in general? It appears to me that options 2) and 3) are the intended solutions for this issue while 1) is more of a workaround. Thanks, Alex > > From a > > maintenance perspective I agree that 2) seems more error prone, > > especially when the build system only catches the error on in-tree > > builds, something I rarely do. Therefore I'm leaning towards option > > 3). The hardcoded path here doesn't seem much of an issue relative to > > the negatives of the other approaches (how often do we move these > > files?) and it keeps the trace support relatively self-contained. Are > > there further arguments for or against these options? Otherwise w
Re: [PATCH] vfio_pci: Add local source directory as include
On 08/01/2019 07:13, Alex Williamson wrote: > On Mon, 7 Jan 2019 20:39:19 +0900 > Masahiro Yamada wrote: > >> On Mon, Jan 7, 2019 at 8:09 PM Cornelia Huck wrote: >>> >>> On Mon, 7 Jan 2019 19:12:10 +0900 >>> Masahiro Yamada wrote: >>> On Mon, Jan 7, 2019 at 6:18 PM Michael Ellerman wrote: > > Laura Abbott writes: >> Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] >> subdriver") introduced a trace.h file in the local directory but >> missed adding the local include path, resulting in compilation >> failures with tracepoints: >> >> In file included from drivers/vfio/pci/trace.h:102, >> from drivers/vfio/pci/vfio_pci_nvlink2.c:29: >> ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No such >> file or directory >> #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) >> >> Fix this by adjusting the include path. >> >> Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] >> subdriver") >> Signed-off-by: Laura Abbott >>> >>> (...) >>> > Alex I assume you'll merge this fix via the vfio tree? > > cheers > >> diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile >> index 9662c063a6b1..08d4676a8495 100644 >> --- a/drivers/vfio/pci/Makefile >> +++ b/drivers/vfio/pci/Makefile >> @@ -1,3 +1,4 @@ >> +ccflags-y += -I$(src) >> >> vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o >> vfio_pci_config.o >> vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o >> -- >> 2.20.1 Hi. If I correctly understand the usage of TRACE_INCLUDE_PATH, the correct fix should be like follows: diff --git a/drivers/vfio/pci/trace.h b/drivers/vfio/pci/trace.h index 228ccdb..4d13e51 100644 --- a/drivers/vfio/pci/trace.h +++ b/drivers/vfio/pci/trace.h @@ -94,7 +94,7 @@ TRACE_EVENT(vfio_pci_npu2_mmap, #endif /* _TRACE_VFIO_PCI_H */ #undef TRACE_INCLUDE_PATH -#define TRACE_INCLUDE_PATH . +#define TRACE_INCLUDE_PATH ../../drivers/vfio/pci #undef TRACE_INCLUDE_FILE #define TRACE_INCLUDE_FILE trace >>> >>> Going from the comments in samples/trace_events/trace-events-sample.h, >>> I think both approaches are possible, and I see both used in various >>> places. >>> >>> Personally, I'd prefer Laura's patch, as it doesn't involve hardcoding >>> a path. > > Numbering options for clarity: > > 1) >> ccflags-y += -I$(src) >> would add the header search path for all files in drivers/vfio/pci/ >> whereas only the drivers/vfio/pci/vfio_pci_nvlink2.c needs it. >> > > 2) >> CFLAGS_vfio_pci_nvlink2.o += -I$(src) >> is a bit better. >> However, it is not obvious why this extra header search path is needed >> until you find vfio_pci_nvlink2.c including trace.h >> > > 3) >> #define TRACE_INCLUDE_PATH ../../drivers/vfio/pci >> clarifies the intention because the related code is all placed in trace.h >> >> >> >> From the comment in include/trace/define_trace.h >> TRACE_INCLUDE_PATH is relative to include/trace/define_trace.h > > In my scan of the tree, the most common solution seems to be 2) as this > is essentially recommended in the sample file. 3) is well represented, > with much fewer examples of 1), though it might depend how liberally > we grep out or examine the use cases. Choice 1) also seems to be the > most shotgun approach, adding to the search path for all files. The shotgun approach is always used when compiling out of tree which is what many people do anyway without realizing that there are additional -I. Why do not we make in-tree behave the same way? Thanks, > From a > maintenance perspective I agree that 2) seems more error prone, > especially when the build system only catches the error on in-tree > builds, something I rarely do. Therefore I'm leaning towards option > 3). The hardcoded path here doesn't seem much of an issue relative to > the negatives of the other approaches (how often do we move these > files?) and it keeps the trace support relatively self-contained. Are > there further arguments for or against these options? Otherwise who > wants to formally post the TRACE_INCLUDE_PATH version? Thanks, -- Alexey
Re: [PATCH] vfio_pci: Add local source directory as include
On Mon, 7 Jan 2019 20:39:19 +0900 Masahiro Yamada wrote: > On Mon, Jan 7, 2019 at 8:09 PM Cornelia Huck wrote: > > > > On Mon, 7 Jan 2019 19:12:10 +0900 > > Masahiro Yamada wrote: > > > > > On Mon, Jan 7, 2019 at 6:18 PM Michael Ellerman > > > wrote: > > > > > > > > Laura Abbott writes: > > > > > Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > > > > > subdriver") introduced a trace.h file in the local directory but > > > > > missed adding the local include path, resulting in compilation > > > > > failures with tracepoints: > > > > > > > > > > In file included from drivers/vfio/pci/trace.h:102, > > > > > from drivers/vfio/pci/vfio_pci_nvlink2.c:29: > > > > > ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No such > > > > > file or directory > > > > > #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) > > > > > > > > > > Fix this by adjusting the include path. > > > > > > > > > > Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > > > > > subdriver") > > > > > Signed-off-by: Laura Abbott > > > > (...) > > > > > > Alex I assume you'll merge this fix via the vfio tree? > > > > > > > > cheers > > > > > > > > > diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile > > > > > index 9662c063a6b1..08d4676a8495 100644 > > > > > --- a/drivers/vfio/pci/Makefile > > > > > +++ b/drivers/vfio/pci/Makefile > > > > > @@ -1,3 +1,4 @@ > > > > > +ccflags-y += -I$(src) > > > > > > > > > > vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o > > > > > vfio_pci_config.o > > > > > vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o > > > > > -- > > > > > 2.20.1 > > > > > > > > > Hi. > > > > > > If I correctly understand the usage of TRACE_INCLUDE_PATH, > > > the correct fix should be like follows: > > > > > > > > > diff --git a/drivers/vfio/pci/trace.h b/drivers/vfio/pci/trace.h > > > index 228ccdb..4d13e51 100644 > > > --- a/drivers/vfio/pci/trace.h > > > +++ b/drivers/vfio/pci/trace.h > > > @@ -94,7 +94,7 @@ TRACE_EVENT(vfio_pci_npu2_mmap, > > > #endif /* _TRACE_VFIO_PCI_H */ > > > > > > #undef TRACE_INCLUDE_PATH > > > -#define TRACE_INCLUDE_PATH . > > > +#define TRACE_INCLUDE_PATH ../../drivers/vfio/pci > > > #undef TRACE_INCLUDE_FILE > > > #define TRACE_INCLUDE_FILE trace > > > > Going from the comments in samples/trace_events/trace-events-sample.h, > > I think both approaches are possible, and I see both used in various > > places. > > > > Personally, I'd prefer Laura's patch, as it doesn't involve hardcoding > > a path. Numbering options for clarity: 1) > ccflags-y += -I$(src) > would add the header search path for all files in drivers/vfio/pci/ > whereas only the drivers/vfio/pci/vfio_pci_nvlink2.c needs it. > 2) > CFLAGS_vfio_pci_nvlink2.o += -I$(src) > is a bit better. > However, it is not obvious why this extra header search path is needed > until you find vfio_pci_nvlink2.c including trace.h > 3) > #define TRACE_INCLUDE_PATH ../../drivers/vfio/pci > clarifies the intention because the related code is all placed in trace.h > > > > From the comment in include/trace/define_trace.h > TRACE_INCLUDE_PATH is relative to include/trace/define_trace.h In my scan of the tree, the most common solution seems to be 2) as this is essentially recommended in the sample file. 3) is well represented, with much fewer examples of 1), though it might depend how liberally we grep out or examine the use cases. Choice 1) also seems to be the most shotgun approach, adding to the search path for all files. From a maintenance perspective I agree that 2) seems more error prone, especially when the build system only catches the error on in-tree builds, something I rarely do. Therefore I'm leaning towards option 3). The hardcoded path here doesn't seem much of an issue relative to the negatives of the other approaches (how often do we move these files?) and it keeps the trace support relatively self-contained. Are there further arguments for or against these options? Otherwise who wants to formally post the TRACE_INCLUDE_PATH version? Thanks, Alex
Re: [PATCH] vfio_pci: Add local source directory as include
On 1/7/19 12:58 AM, Michael Ellerman wrote: Laura Abbott writes: Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] subdriver") introduced a trace.h file in the local directory but missed adding the local include path, resulting in compilation failures with tracepoints: In file included from drivers/vfio/pci/trace.h:102, from drivers/vfio/pci/vfio_pci_nvlink2.c:29: ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No such file or directory #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) Fix this by adjusting the include path. Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] subdriver") Signed-off-by: Laura Abbott --- I'd still like to echo my sentiment that this should not be a def_bool. We hit this error on our internal testing and we couldn't even turn off the driver until we fixed this. I assume there's some reason you can't commit a patch to your tree to change it to bool, or turn it off entirely? That would change the SHA which is perhaps reason enough. In general we have far too many options and most of them never get turned off (or on), so it just creates testing/bug surface for not much benefit. This is one that will probably be turned on in all distro kernels for example. But I have no real objection to making it user configurable. Alex I assume you'll merge this fix via the vfio tree? cheers Well now that everything is fixed, we will probably keep it on. My gripe is that when things break like this it's difficult to work around vs. the configuration option at least makes it possible to work around in a fast way. This may also just be a strong preference of mine for working around problems. Thanks, Laura diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile index 9662c063a6b1..08d4676a8495 100644 --- a/drivers/vfio/pci/Makefile +++ b/drivers/vfio/pci/Makefile @@ -1,3 +1,4 @@ +ccflags-y += -I$(src) vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o -- 2.20.1
Re: [PATCH] vfio_pci: Add local source directory as include
On Mon, Jan 7, 2019 at 8:09 PM Cornelia Huck wrote: > > On Mon, 7 Jan 2019 19:12:10 +0900 > Masahiro Yamada wrote: > > > On Mon, Jan 7, 2019 at 6:18 PM Michael Ellerman wrote: > > > > > > Laura Abbott writes: > > > > Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > > > > subdriver") introduced a trace.h file in the local directory but > > > > missed adding the local include path, resulting in compilation > > > > failures with tracepoints: > > > > > > > > In file included from drivers/vfio/pci/trace.h:102, > > > > from drivers/vfio/pci/vfio_pci_nvlink2.c:29: > > > > ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No such > > > > file or directory > > > > #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) > > > > > > > > Fix this by adjusting the include path. > > > > > > > > Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > > > > subdriver") > > > > Signed-off-by: Laura Abbott > > (...) > > > > Alex I assume you'll merge this fix via the vfio tree? > > > > > > cheers > > > > > > > diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile > > > > index 9662c063a6b1..08d4676a8495 100644 > > > > --- a/drivers/vfio/pci/Makefile > > > > +++ b/drivers/vfio/pci/Makefile > > > > @@ -1,3 +1,4 @@ > > > > +ccflags-y += -I$(src) > > > > > > > > vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o > > > > vfio_pci_config.o > > > > vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o > > > > -- > > > > 2.20.1 > > > > > > Hi. > > > > If I correctly understand the usage of TRACE_INCLUDE_PATH, > > the correct fix should be like follows: > > > > > > diff --git a/drivers/vfio/pci/trace.h b/drivers/vfio/pci/trace.h > > index 228ccdb..4d13e51 100644 > > --- a/drivers/vfio/pci/trace.h > > +++ b/drivers/vfio/pci/trace.h > > @@ -94,7 +94,7 @@ TRACE_EVENT(vfio_pci_npu2_mmap, > > #endif /* _TRACE_VFIO_PCI_H */ > > > > #undef TRACE_INCLUDE_PATH > > -#define TRACE_INCLUDE_PATH . > > +#define TRACE_INCLUDE_PATH ../../drivers/vfio/pci > > #undef TRACE_INCLUDE_FILE > > #define TRACE_INCLUDE_FILE trace > > Going from the comments in samples/trace_events/trace-events-sample.h, > I think both approaches are possible, and I see both used in various > places. > > Personally, I'd prefer Laura's patch, as it doesn't involve hardcoding > a path. ccflags-y += -I$(src) would add the header search path for all files in drivers/vfio/pci/ whereas only the drivers/vfio/pci/vfio_pci_nvlink2.c needs it. CFLAGS_vfio_pci_nvlink2.o += -I$(src) is a bit better. However, it is not obvious why this extra header search path is needed until you find vfio_pci_nvlink2.c including trace.h #define TRACE_INCLUDE_PATH ../../drivers/vfio/pci clarifies the intention because the related code is all placed in trace.h >From the comment in include/trace/define_trace.h TRACE_INCLUDE_PATH is relative to include/trace/define_trace.h -- Best Regards Masahiro Yamada
Re: [PATCH] vfio_pci: Add local source directory as include
On Fri, 4 Jan 2019 11:57:14 -0800 Laura Abbott wrote: > Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > subdriver") introduced a trace.h file in the local directory but > missed adding the local include path, resulting in compilation > failures with tracepoints: > > In file included from drivers/vfio/pci/trace.h:102, > from drivers/vfio/pci/vfio_pci_nvlink2.c:29: > ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No such file or > directory > #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) > > Fix this by adjusting the include path. > > Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > subdriver") > Signed-off-by: Laura Abbott > --- > I'd still like to echo my sentiment that this should not be a def_bool. > We hit this error on our internal testing and we couldn't even turn > off the driver until we fixed this. > --- > drivers/vfio/pci/Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile > index 9662c063a6b1..08d4676a8495 100644 > --- a/drivers/vfio/pci/Makefile > +++ b/drivers/vfio/pci/Makefile > @@ -1,3 +1,4 @@ > +ccflags-y += -I$(src) > > vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o > vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o Reviewed-by: Cornelia Huck
Re: [PATCH] vfio_pci: Add local source directory as include
On Mon, 7 Jan 2019 19:12:10 +0900 Masahiro Yamada wrote: > On Mon, Jan 7, 2019 at 6:18 PM Michael Ellerman wrote: > > > > Laura Abbott writes: > > > Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > > > subdriver") introduced a trace.h file in the local directory but > > > missed adding the local include path, resulting in compilation > > > failures with tracepoints: > > > > > > In file included from drivers/vfio/pci/trace.h:102, > > > from drivers/vfio/pci/vfio_pci_nvlink2.c:29: > > > ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No such > > > file or directory > > > #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) > > > > > > Fix this by adjusting the include path. > > > > > > Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > > > subdriver") > > > Signed-off-by: Laura Abbott (...) > > Alex I assume you'll merge this fix via the vfio tree? > > > > cheers > > > > > diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile > > > index 9662c063a6b1..08d4676a8495 100644 > > > --- a/drivers/vfio/pci/Makefile > > > +++ b/drivers/vfio/pci/Makefile > > > @@ -1,3 +1,4 @@ > > > +ccflags-y += -I$(src) > > > > > > vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o > > > vfio_pci_config.o > > > vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o > > > -- > > > 2.20.1 > > > Hi. > > If I correctly understand the usage of TRACE_INCLUDE_PATH, > the correct fix should be like follows: > > > diff --git a/drivers/vfio/pci/trace.h b/drivers/vfio/pci/trace.h > index 228ccdb..4d13e51 100644 > --- a/drivers/vfio/pci/trace.h > +++ b/drivers/vfio/pci/trace.h > @@ -94,7 +94,7 @@ TRACE_EVENT(vfio_pci_npu2_mmap, > #endif /* _TRACE_VFIO_PCI_H */ > > #undef TRACE_INCLUDE_PATH > -#define TRACE_INCLUDE_PATH . > +#define TRACE_INCLUDE_PATH ../../drivers/vfio/pci > #undef TRACE_INCLUDE_FILE > #define TRACE_INCLUDE_FILE trace Going from the comments in samples/trace_events/trace-events-sample.h, I think both approaches are possible, and I see both used in various places. Personally, I'd prefer Laura's patch, as it doesn't involve hardcoding a path.
Re: [PATCH] vfio_pci: Add local source directory as include
On Mon, Jan 7, 2019 at 6:18 PM Michael Ellerman wrote: > > Laura Abbott writes: > > Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > > subdriver") introduced a trace.h file in the local directory but > > missed adding the local include path, resulting in compilation > > failures with tracepoints: > > > > In file included from drivers/vfio/pci/trace.h:102, > > from drivers/vfio/pci/vfio_pci_nvlink2.c:29: > > ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No such file > > or directory > > #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) > > > > Fix this by adjusting the include path. > > > > Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > > subdriver") > > Signed-off-by: Laura Abbott > > --- > > I'd still like to echo my sentiment that this should not be a def_bool. > > We hit this error on our internal testing and we couldn't even turn > > off the driver until we fixed this. > > I assume there's some reason you can't commit a patch to your tree to > change it to bool, or turn it off entirely? That would change the SHA > which is perhaps reason enough. > > In general we have far too many options and most of them never get > turned off (or on), so it just creates testing/bug surface for not much > benefit. This is one that will probably be turned on in all distro > kernels for example. > > But I have no real objection to making it user configurable. > > > Alex I assume you'll merge this fix via the vfio tree? > > cheers > > > diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile > > index 9662c063a6b1..08d4676a8495 100644 > > --- a/drivers/vfio/pci/Makefile > > +++ b/drivers/vfio/pci/Makefile > > @@ -1,3 +1,4 @@ > > +ccflags-y += -I$(src) > > > > vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o > > vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o > > -- > > 2.20.1 Hi. If I correctly understand the usage of TRACE_INCLUDE_PATH, the correct fix should be like follows: diff --git a/drivers/vfio/pci/trace.h b/drivers/vfio/pci/trace.h index 228ccdb..4d13e51 100644 --- a/drivers/vfio/pci/trace.h +++ b/drivers/vfio/pci/trace.h @@ -94,7 +94,7 @@ TRACE_EVENT(vfio_pci_npu2_mmap, #endif /* _TRACE_VFIO_PCI_H */ #undef TRACE_INCLUDE_PATH -#define TRACE_INCLUDE_PATH . +#define TRACE_INCLUDE_PATH ../../drivers/vfio/pci #undef TRACE_INCLUDE_FILE #define TRACE_INCLUDE_FILE trace -- Best Regards Masahiro Yamada
Re: [PATCH] vfio_pci: Add local source directory as include
Laura Abbott writes: > Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > subdriver") introduced a trace.h file in the local directory but > missed adding the local include path, resulting in compilation > failures with tracepoints: > > In file included from drivers/vfio/pci/trace.h:102, > from drivers/vfio/pci/vfio_pci_nvlink2.c:29: > ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No such file or > directory > #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) > > Fix this by adjusting the include path. > > Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > subdriver") > Signed-off-by: Laura Abbott > --- > I'd still like to echo my sentiment that this should not be a def_bool. > We hit this error on our internal testing and we couldn't even turn > off the driver until we fixed this. I assume there's some reason you can't commit a patch to your tree to change it to bool, or turn it off entirely? That would change the SHA which is perhaps reason enough. In general we have far too many options and most of them never get turned off (or on), so it just creates testing/bug surface for not much benefit. This is one that will probably be turned on in all distro kernels for example. But I have no real objection to making it user configurable. Alex I assume you'll merge this fix via the vfio tree? cheers > diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile > index 9662c063a6b1..08d4676a8495 100644 > --- a/drivers/vfio/pci/Makefile > +++ b/drivers/vfio/pci/Makefile > @@ -1,3 +1,4 @@ > +ccflags-y += -I$(src) > > vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o > vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o > -- > 2.20.1
Re: [PATCH] vfio_pci: Add local source directory as include
On 05/01/2019 06:57, Laura Abbott wrote: > Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > subdriver") introduced a trace.h file in the local directory but > missed adding the local include path, resulting in compilation > failures with tracepoints: > > In file included from drivers/vfio/pci/trace.h:102, > from drivers/vfio/pci/vfio_pci_nvlink2.c:29: > ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No such file or > directory > #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) > > Fix this by adjusting the include path. > > Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] > subdriver") > Signed-off-by: Laura Abbott Reviewed-by: Alexey Kardashevskiy > --- > I'd still like to echo my sentiment that this should not be a def_bool. > We hit this error on our internal testing and we couldn't even turn > off the driver until we fixed this. > --- > drivers/vfio/pci/Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile > index 9662c063a6b1..08d4676a8495 100644 > --- a/drivers/vfio/pci/Makefile > +++ b/drivers/vfio/pci/Makefile > @@ -1,3 +1,4 @@ > +ccflags-y += -I$(src) > > vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o > vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o > -- Alexey
[PATCH] vfio_pci: Add local source directory as include
Commit 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] subdriver") introduced a trace.h file in the local directory but missed adding the local include path, resulting in compilation failures with tracepoints: In file included from drivers/vfio/pci/trace.h:102, from drivers/vfio/pci/vfio_pci_nvlink2.c:29: ./include/trace/define_trace.h:89:42: fatal error: ./trace.h: No such file or directory #include TRACE_INCLUDE(TRACE_INCLUDE_FILE) Fix this by adjusting the include path. Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] subdriver") Signed-off-by: Laura Abbott --- I'd still like to echo my sentiment that this should not be a def_bool. We hit this error on our internal testing and we couldn't even turn off the driver until we fixed this. --- drivers/vfio/pci/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile index 9662c063a6b1..08d4676a8495 100644 --- a/drivers/vfio/pci/Makefile +++ b/drivers/vfio/pci/Makefile @@ -1,3 +1,4 @@ +ccflags-y += -I$(src) vfio-pci-y := vfio_pci.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o -- 2.20.1