linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nathan Chancellor <natechancellor@gmail.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Nick Desaulniers <ndesaulniers@google.com>,
	jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com,
	rodrigo.vivi@intel.com, intel-gfx@lists.freedesktop.org,
	dri-devel@lists.freedesktop.org,
	LKML <linux-kernel@vger.kernel.org>,
	Matthias Kaehlcke <mka@chromium.org>
Subject: Re: [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o
Date: Tue, 18 Dec 2018 12:01:43 -0700	[thread overview]
Message-ID: <20181218190143.GA7036@flashbox> (raw)
In-Reply-To: <154513398652.1108.7150969916024071452@skylake-alporthouse-com>

On Tue, Dec 18, 2018 at 11:53:06AM +0000, Chris Wilson wrote:
> Quoting Nick Desaulniers (2018-10-25 23:20:58)
> > On Thu, Oct 25, 2018 at 12:36 PM Nathan Chancellor
> > <natechancellor@gmail.com> wrote:
> > >
> > > This warning is disabled by default in scripts/Makefile.extrawarn when
> > > W= is not provided but this Makefile adds -Wall after this warning is
> > > disabled so it shows up in the build when it shouldn't:
> > >
> > > In file included from drivers/gpu/drm/i915/intel_breadcrumbs.c:895:
> > > drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c:350:34: error:
> > > variable 'wq' is uninitialized when used within its own initialization
> > > [-Werror,-Wuninitialized]
> > >         DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
> > >                                         ^~
> > > ./include/linux/wait.h:74:63: note: expanded from macro
> > > 'DECLARE_WAIT_QUEUE_HEAD_ONSTACK'
> > >         struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
> > >                                ~~~~                                  ^~~~
> > > ./include/linux/wait.h:72:33: note: expanded from macro
> > > '__WAIT_QUEUE_HEAD_INIT_ONSTACK'
> > >         ({ init_waitqueue_head(&name); name; })
> > >                                        ^~~~
> > > 1 error generated.
> > >
> > > This warning looks to be a false positive given that init_waitqueue_head
> > > initializes name before it is used. Rather than disable the warning for
> > > the full folder like commit 46e2068081e9 ("drm/i915: Disable some extra
> > 
> > cc author/reviewer of 46e2068081e9.
> > 
> > I'm fine with the patch as is, unless others prefer to disable it for
> > the whole subdir?  We could be playing whack-a-mole in the future
> > disabling this warning for other translation units.
> 

Hi Chris,

> Yes, exactly this since the warning is generated by a core header and a
> fairly common pattern its use is not restricted to any single file.
> (Will not all selftests similarly explode?)
> 

Well, -Wuninitialized is turned off for the whole kernel unless W= is
passed. So I suppose it should be turned back on for the whole folder
but I noticed that the i915 Makefile purposefully turns all of the
disabled warnings back on for heavier coverage so it makes some sense to
just leave it off for one translation unit when it's just one
translation unit that has the problem. That said, I'm more than happy to
send a v2 turning it off for the whole folder if you think that best.

> The other false-positive clang-6 gave was for local_clock_us().
> Presumably that one is fixed?
> -Chris

With this patch, I can build i915 using defconfig and allyesconfig
without any warnings with tip-of-tree Clang.

Thank you for the comments!
Nathan

  reply	other threads:[~2018-12-18 19:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-25 19:36 Nathan Chancellor
2018-10-25 22:20 ` Nick Desaulniers
2018-12-18 11:53   ` Chris Wilson
2018-12-18 19:01     ` Nathan Chancellor [this message]
2018-12-18 19:43       ` Nick Desaulniers
2018-12-17 17:41 ` Nathan Chancellor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181218190143.GA7036@flashbox \
    --to=natechancellor@gmail.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mka@chromium.org \
    --cc=ndesaulniers@google.com \
    --cc=rodrigo.vivi@intel.com \
    --subject='Re: [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).