We talked about this on the phone yesterday.
Brian, Jeff, Terry, and Rolf were there. George was able to join for a short period of time.
George's objections seemed to be twofold (please correct if wrong!):
1. Why not change all 3 function pointers to take an additional (void*)? George misunderstood that the other 2 function pointers are mandated by MPI semantics and cannot be changed -- Brian is only proposing to change the one not-mandated-by-MPI function pointer signature that we have in ompi_request_t.
2. UTK has apparently implemented a different solution; they modified ompi_free_list_t to add an additional N bytes after each request that can be used for whatever purpose you want. This gets around the issue where an entity that wants to cache additional information on an ompi_request_t is not the entity that allocates the request, and therefore cannot do a "super" kind of trick that we usually use to cache additional information on a fixed struct. However, George alluded to some issues with reallocing lists of requests to make this work (it wasn't exactly clear what/how, but he did say it was during MPI_INIT before any requests are actually used), but this issue alone seems like a dealbreaker because the one-sided stuff is lazily loaded (i.e., after MPI_INIT) -- re-allocing any existing requests at this point is not possible because their pointer values will change.
So I *think* we came down to it being ok to add the extra (void*) into the 1 callback function signature that Brian was proposing (I'm not 100% sure because George had to leave early).
George -- Rich -- this is your last chance to object... Otherwise, I think we say it's ok for Brian to implement this whenever he gets to it (likely over the Christmas break).
On Nov 29, 2009, at 7:38 PM, Barrett, Brian W wrote:
> George --
> Sure. Since I had talked to you and Jeff about it a year ago (when you
> added the callback) and you didn't complain, I assumed you two would be the
> only ones to care and wouldn't complain this time. Guess I should have
> known better :).
> On 11/27/09 18:24 , "George Bosilca" <bosilca_at_[hidden]> wrote:
> > Brian,
> > This is a pretty big change to be done with a so short notice, especially over
> > the Thanksgiving weekend. I do have a lots of concerns about this approach,
> > but I lack the time to expand on this right now. I'll be back at work on
> > Monday and I'll give detailed informations. Please delay the deadline until at
> > least Wednesday.
> > Thanks,
> > george.
> > On Nov 25, 2009, at 11:52 , Barrett, Brian W wrote:
> >> WHAT: Add a void* extra_state field to ompi_request_t
> >> WHY: When we added the req_complete_cb field so that internal pieces of OMPI
> >> who generated requests (such as the OSC components using the PML) could be
> >> async notified when the request completed (ie, the PML request the OSC
> >> component had initiated was finished), we neglected to add any type of
> >> "extra state" associated with that request/callback. So the completion
> >> callback is almost worthless, because the upper layer has a hard time
> >> figuring out which thing it was working on it can now progress due to the
> >> given (lower?) request completing.
> >> WHERE: One line in each of ompi/request/request.[hc].
> >> WHEN: ASAP
> >> TIMEOUT: Sunday, Nov 29.
> >> More Details
> >> ------------
> >> This is probably not even worth an RFC, which is why I'm not giving a very
> >> long timeout (that, and if I don't get this done during the holiday weekend,
> >> it will never get done). The changes are a single line in request.h adding
> >> a void* extra_state variable to the ompi_request_t and another single line
> >> in request.c to initialize the field to NULL.
> >> While looking for some other code, I stumbled upon the OSC changes I made a
> >> long time ago to try to use req_complete_cb instead of registering a
> >> progress function. The code is actually a lot cleaner that way, and means
> >> no progress functions for the one-side components.
> >> The down side is that it adds another 8 bytes to ompi_request_t, which is
> >> already larger than I'd like. But on the flip side, we have an 8 byte field
> >> (the callback) which is totally unusable without the extra_state field.
> >> Brian
> >> _______________________________________________
> >> devel mailing list
> >> devel_at_[hidden]
> >> http://www.open-mpi.org/mailman/listinfo.cgi/devel
> > _______________________________________________
> > devel mailing list
> > devel_at_[hidden]
> > http://www.open-mpi.org/mailman/listinfo.cgi/devel
> Brian W. Barrett
> Dept. 1423: Scalable System Software
> Sandia National Laboratories
> devel mailing list