Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |  

This web mail archive is frozen.

This page is part of a frozen web archive of this mailing list.

You can still navigate around this archive, but know that no new mails have been added to it since July of 2016.

Click here to be taken to the new web archives of this list; it includes all the mails that are in this frozen archive plus all new mails that have been sent to the list since it was migrated to the new archives.

From: George Bosilca (bosilca_at_[hidden])
Date: 2007-10-18 06:47:01


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 are
> removed 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
>>> devel_at_[hidden]
>>> http://www.open-mpi.org/mailman/listinfo.cgi/devel
>>
>>
>> --
>> Jeff Squyres
>> Cisco Systems
>>
>> _______________________________________________
>> devel mailing list
>> devel_at_[hidden]
>> http://www.open-mpi.org/mailman/listinfo.cgi/devel
>
> --
> Gleb.
> _______________________________________________
> devel mailing list
> devel_at_[hidden]
> http://www.open-mpi.org/mailman/listinfo.cgi/devel



  • application/pkcs7-signature attachment: smime.p7s