I have commited fixes for
opal/mca/compress/base/compress_base_open.c and
opal/mca/crs/base/crs_base_open.c
which return OPAL_SUCCESS but do not open the components if CR is
disabled.
Adrian
On Tue, Jan 07, 2014 at 01:43:00PM -0600, Josh Hursey wrote:
> Either would be fine with me. If you left in the verbose message then it
> might be a bit confusing to any user that might see it.
>
>
> On Fri, Jan 3, 2014 at 9:13 AM, Ralph Castain <r...@open-mpi.org> wrote:
>
> > That would work. Alternatively, you could try just returning OPAL_SUCCESS
> > in place of OPAL_ERR_NOT_AVAILABLE. This would still avoid opening the
> > components for no reason (thus saving some memory) while not causing
> > opal_init to abort.
> >
> >
> > On Jan 3, 2014, at 3:19 AM, Adrian Reber <adr...@lisas.de> wrote:
> >
> > > So removing all output like in this patch would be ok?
> > >
> > > diff --git a/opal/mca/compress/base/compress_base_open.c
> > b/opal/mca/compress/base/compress_base_open.c
> > > index a09fe59..f487752 100644
> > > --- a/opal/mca/compress/base/compress_base_open.c
> > > +++ b/opal/mca/compress/base/compress_base_open.c
> > > @@ -14,13 +14,9 @@
> > >
> > > #include "opal_config.h"
> > >
> > > -#include <string.h>
> > > -#include "opal/mca/mca.h"
> > > #include "opal/mca/base/base.h"
> > > #include "opal/include/opal/constants.h"
> > > -#include "opal/mca/compress/compress.h"
> > > #include "opal/mca/compress/base/base.h"
> > > -#include "opal/util/output.h"
> > >
> > > #include "opal/mca/compress/base/static-components.h"
> > >
> > > @@ -45,13 +41,6 @@ MCA_BASE_FRAMEWORK_DECLARE(opal, compress, NULL,
> > opal_compress_base_register, op
> > >
> > > static int opal_compress_base_register (mca_base_register_flag_t flags)
> > > {
> > > - /* Compression currently only used with C/R */
> > > - if( !opal_cr_is_enabled ) {
> > > - opal_output_verbose(10,
> > opal_compress_base_framework.framework_output,
> > > - "compress:open: FT is not enabled,
> > skipping!");
> > > - return OPAL_ERR_NOT_AVAILABLE;
> > > - }
> > > -
> > > return OPAL_SUCCESS;
> > > }
> > >
> > > @@ -61,13 +50,6 @@ static int opal_compress_base_register
> > (mca_base_register_flag_t flags)
> > > */
> > > int opal_compress_base_open(mca_base_open_flag_t flags)
> > > {
> > > - /* Compression currently only used with C/R */
> > > - if( !opal_cr_is_enabled ) {
> > > - opal_output_verbose(10,
> > opal_compress_base_framework.framework_output,
> > > - "compress:open: FT is not enabled,
> > skipping!");
> > > - return OPAL_SUCCESS;
> > > - }
> > > -
> > > /* Open up all available components */
> > > return mca_base_framework_components_open
> > (&opal_compress_base_framework, flags);
> > > }
> > >
> > >
> > >
> > > On Thu, Jan 02, 2014 at 12:32:32PM -0500, Josh Hursey wrote:
> > >> I think the only reason I protected that framework is to reduce the
> > >> overhead of an application using a build of Open MPI with CR support,
> > but
> > >> no enabling it at runtime. Nothing in the compress framework depends on
> > the
> > >> CR infrastructure (although the CR infrastructure can use the compress
> > >> framework if the user chooses to). So I bet we can remove the protection
> > >> altogether and be fine.
> > >>
> > >> So I think this patch is fine. I might also go as far as removing the
> > 'if'
> > >> block altogether as the protection should not been needed any longer.
> > >>
> > >> Thanks,
> > >> Josh
> > >>
> > >>
> > >>
> > >> On Fri, Dec 27, 2013 at 3:46 PM, Adrian Reber <adr...@lisas.de> wrote:
> > >>
> > >>> Right now the C/R code fails because of a change introduced in
> > >>> opal/mca/compress/base/compress_base_open.c in 2013 with commit
> > >>>
> > >>> git 734c724ff76d9bf814f3ab0396bcd9ee6fddcd1b
> > >>> svn r28239
> > >>>
> > >>> Update OPAL frameworks to use the MCA framework system.
> > >>>
> > >>> This commit changed a lot but also the return value of the function
> > from
> > >>> OPAL_SUCCESS to OPAL_ERR_NOT_AVAILABLE. With following patch I can make
> > >>> it work again:
> > >>>
> > >>> diff --git a/opal/mca/compress/base/compress_base_open.c
> > >>> b/opal/mca/compress/base/compress_base_open.c
> > >>> index a09fe59..69eabfa 100644
> > >>> --- a/opal/mca/compress/base/compress_base_open.c
> > >>> +++ b/opal/mca/compress/base/compress_base_open.c
> > >>> @@ -45,11 +45,11 @@ MCA_BASE_FRAMEWORK_DECLARE(opal, compress, NULL,
> > >>> opal_compress_base_register, op
> > >>>
> > >>> static int opal_compress_base_register (mca_base_register_flag_t flags)
> > >>> {
> > >>> /* Compression currently only used with C/R */
> > >>> if( !opal_cr_is_enabled ) {
> > >>> opal_output_verbose(10,
> > >>> opal_compress_base_framework.framework_output,
> > >>> "compress:open: FT is not enabled,
> > >>> skipping!");
> > >>> - return OPAL_ERR_NOT_AVAILABLE;
> > >>> + return OPAL_SUCCESS;
> > >>> }
> > >>>
> > >>> return OPAL_SUCCESS;
> > >>>
> > >>>
> > >>> My question is if OPAL_ERR_NOT_AVAILABLE is indeed the correct return
> > value
> > >>> and the function calling opal_compress_base_register() should actually
> > >>> handle OPAL_ERR_NOT_AVAILABLE or if returning OPAL_SUCCESS is still the
> > >>> right
> > >>> return value?
> > >>>
> > >>> Adrian
> > >>> _______________________________________________
> > >>> devel mailing list
> > >>> de...@open-mpi.org
> > >>> http://www.open-mpi.org/mailman/listinfo.cgi/devel
> > >>>
> > >>
> > >>
> > >>
> > >> --
> > >> Joshua Hursey
> > >> Assistant Professor of Computer Science
> > >> University of Wisconsin-La Crosse
> > >> http://cs.uwlax.edu/~jjhursey
> > >
> > >> _______________________________________________
> > >> devel mailing list
> > >> de...@open-mpi.org
> > >> http://www.open-mpi.org/mailman/listinfo.cgi/devel
> > > _______________________________________________
> > > devel mailing list
> > > de...@open-mpi.org
> > > http://www.open-mpi.org/mailman/listinfo.cgi/devel
> >
> > _______________________________________________
> > devel mailing list
> > de...@open-mpi.org
> > http://www.open-mpi.org/mailman/listinfo.cgi/devel
> >
>
>
>
> --
> Joshua Hursey
> Assistant Professor of Computer Science
> University of Wisconsin-La Crosse
> http://cs.uwlax.edu/~jjhursey
> _______________________________________________
> devel mailing list
> de...@open-mpi.org
> http://www.open-mpi.org/mailman/listinfo.cgi/devel
Adrian
--
Adrian Reber <adr...@lisas.de> http://lisas.de/~adrian/
Peers's Law:
The solution to a problem changes the nature of the problem.
