Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] vfio_pci: Add local source directory as include
To:     Michael Ellerman <mpe@ellerman.id.au>,
        Alexey Kardashevskiy <aik@ozlabs.ru>,
        Alex Williamson <alex.williamson@redhat.com>
Cc:     kvm@vger.kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
References: <20190104195714.30045-1-labbott@redhat.com>
 <874lakc09x.fsf@concordia.ellerman.id.au>
From:   Laura Abbott <labbott@redhat.com>
Message-ID: <ee4ca932-36ea-73ba-5d96-6359dad1d114@redhat.com>
Date:   Mon, 7 Jan 2019 12:06:58 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.3.1
MIME-Version: 1.0
In-Reply-To: <874lakc09x.fsf@concordia.ellerman.id.au>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 1/7/19 12:58 AM, Michael Ellerman wrote:
> Laura Abbott <labbott@redhat.com> 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 <labbott@redhat.com>
>> ---
>> 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