Looking directly at the patch you are right, the lock is always taken after the atomic. The merge on my branch left both the try lock and lock out of the code without any markers for conflict (<<< … === … >>>). Strange.
Sorry for the noise. George. On Oct 22, 2013, at 17:46 , Ralph Castain <r...@open-mpi.org> wrote: > 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 > > _______________________________________________ > devel mailing list > de...@open-mpi.org > http://www.open-mpi.org/mailman/listinfo.cgi/devel
