[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20071031201952.GA17235@Krystal>
Date: Wed, 31 Oct 2007 16:19:52 -0400
From: Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
To: Dave Hansen <haveblue@...ibm.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Christoph Hellwig <hch@....de>
Subject: Re: fix marker warnings
* Dave Hansen (haveblue@...ibm.com) wrote:
> On Tue, 2007-10-30 at 22:08 -0400, Mathieu Desnoyers wrote:
> > * Dave Hansen (haveblue@...ibm.com) wrote:
> > > I'm seeing these in the latest git:
> > >
> > > kernel/marker.c: In function `marker_probe_unregister':
> > > kernel/marker.c:355: warning: `probe_module' might be used uninitialized in this function
> > > kernel/marker.c: In function `marker_probe_unregister_private_data':
> > > kernel/marker.c:389: warning: `probe_module' might be used uninitialized in this function
> > > kernel/marker.c:392: warning: `entry' might be used uninitialized in this function
> > >
> > > It's due to gcc not detecting that the need_update condition is actually
> > > constant, and will never call marker_update_probes() on an uninitialized
> > > probe_module.
> > >
> > > However, that need_update bit is all due to dropping the mutex before
> > > calling marker_update_probes(). As far as I can tell, every call to
> > > marker_update_probes() has this lock dropping behavior just before
> > > calling it. So, let's just hold the locks over the
> > > marker_update_probes() and document that it needs to have a lock taken
> > > instead.
> > >
> > > This removes code overall. Untested except for a quick compile.
> > > Consider it just a style suggestion. :)
> > >
> >
> > Ok, just ran it and it seems good. It did not appear as trivial during
> > development because locking was is a different order until recently.
> >
> > I wonder what gcc version you are using though, because mine does not
> > warn about anything. I wonder if it is really necessary to "fix" false
> > gcc warnings like this. Let's take it as a cleanup.
>
> dave@...nel:~$ gcc -v
> Reading specs from /usr/lib/gcc-lib/x86_64-linux-gnu/3.3.6/specs
> Configured with: ../src/configure -v --enable-languages=c,c++
> --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info
> --with-gxx-include-dir=/usr/include/c++/3.3 --enable-shared
> --enable-__cxa_atexit --with-system-zlib --enable-nls
> --without-included-gettext --enable-clocale=gnu --enable-debug
> --disable-multilib x86_64-linux-gnu
> Thread model: posix
> gcc version 3.3.6 (Ubuntu 1:3.3.6-15ubuntu1)
>
>
> > Acked-by: Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
>
> Do you have any other patches with which you would like to forward this
> one? Or, shall I send it upstream on its own?
>
For the marker infrastructure itself, that's the only one. I think
sending it to Andrew Morton is the correct approach.
Mathieu
> -- Dave
>
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists