Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: Re: [OMPI devel] return value of opal_compress_base_register() in opal/mca/compress/base/compress_base_open.c
From: Adrian Reber (adrian_at_[hidden])
Date: 2014-01-03 06:19:34


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 <adrian_at_[hidden]> 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
> > devel_at_[hidden]
> > 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
> devel_at_[hidden]
> http://www.open-mpi.org/mailman/listinfo.cgi/devel