From mboxrd@z Thu Jan 1 00:00:00 1970 From: jani.nikula@linux.intel.com (Jani Nikula) Date: Fri, 02 Feb 2018 10:36:18 +0200 Subject: clang warning: implicit conversion in intel_ddi.c:1481 In-Reply-To: References: <20180201180240.GA28042@kroah.com> Message-ID: <87372jkcu5.fsf@intel.com> To: kernelnewbies@lists.kernelnewbies.org List-Id: kernelnewbies.lists.kernelnewbies.org On Thu, 01 Feb 2018, Lukas Bulwahn wrote: > Hi Greg, > > On Thu, 1 Feb 2018, Greg KH wrote: > >> On Thu, Feb 01, 2018 at 06:33:30PM +0100, Ozan Alpay wrote: >> > Dear Rodrigo Vivi, Ville Syrj?l?, >> > >> > My name is Ozan Alpay, and I am a student mentored by Lukas Bulwahn. We >> > intend to use static analysis tools on the kernel source to identify, >> > analyze and report issues. As a very first step, we are looking into >> > clang compiler warnings and will then move to more sophisticated tools. >> > >> > [...] >> > >> > Linux 4.15 is shipped with this clang warning, but we don't see the >> > crucial need to provide a backport commit to the stable branch for 4.15. >> > We just wanted to inform you about our analysis of this clang warning. >> > Ultimately the final call if you would like to address this clang warning >> > in 4.15 is yours. >> >> Note, I have not taken "clang warning fixes" for stable kernel updates >> in the past, and I doubt I will in the future, unless the tree "builds >> clean" with clang. If it eventually gets there, then yes, I will do >> that. >> >> Note, if you are going to email this out to everyone who fixes a warning >> message, you might want to reconsider it. That's going to be a lot of >> work, and for people who have already fixed an issue, it's kind of >> pointless to just remind them of work they have done in the past, right? >> >> What is the goal of these types of emails? >> > > We are interested in providing useful information on potential bugs or bug > patterns that we get from static analysis tools, after we pre-assess them > and manually select them to send to the review process of the patch > submission. > > For me, the clang warnings were an easy starting point for a student to > set up and look at, compared to much more sophisticated tools, such as > coccinelle, sparse or new tools for the kernel development, such as CMBC > or facebook's Infer. > > Once we really understand which tools are useful and which information can > be quickly pre-assessed and sent out in a useful way without just creating > more noise in the discussion, I would have contacted the 0-day > infrastructure team or the kernelci.org team to continue the discussion > how to include our first setup into a proper semi-automated service. > > Using the clang warnings, I wanted to explore how this would even > potentially work. > > Considering clang, of course, currently, we cannot compile the whole > kernel with all possible kernel configurations with clang, but I believe > Nick Desaulniers, Matthias Kaehlcke and others are already working on > that and are getting close to this goal. Hence, assuming they will be > successful soon, I wanted to explore the next step of using static > analysis tools around the clang/LLVM compiler. > > Actually, v4.15 builds almost "cleanly" with clang: For defconfig, there > are only two clang compiler warnings and the one that we looked into > deeper here is already resolved in linux-next, so chances are actually > high that we might get to this "builds clean" soon-ish, at least for > defconfig. > > Concerning clang warnings and how to progress towards that goal of > building cleanly, my strategy is to identify when new clang compiler > warnings are introduced and just point these warnings out as code smells > during the review discussion before they are merged into the > first maintainer tree. If we manually inspect these clang warnings > to make sure that they are genuine code smells of some "imprecise > implementation" before we send them to the mailing list, I would hope that > they are of some value for the developer in the submission process and > he/she could address the warning in a patch v2 while he/she is reacting to > the other review comments from the human reviewers. > > At best, I always considered them as useful information during the review > process, but I certainly DO NOT want to start patching the kernel due to > clang warnings. The chances/risk that we just break more due to naively > fixing warnings without proper understanding is much higher than the > benefit of actually improving the situation. If I recall correctly, I > think this is also one of the lessons learned from motivating newcomers > to address warnings in previous kernel newbies activities. > > Greg, do you think it is worthwhile to invest some effort to move > towards the goal "kernel builds cleanly with clang"? > Would you agree that providing information during the patch review is a > good way to move forward to this goal if we find a suitable manner to > provide this feedback quickly and cleanly at this very early stage of > development? > > If not, we will immediately stop to move in this direction and this is the > first and last email that you have seen of this kind, and we will have to > come up with better/other ideas around work on the Linux kernel. > > If so, we will continue in the direction sketched above, and I think I > just have to point out and apologize for the two obvious things that we > did wrong in this specific case here: > > - We noticed that there were further changes in linux-next, but we > thought that our investigation on v4.15 was valuable nevertheless > for the developers that had touched the source code that we looked at, > although, there is nothing to be done if commits from linux-next are > merged into Linus' tree soon. Taking your response, we have clearly > been WRONG here, overestimating our contribution versus the noise > ratio that we contribute to. > > - We looked at a clang warning, for which we could only provide the > information on this clang warning at this very late stage, i.e., when > the commit under investigation has already been merged and the kernel > was released. So pointing out shortcoming of that kind seems to have > no value, as you, Greg, would not backport commits to stable anyway. > > This has been both errors on my side as a mentor. After my student > has started this week and has worked hard for a week learning a lot about > Linux kernel development and all the tools around it, I did not want to > discourage him and say that the goal set at the beginning of the week to > identify and report a code smell on one commit on the mailing list has > not been achieved as for the reasons above. Instead, we decided to send it > out and were interested in the general reception of our work of this first > week. > > I apologize for that and hope we can leave the specific reported issue now > just rest in peace. > > This experiment shows that I still need to improve my understanding how > to contribute properly to the kernel development. At least to me, the > policy on clang warnings was not clear; and I have learned this now in > this more indirect way. > > We only sent out this one email to see if clang warnings are of interest > at all, and we are glad that you came back to us so quickly with feedback. > > Greg, if you can continue to be a sparing partner and point out when > we are moving in the wrong direction, we will try our best to understand > how we can contribute to turn results from bug finders and static analysis > tools with the manual pre-assessment we can do into valuable contributions > on the mailing list and the linux kernel development. > > We certainly do not intend to spam the mailing list with reports of > findings nobody cares about. Being brutally honest, please write shorter reports and shorter emails to the lists. The static analysis reports are welcome, but only when 1) we didn't already fix it in linux-next, or 2) it reveals an actual bug, not just a warning, warranting a backport. Thanks, Jani. -- Jani Nikula, Intel Open Source Technology Center From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: clang warning: implicit conversion in intel_ddi.c:1481 Date: Fri, 02 Feb 2018 10:36:18 +0200 Message-ID: <87372jkcu5.fsf@intel.com> References: <20180201180240.GA28042@kroah.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Greg KH Cc: sil2review@lists.osadl.org, kernelnewbies@kernelnewbies.org, David Airlie , intel-gfx@lists.freedesktop.org, llvmlinux@lists.linuxfoundation.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Rodrigo Vivi , Ozan Alpay , lukas.bulwahn@gmail.com List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCAwMSBGZWIgMjAxOCwgTHVrYXMgQnVsd2FobiA8bHVrYXMuYnVsd2FobkBnbWFpbC5j b20+IHdyb3RlOgo+IEhpIEdyZWcsCj4KPiBPbiBUaHUsIDEgRmViIDIwMTgsIEdyZWcgS0ggd3Jv dGU6Cj4KPj4gT24gVGh1LCBGZWIgMDEsIDIwMTggYXQgMDY6MzM6MzBQTSArMDEwMCwgT3phbiBB bHBheSB3cm90ZToKPj4gPiBEZWFyIFJvZHJpZ28gVml2aSwgVmlsbGUgU3lyasOkbMOkLAo+PiA+ IAo+PiA+IE15IG5hbWUgaXMgT3phbiBBbHBheSwgYW5kIEkgYW0gYSBzdHVkZW50IG1lbnRvcmVk IGJ5IEx1a2FzIEJ1bHdhaG4uIFdlIAo+PiA+IGludGVuZCB0byB1c2Ugc3RhdGljIGFuYWx5c2lz IHRvb2xzIG9uIHRoZSBrZXJuZWwgc291cmNlIHRvIGlkZW50aWZ5LCAKPj4gPiBhbmFseXplIGFu ZCByZXBvcnQgaXNzdWVzLiBBcyBhIHZlcnkgZmlyc3Qgc3RlcCwgd2UgYXJlIGxvb2tpbmcgaW50 byAKPj4gPiBjbGFuZyBjb21waWxlciB3YXJuaW5ncyBhbmQgd2lsbCB0aGVuIG1vdmUgdG8gbW9y ZSBzb3BoaXN0aWNhdGVkIHRvb2xzLiAKPj4gPgo+PiA+IFsuLi5dCj4+ID4KPj4gPiBMaW51eCA0 LjE1IGlzIHNoaXBwZWQgd2l0aCB0aGlzIGNsYW5nIHdhcm5pbmcsIGJ1dCB3ZSBkb24ndCBzZWUg dGhlIAo+PiA+IGNydWNpYWwgbmVlZCB0byBwcm92aWRlIGEgYmFja3BvcnQgY29tbWl0IHRvIHRo ZSBzdGFibGUgYnJhbmNoIGZvciA0LjE1LiAKPj4gPiBXZSBqdXN0IHdhbnRlZCB0byBpbmZvcm0g eW91IGFib3V0IG91ciBhbmFseXNpcyBvZiB0aGlzIGNsYW5nIHdhcm5pbmcuIAo+PiA+IFVsdGlt YXRlbHkgdGhlIGZpbmFsIGNhbGwgaWYgeW91IHdvdWxkIGxpa2UgdG8gYWRkcmVzcyB0aGlzIGNs YW5nIHdhcm5pbmcgCj4+ID4gaW4gNC4xNSBpcyB5b3Vycy4KPj4gCj4+IE5vdGUsIEkgaGF2ZSBu b3QgdGFrZW4gImNsYW5nIHdhcm5pbmcgZml4ZXMiIGZvciBzdGFibGUga2VybmVsIHVwZGF0ZXMK Pj4gaW4gdGhlIHBhc3QsIGFuZCBJIGRvdWJ0IEkgd2lsbCBpbiB0aGUgZnV0dXJlLCB1bmxlc3Mg dGhlIHRyZWUgImJ1aWxkcwo+PiBjbGVhbiIgd2l0aCBjbGFuZy4gIElmIGl0IGV2ZW50dWFsbHkg Z2V0cyB0aGVyZSwgdGhlbiB5ZXMsIEkgd2lsbCBkbwo+PiB0aGF0Lgo+PiAKPj4gTm90ZSwgaWYg eW91IGFyZSBnb2luZyB0byBlbWFpbCB0aGlzIG91dCB0byBldmVyeW9uZSB3aG8gZml4ZXMgYSB3 YXJuaW5nCj4+IG1lc3NhZ2UsIHlvdSBtaWdodCB3YW50IHRvIHJlY29uc2lkZXIgaXQuICBUaGF0 J3MgZ29pbmcgdG8gYmUgYSBsb3Qgb2YKPj4gd29yaywgYW5kIGZvciBwZW9wbGUgd2hvIGhhdmUg YWxyZWFkeSBmaXhlZCBhbiBpc3N1ZSwgaXQncyBraW5kIG9mCj4+IHBvaW50bGVzcyB0byBqdXN0 IHJlbWluZCB0aGVtIG9mIHdvcmsgdGhleSBoYXZlIGRvbmUgaW4gdGhlIHBhc3QsIHJpZ2h0Pwo+ PiAKPj4gV2hhdCBpcyB0aGUgZ29hbCBvZiB0aGVzZSB0eXBlcyBvZiBlbWFpbHM/Cj4+IAo+Cj4g V2UgYXJlIGludGVyZXN0ZWQgaW4gcHJvdmlkaW5nIHVzZWZ1bCBpbmZvcm1hdGlvbiBvbiBwb3Rl bnRpYWwgYnVncyBvciBidWcgCj4gcGF0dGVybnMgdGhhdCB3ZSBnZXQgZnJvbSBzdGF0aWMgYW5h bHlzaXMgdG9vbHMsIGFmdGVyIHdlIHByZS1hc3Nlc3MgdGhlbSAKPiBhbmQgbWFudWFsbHkgc2Vs ZWN0IHRoZW0gdG8gc2VuZCB0byB0aGUgcmV2aWV3IHByb2Nlc3Mgb2YgdGhlIHBhdGNoIAo+IHN1 Ym1pc3Npb24uCj4KPiBGb3IgbWUsIHRoZSBjbGFuZyB3YXJuaW5ncyB3ZXJlIGFuIGVhc3kgc3Rh cnRpbmcgcG9pbnQgZm9yIGEgc3R1ZGVudCB0byAKPiBzZXQgdXAgYW5kIGxvb2sgYXQsIGNvbXBh cmVkIHRvIG11Y2ggbW9yZSBzb3BoaXN0aWNhdGVkIHRvb2xzLCBzdWNoIGFzIAo+IGNvY2NpbmVs bGUsIHNwYXJzZSBvciBuZXcgdG9vbHMgZm9yIHRoZSBrZXJuZWwgZGV2ZWxvcG1lbnQsIHN1Y2gg YXMgQ01CQyAKPiBvciBmYWNlYm9vaydzIEluZmVyLgo+Cj4gT25jZSB3ZSByZWFsbHkgdW5kZXJz dGFuZCB3aGljaCB0b29scyBhcmUgdXNlZnVsIGFuZCB3aGljaCBpbmZvcm1hdGlvbiBjYW4gCj4g YmUgcXVpY2tseSBwcmUtYXNzZXNzZWQgYW5kIHNlbnQgb3V0IGluIGEgdXNlZnVsIHdheSB3aXRo b3V0IGp1c3QgY3JlYXRpbmcgCj4gbW9yZSBub2lzZSBpbiB0aGUgZGlzY3Vzc2lvbiwgSSB3b3Vs ZCBoYXZlIGNvbnRhY3RlZCB0aGUgMC1kYXkgCj4gaW5mcmFzdHJ1Y3R1cmUgdGVhbSBvciB0aGUg a2VybmVsY2kub3JnIHRlYW0gdG8gY29udGludWUgdGhlIGRpc2N1c3Npb24gCj4gaG93IHRvIGlu Y2x1ZGUgb3VyIGZpcnN0IHNldHVwIGludG8gYSBwcm9wZXIgc2VtaS1hdXRvbWF0ZWQgc2Vydmlj ZS4KPgo+IFVzaW5nIHRoZSBjbGFuZyB3YXJuaW5ncywgSSB3YW50ZWQgdG8gZXhwbG9yZSBob3cg dGhpcyB3b3VsZCBldmVuIAo+IHBvdGVudGlhbGx5IHdvcmsuCj4KPiBDb25zaWRlcmluZyBjbGFu Zywgb2YgY291cnNlLCBjdXJyZW50bHksIHdlIGNhbm5vdCBjb21waWxlIHRoZSB3aG9sZSAKPiBr ZXJuZWwgd2l0aCBhbGwgcG9zc2libGUga2VybmVsIGNvbmZpZ3VyYXRpb25zIHdpdGggY2xhbmcs IGJ1dCBJIGJlbGlldmUgCj4gTmljayBEZXNhdWxuaWVycywgTWF0dGhpYXMgS2FlaGxja2UgYW5k IG90aGVycyBhcmUgYWxyZWFkeSB3b3JraW5nIG9uIAo+IHRoYXQgYW5kIGFyZSBnZXR0aW5nIGNs b3NlIHRvIHRoaXMgZ29hbC4gSGVuY2UsIGFzc3VtaW5nIHRoZXkgd2lsbCBiZSAKPiBzdWNjZXNz ZnVsIHNvb24sIEkgd2FudGVkIHRvIGV4cGxvcmUgdGhlIG5leHQgc3RlcCBvZiB1c2luZyBzdGF0 aWMgCj4gYW5hbHlzaXMgdG9vbHMgYXJvdW5kIHRoZSBjbGFuZy9MTFZNIGNvbXBpbGVyLgo+Cj4g QWN0dWFsbHksIHY0LjE1IGJ1aWxkcyBhbG1vc3QgImNsZWFubHkiIHdpdGggY2xhbmc6IEZvciBk ZWZjb25maWcsIHRoZXJlIAo+IGFyZSBvbmx5IHR3byBjbGFuZyBjb21waWxlciB3YXJuaW5ncyBh bmQgdGhlIG9uZSB0aGF0IHdlIGxvb2tlZCBpbnRvIAo+IGRlZXBlciBoZXJlIGlzIGFscmVhZHkg cmVzb2x2ZWQgaW4gbGludXgtbmV4dCwgc28gY2hhbmNlcyBhcmUgYWN0dWFsbHkgCj4gaGlnaCB0 aGF0IHdlIG1pZ2h0IGdldCB0byB0aGlzICJidWlsZHMgY2xlYW4iIHNvb24taXNoLCBhdCBsZWFz dCBmb3IgCj4gZGVmY29uZmlnLgo+Cj4gQ29uY2VybmluZyBjbGFuZyB3YXJuaW5ncyBhbmQgaG93 IHRvIHByb2dyZXNzIHRvd2FyZHMgdGhhdCBnb2FsIG9mIAo+IGJ1aWxkaW5nIGNsZWFubHksIG15 IHN0cmF0ZWd5IGlzIHRvIGlkZW50aWZ5IHdoZW4gbmV3IGNsYW5nIGNvbXBpbGVyCj4gd2Fybmlu Z3MgYXJlIGludHJvZHVjZWQgYW5kIGp1c3QgcG9pbnQgdGhlc2Ugd2FybmluZ3Mgb3V0IGFzIGNv ZGUgc21lbGxzIAo+IGR1cmluZyB0aGUgcmV2aWV3IGRpc2N1c3Npb24gYmVmb3JlIHRoZXkgYXJl IG1lcmdlZCBpbnRvIHRoZSAKPiBmaXJzdCBtYWludGFpbmVyIHRyZWUuIElmIHdlIG1hbnVhbGx5 IGluc3BlY3QgdGhlc2UgY2xhbmcgd2FybmluZ3MKPiB0byBtYWtlIHN1cmUgdGhhdCB0aGV5IGFy ZSBnZW51aW5lIGNvZGUgc21lbGxzIG9mIHNvbWUgImltcHJlY2lzZSAKPiBpbXBsZW1lbnRhdGlv biIgYmVmb3JlIHdlIHNlbmQgdGhlbSB0byB0aGUgbWFpbGluZyBsaXN0LCBJIHdvdWxkIGhvcGUg dGhhdCAKPiB0aGV5IGFyZSBvZiBzb21lIHZhbHVlIGZvciB0aGUgZGV2ZWxvcGVyIGluIHRoZSBz dWJtaXNzaW9uIHByb2Nlc3MgYW5kIAo+IGhlL3NoZSBjb3VsZCBhZGRyZXNzIHRoZSB3YXJuaW5n IGluIGEgcGF0Y2ggdjIgd2hpbGUgaGUvc2hlIGlzIHJlYWN0aW5nIHRvIAo+IHRoZSBvdGhlciBy ZXZpZXcgY29tbWVudHMgZnJvbSB0aGUgaHVtYW4gcmV2aWV3ZXJzLgo+Cj4gQXQgYmVzdCwgSSBh bHdheXMgY29uc2lkZXJlZCB0aGVtIGFzIHVzZWZ1bCBpbmZvcm1hdGlvbiBkdXJpbmcgdGhlIHJl dmlldyAKPiBwcm9jZXNzLCBidXQgSSBjZXJ0YWlubHkgRE8gTk9UIHdhbnQgdG8gc3RhcnQgcGF0 Y2hpbmcgdGhlIGtlcm5lbCBkdWUgdG8gCj4gY2xhbmcgd2FybmluZ3MuIFRoZSBjaGFuY2VzL3Jp c2sgdGhhdCB3ZSBqdXN0IGJyZWFrIG1vcmUgZHVlIHRvIG5haXZlbHkgCj4gZml4aW5nIHdhcm5p bmdzIHdpdGhvdXQgcHJvcGVyIHVuZGVyc3RhbmRpbmcgaXMgbXVjaCBoaWdoZXIgdGhhbiB0aGUg IAo+IGJlbmVmaXQgb2YgYWN0dWFsbHkgaW1wcm92aW5nIHRoZSBzaXR1YXRpb24uIElmIEkgcmVj YWxsIGNvcnJlY3RseSwgSSAKPiB0aGluayB0aGlzIGlzIGFsc28gb25lIG9mIHRoZSBsZXNzb25z IGxlYXJuZWQgZnJvbSBtb3RpdmF0aW5nIG5ld2NvbWVycyAKPiB0byBhZGRyZXNzIHdhcm5pbmdz IGluIHByZXZpb3VzIGtlcm5lbCBuZXdiaWVzIGFjdGl2aXRpZXMuCj4KPiBHcmVnLCBkbyB5b3Ug dGhpbmsgaXQgaXMgd29ydGh3aGlsZSB0byBpbnZlc3Qgc29tZSBlZmZvcnQgdG8gbW92ZSAKPiB0 b3dhcmRzIHRoZSBnb2FsICJrZXJuZWwgYnVpbGRzIGNsZWFubHkgd2l0aCBjbGFuZyI/Cj4gV291 bGQgeW91IGFncmVlIHRoYXQgcHJvdmlkaW5nIGluZm9ybWF0aW9uIGR1cmluZyB0aGUgcGF0Y2gg cmV2aWV3IGlzIGEgCj4gZ29vZCB3YXkgdG8gbW92ZSBmb3J3YXJkIHRvIHRoaXMgZ29hbCBpZiB3 ZSBmaW5kIGEgc3VpdGFibGUgbWFubmVyIHRvIAo+IHByb3ZpZGUgdGhpcyBmZWVkYmFjayBxdWlj a2x5IGFuZCBjbGVhbmx5IGF0IHRoaXMgdmVyeSBlYXJseSBzdGFnZSBvZiAKPiBkZXZlbG9wbWVu dD8KPgo+IElmIG5vdCwgd2Ugd2lsbCBpbW1lZGlhdGVseSBzdG9wIHRvIG1vdmUgaW4gdGhpcyBk aXJlY3Rpb24gYW5kIHRoaXMgaXMgdGhlIAo+IGZpcnN0IGFuZCBsYXN0IGVtYWlsIHRoYXQgeW91 IGhhdmUgc2VlbiBvZiB0aGlzIGtpbmQsIGFuZCB3ZSB3aWxsIGhhdmUgdG8gCj4gY29tZSB1cCB3 aXRoIGJldHRlci9vdGhlciBpZGVhcyBhcm91bmQgd29yayBvbiB0aGUgTGludXgga2VybmVsLgo+ Cj4gSWYgc28sIHdlIHdpbGwgY29udGludWUgaW4gdGhlIGRpcmVjdGlvbiBza2V0Y2hlZCBhYm92 ZSwgYW5kIEkgdGhpbmsgSSAKPiBqdXN0IGhhdmUgdG8gcG9pbnQgb3V0IGFuZCBhcG9sb2dpemUg Zm9yIHRoZSB0d28gb2J2aW91cyB0aGluZ3MgdGhhdCB3ZSAKPiBkaWQgd3JvbmcgaW4gdGhpcyBz cGVjaWZpYyBjYXNlIGhlcmU6Cj4KPiAgIC0gV2Ugbm90aWNlZCB0aGF0IHRoZXJlIHdlcmUgZnVy dGhlciBjaGFuZ2VzIGluIGxpbnV4LW5leHQsIGJ1dCB3ZQo+ICAgICB0aG91Z2h0IHRoYXQgb3Vy IGludmVzdGlnYXRpb24gb24gdjQuMTUgd2FzIHZhbHVhYmxlIG5ldmVydGhlbGVzcwo+ICAgICBm b3IgdGhlIGRldmVsb3BlcnMgdGhhdCBoYWQgdG91Y2hlZCB0aGUgc291cmNlIGNvZGUgdGhhdCB3 ZSBsb29rZWQgYXQsCj4gICAgIGFsdGhvdWdoLCB0aGVyZSBpcyBub3RoaW5nIHRvIGJlIGRvbmUg aWYgY29tbWl0cyBmcm9tIGxpbnV4LW5leHQgYXJlCj4gICAgIG1lcmdlZCBpbnRvIExpbnVzJyB0 cmVlIHNvb24uIFRha2luZyB5b3VyIHJlc3BvbnNlLCB3ZSBoYXZlIGNsZWFybHkgCj4gICAgIGJl ZW4gV1JPTkcgaGVyZSwgb3ZlcmVzdGltYXRpbmcgb3VyIGNvbnRyaWJ1dGlvbiB2ZXJzdXMgdGhl IG5vaXNlIAo+ICAgICByYXRpbyB0aGF0IHdlIGNvbnRyaWJ1dGUgdG8uCj4gICAKPiAgIC0gV2Ug bG9va2VkIGF0IGEgY2xhbmcgd2FybmluZywgZm9yIHdoaWNoIHdlIGNvdWxkIG9ubHkgcHJvdmlk ZSB0aGUgCj4gICAgIGluZm9ybWF0aW9uIG9uIHRoaXMgY2xhbmcgd2FybmluZyBhdCB0aGlzIHZl cnkgbGF0ZSBzdGFnZSwgaS5lLiwgd2hlbgo+ICAgICB0aGUgY29tbWl0IHVuZGVyIGludmVzdGln YXRpb24gaGFzIGFscmVhZHkgYmVlbiBtZXJnZWQgYW5kIHRoZSBrZXJuZWwgCj4gICAgIHdhcyBy ZWxlYXNlZC4gU28gcG9pbnRpbmcgb3V0IHNob3J0Y29taW5nIG9mIHRoYXQga2luZCBzZWVtcyB0 byBoYXZlIAo+ICAgICBubyB2YWx1ZSwgYXMgeW91LCBHcmVnLCB3b3VsZCBub3QgYmFja3BvcnQg Y29tbWl0cyB0byBzdGFibGUgYW55d2F5Lgo+Cj4gVGhpcyBoYXMgYmVlbiBib3RoIGVycm9ycyBv biBteSBzaWRlIGFzIGEgbWVudG9yLiBBZnRlciBteSBzdHVkZW50Cj4gaGFzIHN0YXJ0ZWQgdGhp cyB3ZWVrIGFuZCBoYXMgd29ya2VkIGhhcmQgZm9yIGEgd2VlayBsZWFybmluZyBhIGxvdCBhYm91 dCAKPiBMaW51eCBrZXJuZWwgZGV2ZWxvcG1lbnQgYW5kIGFsbCB0aGUgdG9vbHMgYXJvdW5kIGl0 LCBJIGRpZCBub3Qgd2FudCB0byAKPiBkaXNjb3VyYWdlIGhpbSBhbmQgc2F5IHRoYXQgdGhlIGdv YWwgc2V0IGF0IHRoZSBiZWdpbm5pbmcgb2YgdGhlIHdlZWsgdG8gCj4gaWRlbnRpZnkgYW5kIHJl cG9ydCBhIGNvZGUgc21lbGwgb24gb25lIGNvbW1pdCBvbiB0aGUgbWFpbGluZyBsaXN0IGhhcyAK PiBub3QgYmVlbiBhY2hpZXZlZCBhcyBmb3IgdGhlIHJlYXNvbnMgYWJvdmUuIEluc3RlYWQsIHdl IGRlY2lkZWQgdG8gc2VuZCBpdCAKPiBvdXQgYW5kIHdlcmUgaW50ZXJlc3RlZCBpbiB0aGUgZ2Vu ZXJhbCByZWNlcHRpb24gb2Ygb3VyIHdvcmsgb2YgdGhpcyBmaXJzdCAKPiB3ZWVrLgo+Cj4gSSBh cG9sb2dpemUgZm9yIHRoYXQgYW5kIGhvcGUgd2UgY2FuIGxlYXZlIHRoZSBzcGVjaWZpYyByZXBv cnRlZCBpc3N1ZSBub3cgCj4ganVzdCByZXN0IGluIHBlYWNlLgo+Cj4gVGhpcyBleHBlcmltZW50 IHNob3dzIHRoYXQgSSBzdGlsbCBuZWVkIHRvIGltcHJvdmUgbXkgdW5kZXJzdGFuZGluZyBob3cg Cj4gdG8gY29udHJpYnV0ZSBwcm9wZXJseSB0byB0aGUga2VybmVsIGRldmVsb3BtZW50LiBBdCBs ZWFzdCB0byBtZSwgdGhlICAKPiBwb2xpY3kgb24gY2xhbmcgd2FybmluZ3Mgd2FzIG5vdCBjbGVh cjsgYW5kIEkgaGF2ZSBsZWFybmVkIHRoaXMgbm93IGluIAo+IHRoaXMgbW9yZSBpbmRpcmVjdCB3 YXkuCj4KPiBXZSBvbmx5IHNlbnQgb3V0IHRoaXMgb25lIGVtYWlsIHRvIHNlZSBpZiBjbGFuZyB3 YXJuaW5ncyBhcmUgb2YgaW50ZXJlc3QgCj4gYXQgYWxsLCBhbmQgd2UgYXJlIGdsYWQgdGhhdCB5 b3UgY2FtZSBiYWNrIHRvIHVzIHNvIHF1aWNrbHkgd2l0aCBmZWVkYmFjay4KPgo+IEdyZWcsIGlm IHlvdSBjYW4gY29udGludWUgdG8gYmUgYSBzcGFyaW5nIHBhcnRuZXIgYW5kIHBvaW50IG91dCB3 aGVuIAo+IHdlIGFyZSBtb3ZpbmcgaW4gdGhlIHdyb25nIGRpcmVjdGlvbiwgd2Ugd2lsbCB0cnkg b3VyIGJlc3QgdG8gdW5kZXJzdGFuZAo+IGhvdyB3ZSBjYW4gY29udHJpYnV0ZSB0byB0dXJuIHJl c3VsdHMgZnJvbSBidWcgZmluZGVycyBhbmQgc3RhdGljIGFuYWx5c2lzIAo+IHRvb2xzIHdpdGgg dGhlIG1hbnVhbCBwcmUtYXNzZXNzbWVudCB3ZSBjYW4gZG8gaW50byB2YWx1YWJsZSBjb250cmli dXRpb25zIAo+IG9uIHRoZSBtYWlsaW5nIGxpc3QgYW5kIHRoZSBsaW51eCBrZXJuZWwgZGV2ZWxv cG1lbnQuCj4KPiBXZSBjZXJ0YWlubHkgZG8gbm90IGludGVuZCB0byBzcGFtIHRoZSBtYWlsaW5n IGxpc3Qgd2l0aCByZXBvcnRzIG9mIAo+IGZpbmRpbmdzIG5vYm9keSBjYXJlcyBhYm91dC4KCkJl aW5nIGJydXRhbGx5IGhvbmVzdCwgcGxlYXNlIHdyaXRlIHNob3J0ZXIgcmVwb3J0cyBhbmQgc2hv cnRlciBlbWFpbHMKdG8gdGhlIGxpc3RzLgoKVGhlIHN0YXRpYyBhbmFseXNpcyByZXBvcnRzIGFy ZSB3ZWxjb21lLCBidXQgb25seSB3aGVuIDEpIHdlIGRpZG4ndAphbHJlYWR5IGZpeCBpdCBpbiBs aW51eC1uZXh0LCBvciAyKSBpdCByZXZlYWxzIGFuIGFjdHVhbCBidWcsIG5vdCBqdXN0IGEKd2Fy bmluZywgd2FycmFudGluZyBhIGJhY2twb3J0LgoKVGhhbmtzLApKYW5pLgoKCi0tIApKYW5pIE5p a3VsYSwgSW50ZWwgT3BlbiBTb3VyY2UgVGVjaG5vbG9neSBDZW50ZXIKX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJ bnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Au b3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Cyrus-Session-Id: sloti22d1t05-1938260-1517560595-2-16728580469737044162 X-Sieve: CMU Sieve 3.0 X-Spam-known-sender: no X-Spam-score: 0.0 X-Spam-hits: BAYES_00 -1.9, RCVD_IN_DNSWL_HI -5, SPF_PASS -0.001, T_RP_MATCHES_RCVD -0.01, LANGUAGES en, BAYES_USED global, SA_VERSION 3.4.0 X-Spam-source: IP='134.134.136.65', Host='mga03.intel.com', Country='US', FromHeader='com', MailFrom='com' X-Spam-charsets: cc='utf-8', plain='utf-8' X-Resolved-to: greg@kroah.com X-Delivered-to: greg@kroah.com X-Mail-from: jani.nikula@intel.com ARC-Seal: i=1; a=rsa-sha256; cv=none; d=messagingengine.com; s=arctest; t=1517560594; b=HK+QW9v+XruzqYFdZyhf01I0WLE8sAHcglLN45ghuyMh+By 1ie3Gurjh/WWC0DHwZVQcfWN6yKr5ZC+iJP3duZtz9fFN8ES+jJxRk1TmzcS4hix pWwdUyiNRuQNPletEEDcQNN1j8ZJWLpV8UN1C540SoGUmmamyMGqONQQYcmuRKvD xflLXZrp0rQ9jIcuYlrPK3HFxeLzdCI9YwNQCeccGIS6gi/3QmF7zk9kN0RLQplq kGXQX8d3nJHTQLip8v7w56Cm5otdVPFOT8pvYWIsmDt1/E01pUArpsf/RIcrlPYL QT0fUS9KtNM4xwx0ngQN6lTUCFybF9+u/cYB8Hw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=from:to:cc:subject:in-reply-to:references :date:message-id:mime-version:content-type :content-transfer-encoding; s=arctest; t=1517560594; bh=ROGHrMbs ymPgF6FK6fwET16sTtj10IDbBYcaj+qsVCo=; b=ckYPG5BWOCrOChs5sLUapvDn thDDar3Dk85l0919aBO0oZcamaLzOgGwnsjTbz+zGYzZqiOMzUu3Z2nicGHMBwMI YvT6l1lnz8dqDmgUHtURCyqdlto79RH76GOXyWGys+2LX2Plol0C6Tk3V4nTB/w5 tbP3kr9h2gcpwrLJpL43f0MgWGx6Rdq3wh6K0BGgpcFJPpYJ3laJllevRPug+jMT /lZgJgvN4QBcfmVfXe5+wMbEmXQSUXKg+3EF7YVyVE2TFZGMxEHRW5hHbY496sjx OiV82rGysrrxt0qroMSeP0yFYodDJRbJjLhhngvuHySppAvGFQOkbCF1ks+ryQ== ARC-Authentication-Results: i=1; mx1.messagingengine.com; arc=none (no signatures found); dkim=none (no signatures found); dmarc=none (p=none,d=none) header.from=linux.intel.com; iprev=pass policy.iprev=134.134.136.65 (mga03.intel.com); spf=pass smtp.mailfrom=jani.nikula@intel.com smtp.helo=mga03.intel.com; x-aligned-from=orgdomain_pass; x-ptr=pass x-ptr-helo=mga03.intel.com x-ptr-lookup=mga03.intel.com; x-return-mx=pass smtp.domain=intel.com smtp.result=pass smtp_is_org_domain=yes header.domain=linux.intel.com header.result=pass header_org.domain=intel.com header_org.result=pass header_is_org_domain=no; x-tls=pass version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128 Authentication-Results: mx1.messagingengine.com; arc=none (no signatures found); dkim=none (no signatures found); dmarc=none (p=none,d=none) header.from=linux.intel.com; iprev=pass policy.iprev=134.134.136.65 (mga03.intel.com); spf=pass smtp.mailfrom=jani.nikula@intel.com smtp.helo=mga03.intel.com; x-aligned-from=orgdomain_pass; x-ptr=pass x-ptr-helo=mga03.intel.com x-ptr-lookup=mga03.intel.com; x-return-mx=pass smtp.domain=intel.com smtp.result=pass smtp_is_org_domain=yes header.domain=linux.intel.com header.result=pass header_org.domain=intel.com header_org.result=pass header_is_org_domain=no; x-tls=pass version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,447,1511856000"; d="scan'208";a="27488440" From: Jani Nikula To: Lukas Bulwahn , Greg KH Cc: Ozan Alpay , Rodrigo Vivi , Ville =?utf-8?B?U3lyasOkbMOk?= , sil2review@lists.osadl.org, kernelnewbies@kernelnewbies.org, David Airlie , intel-gfx@lists.freedesktop.org, Joonas Lahtinen , llvmlinux@lists.linuxfoundation.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, lukas.bulwahn@gmail.com Subject: Re: clang warning: implicit conversion in intel_ddi.c:1481 In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20180201180240.GA28042@kroah.com> Date: Fri, 02 Feb 2018 10:36:18 +0200 Message-ID: <87372jkcu5.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-getmail-retrieved-from-mailbox: INBOX X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Thu, 01 Feb 2018, Lukas Bulwahn wrote: > Hi Greg, > > On Thu, 1 Feb 2018, Greg KH wrote: > >> On Thu, Feb 01, 2018 at 06:33:30PM +0100, Ozan Alpay wrote: >> > Dear Rodrigo Vivi, Ville Syrj=C3=A4l=C3=A4, >> >=20 >> > My name is Ozan Alpay, and I am a student mentored by Lukas Bulwahn. W= e=20 >> > intend to use static analysis tools on the kernel source to identify,= =20 >> > analyze and report issues. As a very first step, we are looking into=20 >> > clang compiler warnings and will then move to more sophisticated tools= .=20 >> > >> > [...] >> > >> > Linux 4.15 is shipped with this clang warning, but we don't see the=20 >> > crucial need to provide a backport commit to the stable branch for 4.1= 5.=20 >> > We just wanted to inform you about our analysis of this clang warning.= =20 >> > Ultimately the final call if you would like to address this clang warn= ing=20 >> > in 4.15 is yours. >>=20 >> Note, I have not taken "clang warning fixes" for stable kernel updates >> in the past, and I doubt I will in the future, unless the tree "builds >> clean" with clang. If it eventually gets there, then yes, I will do >> that. >>=20 >> Note, if you are going to email this out to everyone who fixes a warning >> message, you might want to reconsider it. That's going to be a lot of >> work, and for people who have already fixed an issue, it's kind of >> pointless to just remind them of work they have done in the past, right? >>=20 >> What is the goal of these types of emails? >>=20 > > We are interested in providing useful information on potential bugs or bu= g=20 > patterns that we get from static analysis tools, after we pre-assess them= =20 > and manually select them to send to the review process of the patch=20 > submission. > > For me, the clang warnings were an easy starting point for a student to=20 > set up and look at, compared to much more sophisticated tools, such as=20 > coccinelle, sparse or new tools for the kernel development, such as CMBC= =20 > or facebook's Infer. > > Once we really understand which tools are useful and which information ca= n=20 > be quickly pre-assessed and sent out in a useful way without just creatin= g=20 > more noise in the discussion, I would have contacted the 0-day=20 > infrastructure team or the kernelci.org team to continue the discussion=20 > how to include our first setup into a proper semi-automated service. > > Using the clang warnings, I wanted to explore how this would even=20 > potentially work. > > Considering clang, of course, currently, we cannot compile the whole=20 > kernel with all possible kernel configurations with clang, but I believe= =20 > Nick Desaulniers, Matthias Kaehlcke and others are already working on=20 > that and are getting close to this goal. Hence, assuming they will be=20 > successful soon, I wanted to explore the next step of using static=20 > analysis tools around the clang/LLVM compiler. > > Actually, v4.15 builds almost "cleanly" with clang: For defconfig, there= =20 > are only two clang compiler warnings and the one that we looked into=20 > deeper here is already resolved in linux-next, so chances are actually=20 > high that we might get to this "builds clean" soon-ish, at least for=20 > defconfig. > > Concerning clang warnings and how to progress towards that goal of=20 > building cleanly, my strategy is to identify when new clang compiler > warnings are introduced and just point these warnings out as code smells= =20 > during the review discussion before they are merged into the=20 > first maintainer tree. If we manually inspect these clang warnings > to make sure that they are genuine code smells of some "imprecise=20 > implementation" before we send them to the mailing list, I would hope tha= t=20 > they are of some value for the developer in the submission process and=20 > he/she could address the warning in a patch v2 while he/she is reacting t= o=20 > the other review comments from the human reviewers. > > At best, I always considered them as useful information during the review= =20 > process, but I certainly DO NOT want to start patching the kernel due to= =20 > clang warnings. The chances/risk that we just break more due to naively=20 > fixing warnings without proper understanding is much higher than the=20=20 > benefit of actually improving the situation. If I recall correctly, I=20 > think this is also one of the lessons learned from motivating newcomers=20 > to address warnings in previous kernel newbies activities. > > Greg, do you think it is worthwhile to invest some effort to move=20 > towards the goal "kernel builds cleanly with clang"? > Would you agree that providing information during the patch review is a=20 > good way to move forward to this goal if we find a suitable manner to=20 > provide this feedback quickly and cleanly at this very early stage of=20 > development? > > If not, we will immediately stop to move in this direction and this is th= e=20 > first and last email that you have seen of this kind, and we will have to= =20 > come up with better/other ideas around work on the Linux kernel. > > If so, we will continue in the direction sketched above, and I think I=20 > just have to point out and apologize for the two obvious things that we=20 > did wrong in this specific case here: > > - We noticed that there were further changes in linux-next, but we > thought that our investigation on v4.15 was valuable nevertheless > for the developers that had touched the source code that we looked at, > although, there is nothing to be done if commits from linux-next are > merged into Linus' tree soon. Taking your response, we have clearly=20 > been WRONG here, overestimating our contribution versus the noise=20 > ratio that we contribute to. >=20=20=20 > - We looked at a clang warning, for which we could only provide the=20 > information on this clang warning at this very late stage, i.e., when > the commit under investigation has already been merged and the kernel= =20 > was released. So pointing out shortcoming of that kind seems to have= =20 > no value, as you, Greg, would not backport commits to stable anyway. > > This has been both errors on my side as a mentor. After my student > has started this week and has worked hard for a week learning a lot about= =20 > Linux kernel development and all the tools around it, I did not want to=20 > discourage him and say that the goal set at the beginning of the week to= =20 > identify and report a code smell on one commit on the mailing list has=20 > not been achieved as for the reasons above. Instead, we decided to send i= t=20 > out and were interested in the general reception of our work of this firs= t=20 > week. > > I apologize for that and hope we can leave the specific reported issue no= w=20 > just rest in peace. > > This experiment shows that I still need to improve my understanding how=20 > to contribute properly to the kernel development. At least to me, the=20= =20 > policy on clang warnings was not clear; and I have learned this now in=20 > this more indirect way. > > We only sent out this one email to see if clang warnings are of interest= =20 > at all, and we are glad that you came back to us so quickly with feedback. > > Greg, if you can continue to be a sparing partner and point out when=20 > we are moving in the wrong direction, we will try our best to understand > how we can contribute to turn results from bug finders and static analysi= s=20 > tools with the manual pre-assessment we can do into valuable contribution= s=20 > on the mailing list and the linux kernel development. > > We certainly do not intend to spam the mailing list with reports of=20 > findings nobody cares about. Being brutally honest, please write shorter reports and shorter emails to the lists. The static analysis reports are welcome, but only when 1) we didn't already fix it in linux-next, or 2) it reveals an actual bug, not just a warning, warranting a backport. Thanks, Jani. --=20 Jani Nikula, Intel Open Source Technology Center