On Sep 11, 2007, at 11:05 AM, Gleb Natapov wrote:
On Tue, Sep 11, 2007 at 10:54:25AM -0400, George Bosilca wrote:We don't want to prevent two thread from entering the code is same time. The algorithm you cited support this case. There is only one moment that isAre you sure it support this case? There is a global var mask_in_use that prevent multiple access.
I'm unable to find the mask_in_use global variable. Where it is ? george.
critical. The local selection of the next available cid. And this is what we try to protect there. If after the first run, the collective call do not manage to figure out the correct next_cid then we will execute the while loop again. And then this condition make sense, as only the thread running on the smallest communicator cid will continue. This insure that it will pickup the smallest next available cid, and then it's reduce operation willsucceed. The other threads will wait until the selection of the next available cid is unlocked.Without the code you removed we face a deadlock situation. Multiple threads will pick different next_cid on each process and thy will never succeed with the reduce operation. And this is what we're trying to avoid with thetest.OK. I think now I get the idea behind this test. I'll restore it and leave ompi_comm_unregister_cid() fix in place. Is this OK?george. On Sep 11, 2007, at 10:34 AM, Gleb Natapov wrote:On Tue, Sep 11, 2007 at 10:14:30AM -0400, George Bosilca wrote:Gleb,This patch is not correct. The code preventing the registration of thesame communicator twice is later in the code (same file in the function ompi_comm_register_cid line 326). Once the function ompi_comm_register_cidI saw this code and the comment. The problem is not with the same communicator but with different communicators.is called, we know that each communicator only handle one "communicator creation" function at the same time. Therefore, we want to give priorityThe code I removed was doing it wrongly. I.e the algorithm sometimes isto the smallest com_id, which is what happens in the code you removed.executedfor different communicators simultaneously by different threads. Think about the case where the function is running for cid 1 and then another thread runs it for cid 0. cid 0 will proceed although the function isexecuted on another CPU. And this is not something theoretical, that is happening with sun's thread test suit mpi_coll test case.If the algorithm is really prone to deadlock in case it is concurrentlyWithout the condition in the ompi_comm_register_cid (each communicatoronlyget registered once) your comment make sense. However, with the condition your patch allow a dead end situation, while 2 processes try to create communicators in multiple threads, and they will never succeed, simplybecause they will not order the creation based on the com_id.executed for several different communicators (I haven't check this),then we may want to fix original code to really prevent two threads toenter the function, but then I don't see the reason for all thosecomplications with ompi_comm_register_cid()/ ompi_comm_unregister_cid()The algorithm described here:http://209.85.129.104/search?q=cache:5PV5MMRkBWkJ:ftp:// info.mcs.anl.gov/pub/tech_reports/reports/P1382.pdf+MPI +communicator+dup+algorithm&hl=en&ct=clnk&cd=2in section 5.3 works without it and we can do something similar.george. On Sep 11, 2007, at 9:23 AM, [email protected] wrote:Author: gleb Date: 2007-09-11 09:23:46 EDT (Tue, 11 Sep 2007) New Revision: 16088 URL: https://svn.open-mpi.org/trac/ompi/changeset/16088 Log: The code tries to prevent itself from running for more then one communicatorsimultaneously, but is doing it incorrectly. If the function is runningalreadyfor one communicator and it is called from another thread for othercommunicatorwith lower cid the check comm->c_contextid != ompi_comm_lowest_cid()will fail and the function will be executed for two different communicators by two threads simultaneously. There is nothing in the algorithm that prevent itfrom been running simultaneously for different communicators as far as Ican see,but ompi_comm_unregister_cid() assumes that it is always called for acommunicatorwith the lowest cid and this is not always the case. This patch removesboguslowest cid check and fix ompi_comm_register_cid() to properly remove cidfrom the list. Text files modified:trunk/ompi/communicator/comm_cid.c | 24 +++++++++++ +------------1 files changed, 12 insertions(+), 12 deletions(-) Modified: trunk/ompi/communicator/comm_cid.c================================================================== ============--- trunk/ompi/communicator/comm_cid.c (original)+++ trunk/ompi/communicator/comm_cid.c 2007-09-11 09:23:46 EDT (Tue, 11Sep 2007) @@ -11,6 +11,7 @@ * All rights reserved.* Copyright (c) 2006-2007 University of Houston. All rights reserved.* Copyright (c) 2007 Cisco, Inc. All rights reserved. + * Copyright (c) 2007 Voltaire All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -170,15 +171,6 @@ * This is the real algorithm described in the doc */ - OPAL_THREAD_LOCK(&ompi_cid_lock); - if (comm->c_contextid != ompi_comm_lowest_cid() ) {- /* if not lowest cid, we do not continue, but sleep andtry again */ - OPAL_THREAD_UNLOCK(&ompi_cid_lock); - continue; - } - OPAL_THREAD_UNLOCK(&ompi_cid_lock); - - for (i=start; i < mca_pml.pml_max_contextid ; i++) { flag=ompi_pointer_array_test_and_set_item(&ompi_mpi_communicators,i, comm);@@ -365,10 +357,18 @@ static int ompi_comm_unregister_cid (uint32_t cid) { - ompi_comm_reg_t *regcom=NULL; - opal_list_item_t *item=opal_list_remove_first(&ompi_registered_comms); + ompi_comm_reg_t *regcom; + opal_list_item_t *item; - regcom = (ompi_comm_reg_t *) item; + for (item = opal_list_get_first(&ompi_registered_comms); + item != opal_list_get_end(&ompi_registered_comms); + item = opal_list_get_next(item)) { + regcom = (ompi_comm_reg_t *)item; + if(regcom->cid == cid) { + opal_list_remove_item(&ompi_registered_comms, item); + break; + } + } OBJ_RELEASE(regcom); return OMPI_SUCCESS; } _______________________________________________ svn-full mailing list [email protected] http://www.open-mpi.org/mailman/listinfo.cgi/svn-full_______________________________________________ devel mailing list [email protected] http://www.open-mpi.org/mailman/listinfo.cgi/devel-- Gleb. _______________________________________________ devel mailing list [email protected] http://www.open-mpi.org/mailman/listinfo.cgi/devel_______________________________________________ devel mailing list [email protected] http://www.open-mpi.org/mailman/listinfo.cgi/devel-- Gleb. _______________________________________________ devel mailing list [email protected] http://www.open-mpi.org/mailman/listinfo.cgi/devel
smime.p7s
Description: S/MIME cryptographic signature
