On Oct 17, 2011, at 4:14 PM, George Bosilca wrote:
> Ralph,
>
> I have a concern about the code below (snippet from the commit 25303). You
> moved the setting of proc_flags and proc_name in a function named set_arch.
> As the name and the lengthy comment above it indicate, the scope of this
> particular function is to set the architecture of the remote process, not the
> locality flag or the process name.
I agree. However, there is no harm in moving those settings to that function.
Indeed, there is a significant advantage in doing so as it allows the data to
be exchanged during the modex, instead of mandating that ORTE provide it.
I agree that the function name is now somewhat inaccurate, but chose not to
change it as (a) I couldn't think of a better alternative, and (b) it seemed a
trivial issue. If it bothers you or others, however, please feel free to change
the name of the function.
>
> george.
>
>
> Index: ompi/proc/proc.c
> ===================================================================
> --- ompi/proc/proc.c (revision 25302)
> +++ ompi/proc/proc.c (working copy)
> @@ -119,11 +119,6 @@
> if (OMPI_SUCCESS != (ret = ompi_modex_send_key_value("OMPI_ARCH",
> &proc->proc_arch, OPAL_UINT32))) {
> return ret;
> }
> - } else {
> - /* get the locality information */
> - proc->proc_flags = orte_ess.proc_get_locality(&proc->proc_name);
> - /* get the name of the node it is on */
> - proc->proc_hostname =
> orte_ess.proc_get_hostname(&proc->proc_name);
> }
> }
>
> @@ -149,8 +144,8 @@
> OPAL_THREAD_LOCK(&ompi_proc_lock);
>
> for( item = opal_list_get_first(&ompi_proc_list);
> - item != opal_list_get_end(&ompi_proc_list);
> - item = opal_list_get_next(item)) {
> + item != opal_list_get_end(&ompi_proc_list);
> + item = opal_list_get_next(item)) {
> proc = (ompi_proc_t*)item;
>
> if (proc->proc_name.vpid != ORTE_PROC_MY_NAME->vpid) {
> @@ -177,6 +172,10 @@
> OPAL_THREAD_UNLOCK(&ompi_proc_lock);
> return ret;
> }
> + /* get the locality information */
> + proc->proc_flags = orte_ess.proc_get_locality(&proc->proc_name);
> + /* get the name of the node it is on */
> + proc->proc_hostname =
> orte_ess.proc_get_hostname(&proc->proc_name);
> }
> }
> OPAL_THREAD_UNLOCK(&ompi_proc_lock);
>
>
>
>
> On Oct 17, 2011, at 16:51 , r...@osl.iu.edu wrote:
>
>> Author: rhc
>> Date: 2011-10-17 16:51:22 EDT (Mon, 17 Oct 2011)
>> New Revision: 25303
>> URL: https://svn.open-mpi.org/trac/ompi/changeset/25303
>>
>> Log:
>>
>> Complete implementation of pmi support. Ensure we support both mpirun and
>> direct launch within same configuration to avoid requiring separate builds.
>> Add support for generic pmi, not just under slurm. Add publish/subscribe
>> support, although slurm's pmi implementation will just return an error as it
>> hasn't been done yet.
>>
>>
>>
>> Added:
>> trunk/ompi/mca/pubsub/pmi/ (props changed)
>> trunk/ompi/mca/pubsub/pmi/Makefile.am
>> trunk/ompi/mca/pubsub/pmi/configure.m4
>> trunk/ompi/mca/pubsub/pmi/pubsub_pmi.c
>> trunk/ompi/mca/pubsub/pmi/pubsub_pmi.h
>> trunk/ompi/mca/pubsub/pmi/pubsub_pmi_component.c
>> trunk/orte/mca/ess/pmi/ (props changed)
>> trunk/orte/mca/ess/pmi/Makefile.am
>> trunk/orte/mca/ess/pmi/configure.m4
>> trunk/orte/mca/ess/pmi/ess_pmi.h
>> trunk/orte/mca/ess/pmi/ess_pmi_component.c
>> trunk/orte/mca/ess/pmi/ess_pmi_module.c
>> Text files modified:
>> trunk/ompi/proc/proc.c | 13
>>
>> trunk/orte/config/orte_check_pmi.m4 | 3
>>
>> trunk/orte/mca/ess/slurmd/Makefile.am | 10
>>
>> trunk/orte/mca/ess/slurmd/configure.m4 | 16 -
>>
>> trunk/orte/mca/ess/slurmd/ess_slurmd_component.c | 16 -
>>
>> trunk/orte/mca/ess/slurmd/ess_slurmd_module.c | 158 +++++---------
>>
>> trunk/orte/mca/grpcomm/pmi/grpcomm_pmi_component.c | 43 +++
>>
>> trunk/orte/mca/grpcomm/pmi/grpcomm_pmi_module.c | 414
>> +++++++++++++++++++++++++++------------
>> trunk/orte/util/nidmap.c | 3
>>
>> 9 files changed, 396 insertions(+), 280 deletions(-)
>>
>>
>> Diff not shown due to size (69184 bytes).
>> To see the diff, run the following command:
>>
>> svn diff -r 25302:25303 --no-diff-deleted
>>
>> _______________________________________________
>> svn mailing list
>> s...@open-mpi.org
>> http://www.open-mpi.org/mailman/listinfo.cgi/svn
>
>
> _______________________________________________
> devel mailing list
> de...@open-mpi.org
> http://www.open-mpi.org/mailman/listinfo.cgi/devel
