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 :). > > Brian > > > On 11/27/09 18:24 , "George Bosilca" <bosi...@eecs.utk.edu> 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 > >> 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 > > > > -- > Brian W. Barrett > Dept. 1423: Scalable System Software > Sandia National Laboratories > > > > _______________________________________________ > devel mailing list > de...@open-mpi.org > http://www.open-mpi.org/mailman/listinfo.cgi/devel > -- Jeff Squyres jsquy...@cisco.com
