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: Ralph Castain (rhc_at_[hidden])
Date: 2014-01-03 10:13:38


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