| Message ID | 20240126165749.2152018-1-roberto.acosta@luizalabs.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,ovs] Documentation: Adding note about using the jemalloc library. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
Thanks for raising the patch towards the documentation, Roberto! On Fri, Jan 26, 2024 at 5:57 PM Roberto Bartzen Acosta via dev <ovs-dev@openvswitch.org> wrote: > > Current version of debian/rules simply uses the default GCC optimization settings during the linkage process. > > The main problem with this approach is that GCC on OS like Ubuntu Jammy, for example, can enable the -flto=auto flag option during the Open vSwitch building and linking process. In this case, the linked dynamic libraries would need to be compatible with the same lto optimization options, at the risk of not working, according to documentation [1]. However, it could be possible to link to jemalloc using the LTO option but the GIMPLE-bytecode may not work correctly. In this case, it would be necessary to disable the -fbuiltin flag [2] if it is set as default. As we started to discuss in the other review, I'm not entirely convinced that the mechanics of LTO is directly involved in the linkers omission to register the jemalloc shared library as a dependency. It might as well be a compiler or linker bug related to compiler built-ins that is revealed when LTO is enabled? We could of course spend weeks getting to the bottom of that, or we could tone that part of the commit message and in-line note down and provide a more generic observation based guidance, unless you are certain about the correlation?
Em sex., 26 de jan. de 2024 às 16:55, Frode Nordahl < frode.nordahl@canonical.com> escreveu: > Thanks for raising the patch towards the documentation, Roberto! > > On Fri, Jan 26, 2024 at 5:57 PM Roberto Bartzen Acosta via dev > <ovs-dev@openvswitch.org> wrote: > > > > Current version of debian/rules simply uses the default GCC optimization > settings during the linkage process. > > > > The main problem with this approach is that GCC on OS like Ubuntu Jammy, > for example, can enable the -flto=auto flag option during the Open vSwitch > building and linking process. In this case, the linked dynamic libraries > would need to be compatible with the same lto optimization options, at the > risk of not working, according to documentation [1]. However, it could be > possible to link to jemalloc using the LTO option but the GIMPLE-bytecode > may not work correctly. In this case, it would be necessary to disable the > -fbuiltin flag [2] if it is set as default. > > As we started to discuss in the other review, I'm not entirely > convinced that the mechanics of LTO is directly involved in the > linkers omission to register the jemalloc shared library as a > dependency. It might as well be a compiler or linker bug related to > compiler built-ins that is revealed when LTO is enabled? > Yes, that would be a possible theory for this problem. So, without LTO enabled I don't see a problem with build-ins when linking with jemalloc. > We could of course spend weeks getting to the bottom of that, or we > could tone that part of the commit message and in-line note down and > provide a more generic observation based guidance, unless you are > certain about the correlation? > It seems reasonable to me but I think about keeping the in-line note about the two examples that can solve the problem. I mean, referencing disabling LTO or built-in flags. .. note:: Linking Open vSwitch with the jemalloc shared library may not work as expected in certain operating system development environments. You can override the automatic compiler decision to avoid possible linker issues by passing ``-fno-lto`` or disabling ``-fno-builtin`` flag since the jemalloc overrides standard built-in memory allocation functions such as malloc, calloc, etc. Both options can solve possible jemalloc linker issues with pros and cons for each case, feel free to choose the path that appears best to you. Disabling LTO flag example:: $ ./configure LIBS=-ljemalloc CFLAGS=-fno-lto Disabling built-in flag example:: ./configure LIBS=-ljemalloc CFLAGS=-fno-builtin Do you think this would be good? Thanks > > -- > Frode Nordahl > > > > > Updating the reference documentation with the inclusion of possible > building problems with libjemalloc and solution suggestions. > > > > [1] https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-flto > > [2] https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html > > > > Reported-at: > https://bugs.launchpad.net/ubuntu/+source/openvswitch/+bug/2015748 > > Signed-off-by: Roberto Bartzen Acosta <roberto.acosta@luizalabs.com> > > --- > > Documentation/intro/install/general.rst | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/Documentation/intro/install/general.rst > b/Documentation/intro/install/general.rst > > index 19e360d47..ef4e6181d 100644 > > --- a/Documentation/intro/install/general.rst > > +++ b/Documentation/intro/install/general.rst > > @@ -344,6 +344,23 @@ you wish to link with jemalloc add it to LIBS:: > > > > $ ./configure LIBS=-ljemalloc > > > > +.. note:: > > + Linking Open vSwitch with the jemalloc shared library may not work as > > + expected in certain operating system development environments if the > GCC > > + compiler tries to optimize the linking process using the ``-flto`` > flag. > > + You can overrides the automatic compiler decision removing the > link-time > > + optimization by passing ``-fno-lto`` or disabling ``-fno-builtin`` > flag since > > + the jemalloc override standard built-in memory allocation functions > such as > > + malloc, calloc, etc. Both options can solve possible jemalloc linker > issues > > + with pros and cons for each case, feel free to choose the path that > appears > > + best to you. Disabling LTO flag example:: > > + > > + $ ./configure LIBS=-ljemalloc CFLAGS=-fno-lto > > + > > + Disabling built-in flag example:: > > + > > + ./configure LIBS=-ljemalloc CFLAGS=-fno-builtin > > + > > .. _general-building: > > > > Building > > -- > > 2.25.1 > > > > > > -- > > > > > > > > > > _'Esta mensagem é direcionada apenas para os endereços constantes no > > cabeçalho inicial. Se você não está listado nos endereços constantes no > > cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa > > mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas > estão > > imediatamente anuladas e proibidas'._ > > > > > > * **'Apesar do Magazine Luiza tomar > > todas as precauções razoáveis para assegurar que nenhum vírus esteja > > presente nesse e-mail, a empresa não poderá aceitar a responsabilidade > por > > quaisquer perdas ou danos causados por esse e-mail ou por seus anexos'.* > > > > > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Fri, Jan 26, 2024 at 10:29 PM Roberto Bartzen Acosta <roberto.acosta@luizalabs.com> wrote: > > > > Em sex., 26 de jan. de 2024 às 16:55, Frode Nordahl <frode.nordahl@canonical.com> escreveu: >> >> Thanks for raising the patch towards the documentation, Roberto! >> >> On Fri, Jan 26, 2024 at 5:57 PM Roberto Bartzen Acosta via dev >> <ovs-dev@openvswitch.org> wrote: >> > >> > Current version of debian/rules simply uses the default GCC optimization settings during the linkage process. >> > >> > The main problem with this approach is that GCC on OS like Ubuntu Jammy, for example, can enable the -flto=auto flag option during the Open vSwitch building and linking process. In this case, the linked dynamic libraries would need to be compatible with the same lto optimization options, at the risk of not working, according to documentation [1]. However, it could be possible to link to jemalloc using the LTO option but the GIMPLE-bytecode may not work correctly. In this case, it would be necessary to disable the -fbuiltin flag [2] if it is set as default. >> >> As we started to discuss in the other review, I'm not entirely >> convinced that the mechanics of LTO is directly involved in the >> linkers omission to register the jemalloc shared library as a >> dependency. It might as well be a compiler or linker bug related to >> compiler built-ins that is revealed when LTO is enabled? > > > Yes, that would be a possible theory for this problem. So, without LTO enabled I don't see a problem with build-ins when linking with jemalloc. > >> >> We could of course spend weeks getting to the bottom of that, or we >> could tone that part of the commit message and in-line note down and >> provide a more generic observation based guidance, unless you are >> certain about the correlation? > > > It seems reasonable to me but I think about keeping the in-line note about the two examples that can solve the problem. I mean, referencing disabling LTO or built-in flags. > .. note:: Linking Open vSwitch with the jemalloc shared library may not work as expected in certain operating system development environments. You can override the automatic compiler decision to avoid possible linker issues by passing ``-fno-lto`` or disabling ``-fno-builtin`` flag since the jemalloc overrides standard built-in memory allocation functions such as malloc, calloc, etc. Both options can solve possible jemalloc linker issues with pros and cons for each case, feel free to choose the path that appears best to you. Disabling LTO flag example:: > $ ./configure LIBS=-ljemalloc CFLAGS=-fno-lto > Disabling built-in flag example:: > ./configure LIBS=-ljemalloc CFLAGS=-fno-builtin > > Do you think this would be good? Yes, this wording looks like a good approach to me, others might have opinions too, and if you post a v2 like this with a suitable commit message it would LGTM. Thanks!
diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst index 19e360d47..ef4e6181d 100644 --- a/Documentation/intro/install/general.rst +++ b/Documentation/intro/install/general.rst @@ -344,6 +344,23 @@ you wish to link with jemalloc add it to LIBS:: $ ./configure LIBS=-ljemalloc +.. note:: + Linking Open vSwitch with the jemalloc shared library may not work as + expected in certain operating system development environments if the GCC + compiler tries to optimize the linking process using the ``-flto`` flag. + You can overrides the automatic compiler decision removing the link-time + optimization by passing ``-fno-lto`` or disabling ``-fno-builtin`` flag since + the jemalloc override standard built-in memory allocation functions such as + malloc, calloc, etc. Both options can solve possible jemalloc linker issues + with pros and cons for each case, feel free to choose the path that appears + best to you. Disabling LTO flag example:: + + $ ./configure LIBS=-ljemalloc CFLAGS=-fno-lto + + Disabling built-in flag example:: + + ./configure LIBS=-ljemalloc CFLAGS=-fno-builtin + .. _general-building: Building
Current version of debian/rules simply uses the default GCC optimization settings during the linkage process. The main problem with this approach is that GCC on OS like Ubuntu Jammy, for example, can enable the -flto=auto flag option during the Open vSwitch building and linking process. In this case, the linked dynamic libraries would need to be compatible with the same lto optimization options, at the risk of not working, according to documentation [1]. However, it could be possible to link to jemalloc using the LTO option but the GIMPLE-bytecode may not work correctly. In this case, it would be necessary to disable the -fbuiltin flag [2] if it is set as default. Updating the reference documentation with the inclusion of possible building problems with libjemalloc and solution suggestions. [1] https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-flto [2] https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html Reported-at: https://bugs.launchpad.net/ubuntu/+source/openvswitch/+bug/2015748 Signed-off-by: Roberto Bartzen Acosta <roberto.acosta@luizalabs.com> --- Documentation/intro/install/general.rst | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)