vfio_pci: set TRACE_INCLUDE_PATH to fix the build error
diff mbox series

Message ID 1546916883-25911-1-git-send-email-yamada.masahiro@socionext.com
State In Next
Commit d1fc1176c055c9ec9c6ec4d113a284e0bad9d09a
Headers show
Series
  • vfio_pci: set TRACE_INCLUDE_PATH to fix the build error
Related show

Commit Message

Masahiro Yamada Jan. 8, 2019, 3:08 a.m. UTC
drivers/vfio/pci/vfio_pci_nvlink2.c cannot be compiled for in-tree
building.

    CC      drivers/vfio/pci/vfio_pci_nvlink2.o
  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)
                                          ^
  compilation terminated.
  make[1]: *** [scripts/Makefile.build;277: drivers/vfio/pci/vfio_pci_nvlink2.o] Error 1

To fix the build error, let's tell include/trace/define_trace.h the
location of drivers/vfio/pci/trace.h

Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] subdriver")
Reported-by: Laura Abbott <labbott@redhat.com>
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/vfio/pci/trace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Cornelia Huck Jan. 8, 2019, 9:47 a.m. UTC | #1
On Tue,  8 Jan 2019 12:08:03 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> drivers/vfio/pci/vfio_pci_nvlink2.c cannot be compiled for in-tree
> building.
> 
>     CC      drivers/vfio/pci/vfio_pci_nvlink2.o
>   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)
>                                           ^
>   compilation terminated.
>   make[1]: *** [scripts/Makefile.build;277: drivers/vfio/pci/vfio_pci_nvlink2.o] Error 1
> 
> To fix the build error, let's tell include/trace/define_trace.h the
> location of drivers/vfio/pci/trace.h
> 
> Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] subdriver")
> Reported-by: Laura Abbott <labbott@redhat.com>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  drivers/vfio/pci/trace.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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
>  

I think both this and the other fix option via the Makefile are fine
(even though I still slightly prefer the latter), so I'll leave the
choice to the maintainer.

Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Alex Williamson Jan. 8, 2019, 7:28 p.m. UTC | #2
On Tue,  8 Jan 2019 12:08:03 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> drivers/vfio/pci/vfio_pci_nvlink2.c cannot be compiled for in-tree
> building.
> 
>     CC      drivers/vfio/pci/vfio_pci_nvlink2.o
>   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)
>                                           ^
>   compilation terminated.
>   make[1]: *** [scripts/Makefile.build;277: drivers/vfio/pci/vfio_pci_nvlink2.o] Error 1
> 
> To fix the build error, let's tell include/trace/define_trace.h the
> location of drivers/vfio/pci/trace.h
> 
> Fixes: 7f92891778df ("vfio_pci: Add NVIDIA GV100GL [Tesla V100 SXM2] subdriver")
> Reported-by: Laura Abbott <labbott@redhat.com>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---

Thanks for posting this, it's my preferred fix.  I'll give it another
day to collect reviews/objections then pop it into my for-linus branch
for rc2.  Thanks!

Alex
 
>  drivers/vfio/pci/trace.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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
>
Steven Rostedt Jan. 10, 2019, 2:47 p.m. UTC | #3
On Tue, Jan 08, 2019 at 12:08:03PM +0900, Masahiro Yamada wrote:
> ---
> 
>  drivers/vfio/pci/trace.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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

Note, the reason why I did not show this method in the samples/trace_events/
is that there's one "gotcha" that you need to be careful about. It may not be
an issue here, but please be aware of it.

The words in TRACE_INCLUDE_PATH can be updated by C preprocessor defines. For
example, if for some reason you had:

#define pci special_pci

The above would turn into:

 ../../drivers/vfio/special_pci

and it wont build, and you will be left scratching your head wondering why.

-- Steve

>  #undef TRACE_INCLUDE_FILE
>  #define TRACE_INCLUDE_FILE trace
>  
> -- 
> 2.7.4
Alexey Kardashevskiy Jan. 11, 2019, 1:13 a.m. UTC | #4
On 11/01/2019 01:47, Steven Rostedt wrote:
> On Tue, Jan 08, 2019 at 12:08:03PM +0900, Masahiro Yamada wrote:
>> ---
>>
>>  drivers/vfio/pci/trace.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> 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
> 
> Note, the reason why I did not show this method in the samples/trace_events/
> is that there's one "gotcha" that you need to be careful about. It may not be
> an issue here, but please be aware of it.
> 
> The words in TRACE_INCLUDE_PATH can be updated by C preprocessor defines. For
> example, if for some reason you had:
> 
> #define pci special_pci
> 
> The above would turn into:
> 
>  ../../drivers/vfio/special_pci
> 
> and it wont build, and you will be left scratching your head wondering why.

Lovely :) imho it is +1 for
CFLAGS_vfio_pci_nvlink2.o += -I$(src)
and a comment.
Alex Williamson Jan. 11, 2019, 1:41 a.m. UTC | #5
On Fri, 11 Jan 2019 12:13:35 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> On 11/01/2019 01:47, Steven Rostedt wrote:
> > On Tue, Jan 08, 2019 at 12:08:03PM +0900, Masahiro Yamada wrote:  
> >> ---
> >>
> >>  drivers/vfio/pci/trace.h | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> 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  
> > 
> > Note, the reason why I did not show this method in the samples/trace_events/
> > is that there's one "gotcha" that you need to be careful about. It may not be
> > an issue here, but please be aware of it.
> > 
> > The words in TRACE_INCLUDE_PATH can be updated by C preprocessor defines. For
> > example, if for some reason you had:
> > 
> > #define pci special_pci
> > 
> > The above would turn into:
> > 
> >  ../../drivers/vfio/special_pci
> > 
> > and it wont build, and you will be left scratching your head wondering why.  

Thanks for the info Steve, that'd definitely be a head scratcher, but
it also seems really unlikely for this path.

> Lovely :) imho it is +1 for
> CFLAGS_vfio_pci_nvlink2.o += -I$(src)
> and a comment.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d1fc1176c055c9ec9c6ec4d113a284e0bad9d09a

Obviously we can still refine further, but I don't see this new piece
of information making a meaningful difference in the choice.  Thanks,

Alex
Steven Rostedt Jan. 11, 2019, 1:49 a.m. UTC | #6
On Fri, 11 Jan 2019 12:13:35 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> > The words in TRACE_INCLUDE_PATH can be updated by C preprocessor defines. For
> > example, if for some reason you had:
> > 
> > #define pci special_pci
> > 
> > The above would turn into:
> > 
> >  ../../drivers/vfio/special_pci
> > 
> > and it wont build, and you will be left scratching your head wondering why.  
> 
> Lovely :) imho it is +1 for
> CFLAGS_vfio_pci_nvlink2.o += -I$(src)
> and a comment.

A more realistic example is:

#define pci 1

which I hit when I first tried to do it this way when I first
implemented this code (not with "pci" but a similar word).

I'll leave this up to the maintainers of the code to decide which way
they want to do it, as they are the ones that have to deal with the
fallout if something goes wrong ;-)

-- Steve

Patch
diff mbox series

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