| Message ID | 20240129113434.40940-1-roberto.acosta@luizalabs.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,ovs,v2] 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 |
On Mon, Jan 29, 2024 at 12:33 PM Roberto Bartzen Acosta <roberto.acosta@luizalabs.com> wrote: > > Updating the reference documentation with the inclusion of possible building problems with libjemalloc and solution suggestions. nit: the above line is very long and does not look well in `git show` > 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 | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst > index 19e360d47..e2eb19510 100644 > --- a/Documentation/intro/install/general.rst > +++ b/Documentation/intro/install/general.rst > @@ -344,6 +344,22 @@ 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. You can > + override the automatic compiler decision to avoid possible linker issues by > + passing ``-fno-lto`` or disabling ``-fno-builtin`` flag since the jemalloc nit: Using the word `disabling` here creates a slightly confusing double negative because of the flag already having a ``no-`` in its name, in the current form I think we could just drop `disabling`, 'passing A or B flag' works fine without it. > + 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 Thanks for updating the docs with this information, I had a couple of nits above, let's hear from one of the maintainers if they agree and if any update could be incorporated as part of a merge before deciding if any iterations are required. Reviewed-by: Frode Nordahl <frode.nordahl@canonical.com> -- Frode Nordahl
On Wed, Jan 31, 2024 at 10:00:11AM +0100, Frode Nordahl wrote: > On Mon, Jan 29, 2024 at 12:33 PM Roberto Bartzen Acosta > <roberto.acosta@luizalabs.com> wrote: > > > > Updating the reference documentation with the inclusion of possible building problems with libjemalloc and solution suggestions. > > nit: the above line is very long and does not look well in `git show` > > > 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 | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst > > index 19e360d47..e2eb19510 100644 > > --- a/Documentation/intro/install/general.rst > > +++ b/Documentation/intro/install/general.rst > > @@ -344,6 +344,22 @@ 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. You can > > + override the automatic compiler decision to avoid possible linker issues by > > + passing ``-fno-lto`` or disabling ``-fno-builtin`` flag since the jemalloc > > nit: Using the word `disabling` here creates a slightly confusing > double negative because of the flag already having a ``no-`` in its > name, in the current form I think we could just drop `disabling`, > 'passing A or B flag' works fine without it. > > > + 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 > > Thanks for updating the docs with this information, I had a couple of > nits above, let's hear from one of the maintainers if they agree and > if any update could be incorporated as part of a merge before deciding > if any iterations are required. > > Reviewed-by: Frode Nordahl <frode.nordahl@canonical.com> Thanks Robert and Frode, I for one am happy with this approach. But I would like to see Frode's nit's addressed in a v3.
diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst index 19e360d47..e2eb19510 100644 --- a/Documentation/intro/install/general.rst +++ b/Documentation/intro/install/general.rst @@ -344,6 +344,22 @@ 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. You can + override the automatic compiler decision to avoid possible linker issues 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
Updating the reference documentation with the inclusion of possible building problems with libjemalloc and solution suggestions. 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 | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)