From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915: mark expected switch fall-through Date: Wed, 27 Jun 2018 12:30:49 +0300 Message-ID: <87tvpotw3a.fsf@intel.com> References: <20180620133100.GA608@embeddedor.com> <20180620190628.GC8258@intel.com> <26740470-d7b7-0a58-0aae-fc628bd4f416@embeddedor.com> <87r2l0va65.fsf@intel.com> <357b53aa-d8ce-c9db-0b81-2e8e1aa821bb@embeddedor.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <357b53aa-d8ce-c9db-0b81-2e8e1aa821bb@embeddedor.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "Gustavo A. R. Silva" , Rodrigo Vivi Cc: David Airlie , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org T24gVHVlLCAyNiBKdW4gMjAxOCwgIkd1c3Rhdm8gQS4gUi4gU2lsdmEiIDxndXN0YXZvQGVtYmVk ZGVkb3IuY29tPiB3cm90ZToKPiBIaSBKYW5pLAo+Cj4gT24gMDYvMjEvMjAxOCAwMzowMyBBTSwg SmFuaSBOaWt1bGEgd3JvdGU6Cj4+IE9uIFdlZCwgMjAgSnVuIDIwMTgsICJHdXN0YXZvIEEuIFIu IFNpbHZhIiA8Z3VzdGF2b0BlbWJlZGRlZG9yLmNvbT4gd3JvdGU6Cj4+PiBPbiAwNi8yMC8yMDE4 IDAyOjA2IFBNLCBSb2RyaWdvIFZpdmkgd3JvdGU6Cj4+Pj4gT24gV2VkLCBKdW4gMjAsIDIwMTgg YXQgMDg6MzE6MDBBTSAtMDUwMCwgR3VzdGF2byBBLiBSLiBTaWx2YSB3cm90ZToKPj4+Pj4gSW4g cHJlcGFyYXRpb24gdG8gZW5hYmxpbmcgLVdpbXBsaWNpdC1mYWxsdGhyb3VnaCwgbWFyayBzd2l0 Y2ggY2FzZXMKPj4+Pj4gd2hlcmUgd2UgYXJlIGV4cGVjdGluZyB0byBmYWxsIHRocm91Z2guCj4+ Pj4+Cj4+Pj4+IEFkZHJlc3Nlcy1Db3Zlcml0eS1JRDogMTQ3MDEwMiAoIk1pc3NpbmcgYnJlYWsg aW4gc3dpdGNoIikKPj4+Pgo+Pj4+IEFueSBvdGhlciBhZHZhbnRhZ2UgYmVzaWRlcyBjb3Zlcml0 eT8KPj4+PiBDYW4ndCB3ZSBhZGRyZXNzIGl0IGJ5IG1hcmtpbmcgYXMgIkludGVudGlvbmFsIiBv biB0aGUgdG9vbD8KPj4+Pgo+Pj4KPj4+IFllcy4gVGhlIGFkdmFudGFnZSBvZiB0aGlzIGlzIHRo YXQgaXQgd2lsbCBldmVudHVhbGx5IGFsbG93cyB0byBlbmFibGUgCj4+PiAtV2ltcGxpY2l0LWZh bGx0aHJvdWdoLCBoZW5jZSwgZW5hYmxpbmcgdGhlIGNvbXBpbGVyIHRvIHRyaWdnZXIgYSAKPj4+ IHdhcm5pbmcsIHdoaWNoIHdpbGwgZm9yY2UgdXMgdG8gZG91YmxlIGNoZWNrIGlmIHdlIGFyZSBh Y3R1YWxseSBtaXNzaW5nIAo+Pj4gYSBicmVhayBiZWZvcmUgY29tbWl0dGluZyB0aGUgY29kZS4K Pj4gCj4+IEkgYXBwbGF1ZCB0aGUgZWZmb3J0cy4gU2luY2UgeW91J3JlIGRvaW5nIHRoZSBjb21t ZW50IGNoYW5nZXMsIGRvIHlvdQo+PiBoYXZlIGFuIGlkZWEgd2hhdCAtV2ltcGxpY2l0LWZhbGx0 aHJvdWdoPU4gbGV2ZWwgaXMgYmVpbmcgY29uc2lkZXJlZCBmb3IKPj4ga2VybmVsPwo+PiAKPgo+ IEN1cnJlbnRseSwgd2UgYXJlIHRyeWluZyBsZXZlbCAyLgo+Cj4+Pj4gSSdtIGFmcmFpZCB0aGVy ZSB3aWxsIGJlIHNvIG1hbnkgbW9yZSBwbGFjZXMgdG8gYWRkIGZhbGx0aHJvdWdoCj4+Pj4gbWFy a3MuLi4uCj4+Pj4KPj4+Cj4+PiBPaCB5ZWFoLCB0aGVyZSBhcmUgYXJvdW5kIDEwMDAgc2ltaWxh ciBwbGFjZXMgaW4gdGhlIHdob2xlIGNvZGViYXNlLiAKPj4+IFRoZXJlIGlzIGFuIG9uZ29pbmcg ZWZmb3J0IHRvIHJldmlldyBlYWNoIGNhc2UuIE1vbnRocyBhZ28sIGl0IHVzZWQgdG8gCj4+PiBi ZSBhcm91bmQgMTUwMCBvZiB0aGVzZSBjYXNlcy4KPj4gCj4+IFdlIHVzZSBvdXIgb3duIE1JU1NJ TkdfQ0FTRSgpIHRvIGluZGljYXRlIHN0dWZmIHRoYXQncyBub3Qgc3VwcG9zZWQgdG8KPj4gaGFw cGVuLCBvciB0byBiZSBpbXBsZW1lbnRlZCwgZXRjLiBhbmQgaW4gbWFueSBjYXNlcyB0aGUgZmFs bHRocm91Z2ggaXMKPj4gbm9ybWFsLiBJIHdvbmRlciBpZiB3ZSBjb3VsZCBlbWJlZCBfX2F0dHJp YnV0ZV9fICgoZmFsbHRocm91Z2gpKSBpbgo+PiB0aGVyZSB0byB0YWNrbGUgYWxsIG9mIHRoZXNl IHdpdGhvdXQgYSBjb21tZW50Lgo+PiAKPgo+IEkndmUgdHJpZWQgdGhpczoKPgo+IGRpZmYgLS1n aXQgYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X3V0aWxzLmggYi9kcml2ZXJzL2dwdS9kcm0v aTkxNS9pOTE1X3V0aWxzLmgKPiBpbmRleCAwMDE2NWFkLi44Mjk1NzJjIDEwMDY0NAo+IC0tLSBh L2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfdXRpbHMuaAo+ICsrKyBiL2RyaXZlcnMvZ3B1L2Ry bS9pOTE1L2k5MTVfdXRpbHMuaAo+IEBAIC00MCw4ICs0MCwxMCBAQAo+ICAjdW5kZWYgV0FSTl9P Tl9PTkNFCj4gICNkZWZpbmUgV0FSTl9PTl9PTkNFKHgpIFdBUk5fT05DRSgoeCksICIlcyIsICJX QVJOX09OX09OQ0UoIiBfX3N0cmluZ2lmeSh4KSAiKSIpCj4KPiAtI2RlZmluZSBNSVNTSU5HX0NB U0UoeCkgV0FSTigxLCAiTWlzc2luZyBjYXNlICglcyA9PSAlbGQpXG4iLCBcCj4gLSAgICAgICAg ICAgICAgICAgICAgICAgICAgICBfX3N0cmluZ2lmeSh4KSwgKGxvbmcpKHgpKQo+ICsjZGVmaW5l IE1JU1NJTkdfQ0FTRSh4KSAoeyBcCj4gKyAgICAgICBXQVJOKDEsICJNaXNzaW5nIGNhc2UgKCVz ID09ICVsZClcbiIsIF9fc3RyaW5naWZ5KHgpLCAobG9uZykoeCkpOyBcCj4gKyAgICAgICBfX2F0 dHJpYnV0ZV9fICgoZmFsbHRocm91Z2gpKTsgXAo+ICt9KQo+Cj4gICNpZiBHQ0NfVkVSU0lPTiA+ PSA3MDAwMAo+ICAjZGVmaW5lIGFkZF9vdmVyZmxvd3MoQSwgQikgXAo+Cj4gYW5kIEkgZ2V0IHRo ZSBmb2xsb3dpbmcgd2FybmluZ3MgYXMgYSBjb25zZXF1ZW5jZToKClJpZ2h0LiBUaGF0J3MgYmVj YXVzZSB3ZSd2ZSB1c2VkIE1JU1NJTkdfQ0FTRSgpIGFsc28gaW4gaWYtbGFkZGVycyBpbgphZGRp dGlvbiB0byB0aGUgc3dpdGNoIGRlZmF1bHQgY2FzZS4gRnJvbSBvdXIgUE9WIHRoZSB1c2FnZSBp cyBzaW1pbGFyLgoKKnNocnVnKgoKSSBndWVzcyBJIGxpa2UgLyogZmFsbCB0aHJvdWdoICovIGFu bm90YXRpb25zIG5leHQgdG8gTUlTU0lOR19DQVNFKCkKYmV0dGVyIHRoYW4gaGF2aW5nIHR3byBk aWZmZXJlbnQgbWFjcm9zIGRlcGVuZGluZyBvbiB3aGVyZSB0aGV5J3JlIGJlaW5nCnVzZWQuCgpU aGFua3MgZm9yIHRyeWluZyBpdCBvdXQgYW55d2F5LgoKQlIsCkphbmkuCgoKPgo+IGRyaXZlcnMv Z3B1L2RybS9pOTE1L2ludGVsX3BtLmM6IEluIGZ1bmN0aW9uIOKAmGludGVsX2luaXRfY2xvY2tf Z2F0aW5nX2hvb2tz4oCZOgo+IGRyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfdXRpbHMuaDo0ODoy OiBlcnJvcjogaW52YWxpZCB1c2Ugb2YgYXR0cmlidXRlIOKAmGZhbGx0aHJvdWdo4oCZCj4gICBf X2F0dHJpYnV0ZV9fICgoZmFsbHRocm91Z2gpKTsgXAo+ICAgXgo+IGRyaXZlcnMvZ3B1L2RybS9p OTE1L2ludGVsX3BtLmM6OTI0MDozOiBub3RlOiBpbiBleHBhbnNpb24gb2YgbWFjcm8g4oCYTUlT U0lOR19DQVNF4oCZCj4gICAgTUlTU0lOR19DQVNFKElOVEVMX0RFVklEKGRldl9wcml2KSk7Cj4g ICAgXn5+fn5+fn5+fn5+Cj4gZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfcG0uYzogSW4gZnVu Y3Rpb24g4oCYaW50ZWxfcmVhZF93bV9sYXRlbmN54oCZOgo+IGRyaXZlcnMvZ3B1L2RybS9pOTE1 L2k5MTVfdXRpbHMuaDo0ODoyOiBlcnJvcjogaW52YWxpZCB1c2Ugb2YgYXR0cmlidXRlIOKAmGZh bGx0aHJvdWdo4oCZCj4gICBfX2F0dHJpYnV0ZV9fICgoZmFsbHRocm91Z2gpKTsgXAo+ICAgXgo+ IGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX3BtLmM6MjkwMjozOiBub3RlOiBpbiBleHBhbnNp b24gb2YgbWFjcm8g4oCYTUlTU0lOR19DQVNF4oCZCj4gICAgTUlTU0lOR19DQVNFKElOVEVMX0RF VklEKGRldl9wcml2KSk7Cj4gICAgXn5+fn5+fn5+fn5+Cj4KPiBUaGFua3MKPiAtLQo+IEd1c3Rh dm8KCi0tIApKYW5pIE5pa3VsYSwgSW50ZWwgT3BlbiBTb3VyY2UgR3JhcGhpY3MgQ2VudGVyCl9f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCkludGVsLWdmeCBt YWlsaW5nIGxpc3QKSW50ZWwtZ2Z4QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3Rz LmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2ludGVsLWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4256FC43142 for ; Wed, 27 Jun 2018 09:31:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id EF62726461 for ; Wed, 27 Jun 2018 09:31:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EF62726461 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933603AbeF0JbO convert rfc822-to-8bit (ORCPT ); Wed, 27 Jun 2018 05:31:14 -0400 Received: from mga02.intel.com ([134.134.136.20]:50080 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932193AbeF0JbM (ORCPT ); Wed, 27 Jun 2018 05:31:12 -0400 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Jun 2018 02:31:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,278,1526367600"; d="scan'208";a="67688398" Received: from jnikula-mobl2.fi.intel.com (HELO localhost) ([10.237.72.62]) by fmsmga001.fm.intel.com with ESMTP; 27 Jun 2018 02:31:01 -0700 From: Jani Nikula To: "Gustavo A. R. Silva" , Rodrigo Vivi Cc: Joonas Lahtinen , David Airlie , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: mark expected switch fall-through In-Reply-To: <357b53aa-d8ce-c9db-0b81-2e8e1aa821bb@embeddedor.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20180620133100.GA608@embeddedor.com> <20180620190628.GC8258@intel.com> <26740470-d7b7-0a58-0aae-fc628bd4f416@embeddedor.com> <87r2l0va65.fsf@intel.com> <357b53aa-d8ce-c9db-0b81-2e8e1aa821bb@embeddedor.com> Date: Wed, 27 Jun 2018 12:30:49 +0300 Message-ID: <87tvpotw3a.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 26 Jun 2018, "Gustavo A. R. Silva" wrote: > Hi Jani, > > On 06/21/2018 03:03 AM, Jani Nikula wrote: >> On Wed, 20 Jun 2018, "Gustavo A. R. Silva" wrote: >>> On 06/20/2018 02:06 PM, Rodrigo Vivi wrote: >>>> On Wed, Jun 20, 2018 at 08:31:00AM -0500, Gustavo A. R. Silva wrote: >>>>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases >>>>> where we are expecting to fall through. >>>>> >>>>> Addresses-Coverity-ID: 1470102 ("Missing break in switch") >>>> >>>> Any other advantage besides coverity? >>>> Can't we address it by marking as "Intentional" on the tool? >>>> >>> >>> Yes. The advantage of this is that it will eventually allows to enable >>> -Wimplicit-fallthrough, hence, enabling the compiler to trigger a >>> warning, which will force us to double check if we are actually missing >>> a break before committing the code. >> >> I applaud the efforts. Since you're doing the comment changes, do you >> have an idea what -Wimplicit-fallthrough=N level is being considered for >> kernel? >> > > Currently, we are trying level 2. > >>>> I'm afraid there will be so many more places to add fallthrough >>>> marks.... >>>> >>> >>> Oh yeah, there are around 1000 similar places in the whole codebase. >>> There is an ongoing effort to review each case. Months ago, it used to >>> be around 1500 of these cases. >> >> We use our own MISSING_CASE() to indicate stuff that's not supposed to >> happen, or to be implemented, etc. and in many cases the fallthrough is >> normal. I wonder if we could embed __attribute__ ((fallthrough)) in >> there to tackle all of these without a comment. >> > > I've tried this: > > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h > index 00165ad..829572c 100644 > --- a/drivers/gpu/drm/i915/i915_utils.h > +++ b/drivers/gpu/drm/i915/i915_utils.h > @@ -40,8 +40,10 @@ > #undef WARN_ON_ONCE > #define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")") > > -#define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \ > - __stringify(x), (long)(x)) > +#define MISSING_CASE(x) ({ \ > + WARN(1, "Missing case (%s == %ld)\n", __stringify(x), (long)(x)); \ > + __attribute__ ((fallthrough)); \ > +}) > > #if GCC_VERSION >= 70000 > #define add_overflows(A, B) \ > > and I get the following warnings as a consequence: Right. That's because we've used MISSING_CASE() also in if-ladders in addition to the switch default case. From our POV the usage is similar. *shrug* I guess I like /* fall through */ annotations next to MISSING_CASE() better than having two different macros depending on where they're being used. Thanks for trying it out anyway. BR, Jani. > > drivers/gpu/drm/i915/intel_pm.c: In function ‘intel_init_clock_gating_hooks’: > drivers/gpu/drm/i915/i915_utils.h:48:2: error: invalid use of attribute ‘fallthrough’ > __attribute__ ((fallthrough)); \ > ^ > drivers/gpu/drm/i915/intel_pm.c:9240:3: note: in expansion of macro ‘MISSING_CASE’ > MISSING_CASE(INTEL_DEVID(dev_priv)); > ^~~~~~~~~~~~ > drivers/gpu/drm/i915/intel_pm.c: In function ‘intel_read_wm_latency’: > drivers/gpu/drm/i915/i915_utils.h:48:2: error: invalid use of attribute ‘fallthrough’ > __attribute__ ((fallthrough)); \ > ^ > drivers/gpu/drm/i915/intel_pm.c:2902:3: note: in expansion of macro ‘MISSING_CASE’ > MISSING_CASE(INTEL_DEVID(dev_priv)); > ^~~~~~~~~~~~ > > Thanks > -- > Gustavo -- Jani Nikula, Intel Open Source Graphics Center