I may be missing it, but it looks to me like the lock is taken right before the list manipulation. All he did was add a check for re-entry before taking the lock.
On Oct 22, 2013, at 8:39 AM, George Bosilca <bosi...@icl.utk.edu> wrote: > This patch is not correct, the list manipulation should remain protected > behind the mutex. > > George. > > On Oct 22, 2013, at 17:33 , svn-commit-mai...@open-mpi.org wrote: > >> Author: hjelmn (Nathan Hjelm) >> Date: 2013-10-22 11:33:39 EDT (Tue, 22 Oct 2013) >> New Revision: 29465 >> URL: https://svn.open-mpi.org/trac/ompi/changeset/29465 >> >> Log: >> Fix rentry check in communicator request progress. >> >> cmr=v1.7.4:ticket=3796 >> >> Text files modified: >> trunk/ompi/communicator/comm_request.c | 9 ++++++++- >> >> 1 files changed, 8 insertions(+), 1 deletions(-) >> >> Modified: trunk/ompi/communicator/comm_request.c >> ============================================================================== >> --- trunk/ompi/communicator/comm_request.c Tue Oct 22 11:33:32 2013 >> (r29464) >> +++ trunk/ompi/communicator/comm_request.c 2013-10-22 11:33:39 EDT (Tue, >> 22 Oct 2013) (r29465) >> @@ -11,6 +11,8 @@ >> >> #include "comm_request.h" >> >> +#include "opal/include/opal/sys/atomic.h" >> + >> opal_free_list_t ompi_comm_requests; >> opal_list_t ompi_comm_requests_active; >> opal_mutex_t ompi_comm_request_mutex; >> @@ -89,11 +91,15 @@ >> static int ompi_comm_request_progress (void) >> { >> ompi_comm_request_t *request, *next; >> + static int32_t progressing = 0; >> >> - if (opal_mutex_trylock (&ompi_comm_request_mutex)) { >> + /* don't allow re-entry */ >> + if (opal_atomic_swap_32 (&progressing, 1)) { >> return 0; >> } >> >> + opal_mutex_lock (&ompi_comm_request_mutex); >> + >> OPAL_LIST_FOREACH_SAFE(request, next, &ompi_comm_requests_active, >> ompi_comm_request_t) { >> int rc = OMPI_SUCCESS; >> >> @@ -140,6 +146,7 @@ >> } >> >> opal_mutex_unlock (&ompi_comm_request_mutex); >> + progressing = 0; >> >> return 1; >> } >> _______________________________________________ >> 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
