On Oct 18, 2007, at 4:13 AM, Gleb Natapov wrote:
My claim is that the logic is equivalent :) But I am asking here for others to comment.
I was thinking that being quiet is a kind of agreement :) Anyway, I think this is a useful change, and there is no apparent behavior modification between the original code and yours. Please go ahead and commit it.
Thanks,
george.
In most places the logic is like this: if (req is not completed) { if (more then one thread) { acquire lock wait for completion release lock } else { wait for completion } } inline function does: if (req is not completed) { if (more then one thread) { acquire lock } wait for completion if (more then one thread) { release lock } }And in non threaded build both "if (one the one thread){}" statements areremoved by preprocessor.Also, a minor nit -- it would be nice if the new inline function conformed to our coding standards (constants on the left of ==, {} around all blocks, etc.). :-)OK. The new code mimics the code it replaces :)On Oct 15, 2007, at 10:27 AM, Gleb Natapov wrote:Hi, Each time a someone needs to wait for request completion he implements the same piece of code. Why not put this code into inline function and use it instead. Look at the included patch, it moves the common code into ompi_request_wait_completion() function. Does somebody have any objection against committing it to the trunk? diff --git a/ompi/mca/crcp/coord/crcp_coord_pml.c b/ompi/mca/crcp/ coord/crcp_coord_pml.c index b2392e4..eb9b9c1 100644 --- a/ompi/mca/crcp/coord/crcp_coord_pml.c +++ b/ompi/mca/crcp/coord/crcp_coord_pml.c @@ -3857,13 +3857,7 @@ static int coord_request_wait_all( size_t count, static int coord_request_wait( ompi_request_t * req, ompi_status_public_t * status) { - OPAL_THREAD_LOCK(&ompi_request_lock); - ompi_request_waiting++; - while (req->req_complete == false) {- opal_condition_wait(&ompi_request_cond, &ompi_request_lock);- } - ompi_request_waiting--; - OPAL_THREAD_UNLOCK(&ompi_request_lock); + ompi_request_wait_completion(req); if( MPI_STATUS_IGNORE != status ) { status->MPI_TAG = req->req_status.MPI_TAG; diff --git a/ompi/mca/pml/cm/pml_cm_recv.c b/ompi/mca/pml/cm/ pml_cm_recv.c index 0e23c9a..00efffc 100644 --- a/ompi/mca/pml/cm/pml_cm_recv.c +++ b/ompi/mca/pml/cm/pml_cm_recv.c @@ -112,22 +112,7 @@ mca_pml_cm_recv(void *addr, return ret; } - if (recvreq->req_base.req_ompi.req_complete == false) { - /* give up and sleep until completion */ - if (opal_using_threads()) { - opal_mutex_lock(&ompi_request_lock); - ompi_request_waiting++;- while (recvreq->req_base.req_ompi.req_complete == false)- opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; - opal_mutex_unlock(&ompi_request_lock); - } else { - ompi_request_waiting++;- while (recvreq->req_base.req_ompi.req_complete == false)- opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; - } - } + ompi_request_wait_completion(&recvreq->req_base.req_ompi); if (NULL != status) { /* return status */ *status = recvreq->req_base.req_ompi.req_status; diff --git a/ompi/mca/pml/cm/pml_cm_send.c b/ompi/mca/pml/cm/ pml_cm_send.c index ed9b189..f7d2e8c 100644 --- a/ompi/mca/pml/cm/pml_cm_send.c +++ b/ompi/mca/pml/cm/pml_cm_send.c @@ -175,23 +175,8 @@ mca_pml_cm_send(void *buf, MCA_PML_CM_THIN_SEND_REQUEST_RETURN(sendreq); return ret; } - - if (sendreq->req_send.req_base.req_ompi.req_complete == false) { - /* give up and sleep until completion */ - if (opal_using_threads()) { - opal_mutex_lock(&ompi_request_lock); - ompi_request_waiting++; - while (sendreq-req_send.req_base.req_ompi.req_complete == false)- opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; - opal_mutex_unlock(&ompi_request_lock); - } else { - ompi_request_waiting++; - while (sendreq-req_send.req_base.req_ompi.req_complete == false)- opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; - } - } + + ompi_request_wait_completion(&sendreq-req_send.req_base.req_ompi);ompi_request_free( (ompi_request_t**)&sendreq ); } else { diff --git a/ompi/mca/pml/dr/pml_dr_iprobe.c b/ompi/mca/pml/dr/ pml_dr_iprobe.c index 9149174..2063c54 100644 --- a/ompi/mca/pml/dr/pml_dr_iprobe.c +++ b/ompi/mca/pml/dr/pml_dr_iprobe.c @@ -64,22 +64,7 @@ int mca_pml_dr_probe(int src, MCA_PML_DR_RECV_REQUEST_INIT(&recvreq, NULL, 0, &ompi_mpi_char, src, tag, comm, true); MCA_PML_DR_RECV_REQUEST_START(&recvreq); - if (recvreq.req_recv.req_base.req_ompi.req_complete == false) { - /* give up and sleep until completion */ - if (opal_using_threads()) { - opal_mutex_lock(&ompi_request_lock); - ompi_request_waiting++; - while (recvreq.req_recv.req_base.req_ompi.req_complete == false) - opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; - opal_mutex_unlock(&ompi_request_lock); - } else { - ompi_request_waiting++; - while (recvreq.req_recv.req_base.req_ompi.req_complete == false) - opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; - } - } + ompi_request_wait_completion (&recvreq.req_recv.req_base.req_ompi); if (NULL != status) { *status = recvreq.req_recv.req_base.req_ompi.req_status; @@ -87,4 +72,3 @@ int mca_pml_dr_probe(int src, MCA_PML_BASE_RECV_REQUEST_FINI( &recvreq.req_recv ); return OMPI_SUCCESS; } - diff --git a/ompi/mca/pml/dr/pml_dr_irecv.c b/ompi/mca/pml/dr/ pml_dr_irecv.c index 4627341..8dd8c57 100644 --- a/ompi/mca/pml/dr/pml_dr_irecv.c +++ b/ompi/mca/pml/dr/pml_dr_irecv.c @@ -87,33 +87,8 @@ int mca_pml_dr_recv(void *addr, count, datatype, src, tag, comm, false); MCA_PML_DR_RECV_REQUEST_START(recvreq);- if (recvreq->req_recv.req_base.req_ompi.req_complete == false) {-#if OMPI_ENABLE_PROGRESS_THREADS - if(opal_progress_spin(&recvreq-req_recv.req_base.req_ompi.req_complete)) {- goto finished; - } -#endif + ompi_request_wait_completion(&recvreq-req_recv.req_base.req_ompi);- /* give up and sleep until completion */ - if (opal_using_threads()) { - opal_mutex_lock(&ompi_request_lock); - ompi_request_waiting++; - while (recvreq-req_recv.req_base.req_ompi.req_complete == false)- opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; - opal_mutex_unlock(&ompi_request_lock); - } else { - ompi_request_waiting++; - while (recvreq-req_recv.req_base.req_ompi.req_complete == false)- opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; - } - } - -#if OMPI_ENABLE_PROGRESS_THREADS -finished: -#endif - if (NULL != status) { /* return status */ *status = recvreq->req_recv.req_base.req_ompi.req_status; } diff --git a/ompi/mca/pml/dr/pml_dr_isend.c b/ompi/mca/pml/dr/ pml_dr_isend.c index 206599a..b4d9192 100644 --- a/ompi/mca/pml/dr/pml_dr_isend.c +++ b/ompi/mca/pml/dr/pml_dr_isend.c @@ -105,33 +105,10 @@ int mca_pml_dr_send(void *buf, return rc; }- if (sendreq->req_send.req_base.req_ompi.req_complete == false) {-#if OMPI_ENABLE_PROGRESS_THREADS - if(opal_progress_spin(&sendreq-req_send.req_base.req_ompi.req_complete)) {- ompi_request_free( (ompi_request_t**)&sendreq ); - return OMPI_SUCCESS; - } -#endif - - /* give up and sleep until completion */ - if (opal_using_threads()) { - opal_mutex_lock(&ompi_request_lock); - ompi_request_waiting++; - while (sendreq-req_send.req_base.req_ompi.req_complete == false)- opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; - opal_mutex_unlock(&ompi_request_lock); - } else { - ompi_request_waiting++; - while (sendreq-req_send.req_base.req_ompi.req_complete == false)- opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; - } - } + ompi_request_wait_completion(&sendreq-req_send.req_base.req_ompi);/* return request to pool */ rc = sendreq->req_send.req_base.req_ompi.req_status.MPI_ERROR; ompi_request_free((ompi_request_t **) & sendreq); return rc; } - diff --git a/ompi/mca/pml/ob1/pml_ob1_iprobe.c b/ompi/mca/pml/ob1/ pml_ob1_iprobe.c index ac0c54d..c86f1c7 100644 --- a/ompi/mca/pml/ob1/pml_ob1_iprobe.c +++ b/ompi/mca/pml/ob1/pml_ob1_iprobe.c @@ -64,22 +64,7 @@ int mca_pml_ob1_probe(int src, MCA_PML_OB1_RECV_REQUEST_INIT(&recvreq, NULL, 0, &ompi_mpi_char, src, tag, comm, true); MCA_PML_OB1_RECV_REQUEST_START(&recvreq); - if (recvreq.req_recv.req_base.req_ompi.req_complete == false) { - /* give up and sleep until completion */ - if (opal_using_threads()) { - opal_mutex_lock(&ompi_request_lock); - ompi_request_waiting++; - while (recvreq.req_recv.req_base.req_ompi.req_complete == false) - opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; - opal_mutex_unlock(&ompi_request_lock); - } else { - ompi_request_waiting++; - while (recvreq.req_recv.req_base.req_ompi.req_complete == false) - opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; - } - } + ompi_request_wait_completion (&recvreq.req_recv.req_base.req_ompi); if (NULL != status) { *status = recvreq.req_recv.req_base.req_ompi.req_status; @@ -87,4 +72,3 @@ int mca_pml_ob1_probe(int src, MCA_PML_BASE_RECV_REQUEST_FINI( &recvreq.req_recv ); return OMPI_SUCCESS; } - diff --git a/ompi/mca/pml/ob1/pml_ob1_irecv.c b/ompi/mca/pml/ob1/ pml_ob1_irecv.c index d2d89ce..a8d95bd 100644 --- a/ompi/mca/pml/ob1/pml_ob1_irecv.c +++ b/ompi/mca/pml/ob1/pml_ob1_irecv.c @@ -101,37 +101,7 @@ int mca_pml_ob1_recv(void *addr, PERUSE_RECV); MCA_PML_OB1_RECV_REQUEST_START(recvreq);- if (recvreq->req_recv.req_base.req_ompi.req_complete == false) {-#if OMPI_ENABLE_PROGRESS_THREADS - if(opal_progress_spin(&recvreq-req_recv.req_base.req_ompi.req_complete)) {- goto finished; - } -#endif - /* give up and sleep until completion */ - if (opal_using_threads()) { - opal_mutex_lock(&ompi_request_lock); - ompi_request_waiting++; - while (recvreq-req_recv.req_base.req_ompi.req_complete == false)- opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; - opal_mutex_unlock(&ompi_request_lock); - } else { -#if OMPI_ENABLE_DEBUG && !OMPI_HAVE_THREAD_SUPPORT - OPAL_THREAD_LOCK(&ompi_request_lock); -#endif - ompi_request_waiting++; - while (recvreq-req_recv.req_base.req_ompi.req_complete == false)- opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; -#if OMPI_ENABLE_DEBUG && !OMPI_HAVE_THREAD_SUPPORT - OPAL_THREAD_UNLOCK(&ompi_request_lock); -#endif - } - } - -#if OMPI_ENABLE_PROGRESS_THREADS -finished: -#endif + ompi_request_wait_completion(&recvreq-req_recv.req_base.req_ompi);if (NULL != status) { /* return status */ *status = recvreq->req_recv.req_base.req_ompi.req_status; diff --git a/ompi/mca/pml/ob1/pml_ob1_isend.c b/ompi/mca/pml/ob1/ pml_ob1_isend.c index a647fac..0fbf7f6 100644 --- a/ompi/mca/pml/ob1/pml_ob1_isend.c +++ b/ompi/mca/pml/ob1/pml_ob1_isend.c @@ -120,38 +120,9 @@ int mca_pml_ob1_send(void *buf, return rc; }- if (sendreq->req_send.req_base.req_ompi.req_complete == false) {-#if OMPI_ENABLE_PROGRESS_THREADS - if(opal_progress_spin(&sendreq-req_send.req_base.req_ompi.req_complete)) {- ompi_request_free( (ompi_request_t**)&sendreq ); - return OMPI_SUCCESS; - } -#endif - - /* give up and sleep until completion */ - if (opal_using_threads()) { - opal_mutex_lock(&ompi_request_lock); - ompi_request_waiting++; - while (sendreq-req_send.req_base.req_ompi.req_complete == false)- opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; - opal_mutex_unlock(&ompi_request_lock); - } else { -#if OMPI_ENABLE_DEBUG && !OMPI_HAVE_THREAD_SUPPORT - OPAL_THREAD_LOCK(&ompi_request_lock); -#endif - ompi_request_waiting++; - while (sendreq-req_send.req_base.req_ompi.req_complete == false)- opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - ompi_request_waiting--; -#if OMPI_ENABLE_DEBUG && !OMPI_HAVE_THREAD_SUPPORT - OPAL_THREAD_UNLOCK(&ompi_request_lock); -#endif - } - } + ompi_request_wait_completion(&sendreq-req_send.req_base.req_ompi);rc = sendreq->req_send.req_base.req_ompi.req_status.MPI_ERROR; ompi_request_free( (ompi_request_t**)&sendreq ); return rc; } - diff --git a/ompi/request/req_wait.c b/ompi/request/req_wait.c index 0bb4fb3..4751804 100644 --- a/ompi/request/req_wait.c +++ b/ompi/request/req_wait.c @@ -31,27 +31,7 @@ int ompi_request_wait( { ompi_request_t *req = *req_ptr; - if(req->req_complete == false) { - -#if OMPI_ENABLE_PROGRESS_THREADS - /* poll for completion */ - if(opal_progress_spin(&req->req_complete)) - goto finished; -#endif - - /* give up and sleep until completion */ - OPAL_THREAD_LOCK(&ompi_request_lock); - ompi_request_waiting++; - while (req->req_complete == false) { - opal_condition_wait(&ompi_request_cond, &ompi_request_lock); - } - ompi_request_waiting--; - OPAL_THREAD_UNLOCK(&ompi_request_lock); - } - -#if OMPI_ENABLE_PROGRESS_THREADS -finished: -#endif + ompi_request_wait_completion(req); #if OPAL_ENABLE_FT == 1 OMPI_CRCP_REQUEST_COMPLETE(req); diff --git a/ompi/request/request.h b/ompi/request/request.h index 70f38e1..8dc0920 100644 --- a/ompi/request/request.h +++ b/ompi/request/request.h @@ -374,7 +374,22 @@ OMPI_DECLSPEC int ompi_request_wait_some( int * indices, ompi_status_public_t * statuses);+static inline void ompi_request_wait_completion(ompi_request_t *req)+{ + if(req->req_complete == false) { +#if OMPI_ENABLE_PROGRESS_THREADS + if(opal_progress_spin(&req->req_complete)) + return; +#endif + OPAL_THREAD_LOCK(&ompi_request_lock); + ompi_request_waiting++; + while(req->req_complete == false) + opal_condition_wait(&ompi_request_cond, &ompi_request_lock); + ompi_request_waiting--; + OPAL_THREAD_UNLOCK(&ompi_request_lock); + } +} + END_C_DECLS #endif - -- Gleb. _______________________________________________ devel mailing list de...@open-mpi.org http://www.open-mpi.org/mailman/listinfo.cgi/devel-- Jeff Squyres Cisco Systems _______________________________________________ devel mailing list de...@open-mpi.org http://www.open-mpi.org/mailman/listinfo.cgi/devel-- Gleb. _______________________________________________ devel mailing list de...@open-mpi.org http://www.open-mpi.org/mailman/listinfo.cgi/devel
smime.p7s
Description: S/MIME cryptographic signature
