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.

Subject: Re: [OMPI devel] RFC: Add extra_state field to ompi_request_t
From: Jeff Squyres (jsquyres_at_[hidden])
Date: 2009-12-11 10:58:39

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" <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].
> >>
> >>
> >> 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]
> >>
> >
> >
> > _______________________________________________
> > devel mailing list
> > devel_at_[hidden]
> >
> >
> --
> Brian W. Barrett
> Dept. 1423: Scalable System Software
> Sandia National Laboratories
> _______________________________________________
> devel mailing list
> devel_at_[hidden]

Jeff Squyres