Re: Modifying GlusterFS to cope with C99 inline semantics

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On Mon, Jul 27, 2015 at 08:28:48AM -0400, Kaleb KEITHLEY wrote:
> > 
> > My opinion: don't try to second guess the compiler/optimizer; stop using
> > inline. Full Stop.
> 
> Indeed, I fully agree.

+1

> 
> Niels
> 
> > 
> > On 07/27/2015 01:56 AM, Anoop C S wrote:
> > >Hi all,
> > >
> > >Due to the recent changes with GCC v5 [default standard is now GNU11],
> > >we have been reported with some issues regarding undefined symbol
> > >errors from different components. One important change while porting to
> > >GCC v5 is with respect to inline functions [see GCC 5 porting changes
> > >@https://gcc.gnu.org/gcc-5/porting_to.html] These errors were basically
> > >root caused to the wrong usage of inline functions when compiled against
> > >GCC v5, which now follows the C99 inline semantics by default [previously
> > >it was GNU89 inline semantics]. Most of them were fixed at the earliest
> > >so that we don't see such errors in our normal work flow. Latest one was
> > >failure to start the volume when glusterfs was compiled in debug mode.
> > >This was not seen when compiled in non-debug mode due to the presence of
> > >a particular compiler optimization level provided via configure.ac [see
> > >detailed discussion @
> > >http://www.gluster.org/pipermail/gluster-devel/2015-July/046211.html].
> > >
> > >Inline functions which doesn't require external usage.
> > >------------------------------------------------------
> > >Looking through the code base, we can see considerable instances for
> > >inline functions. According to C99 inline rules [
> > >http://www.greenend.org.uk/rjk/tech/inline.html], if we wanted to keep
> > >our inline function entirely private to one translation unit, we make
> > >it static inline. As of now there are instances where we have failed to
> > >do so. It is recommended to have those functions as static inline if we
> > >really require those functions to be 'inlined' and can be easily done.
> > >Or else we can just remove the inline keyword as a whole.
> > >
> > >Inline functions which require external usage.
> > >----------------------------------------------
> > >On the other side, we have inline functions defined to be used
> > >externally. The current implementation in glusterfs doesn't follow the
> > >proper inline semantics for such functions. An inline function must be
> > >defined in all files that calls it. According to C99 rules [
> > >http://www.greenend.org.uk/rjk/tech/inline.html], there are two ways to
> > >generate such an external definition for inline functions. Apart from
> > >defining the inline function in a .h file we need to either declare it
> > >as 'extern' exactly once or declare it by omitting 'inline' exactly
> > >once.
> > >
> > >In doing so we can make sure that the compiler sees the definition
> > >whenever it is used from multiple files and can be 'inlined'
> > >effectively. For us, this correction will require moving definitions
> > >out of .c files to .h file. Instead of changing all such instances by a
> > >single person, it can be done component-wise. Here also we have the
> > >other option of removing the inline keyword as a whole.
> > >
> > >Overall, we have 3 options:
> > >[1] Fix all inline functions according to C99 standard.
> > >[2] Fix only those inline functions which doesn't require external
> > >linkage and remove inline from those who require external linkage.
> > >[3] Remove the inline keyword from functions.
> > >
> > >Suggestions/comments are mostly welcome.
> > >
> > >Thanks,
> > >--Anoop C S.
> > >
> > 
> _______________________________________________
> Gluster-devel mailing list
> Gluster-devel@xxxxxxxxxxx
> http://www.gluster.org/mailman/listinfo/gluster-devel
> 
_______________________________________________
Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
http://www.gluster.org/mailman/listinfo/gluster-devel



[Index of Archives]     [Gluster Users]     [Ceph Users]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux