| File | FIXME Note |
|---|---|
| src/mpi/comm/comm_group.c | Add a sanity check that the size of the group is the same as the size of the communicator. This helps catch corrupted communicators |
| src/mpi/comm/comm_split_type.c | this error code is wrong, it's the error code for regular MPI_Comm_split |
| src/mpi/comm/intercomm_create.c | A temporary version for lpids within my comm world |
| src/mpi/comm/intercomm_create.c | A temp for lpids within my comm world |
| src/mpi/comm/intercomm_create.c | for MPI1, all process ids are relative to MPI_COMM_WORLD. For MPI2, we'll need to do something more complex |
| src/mpi/comm/intercomm_create.c | Error |
| src/mpi/comm/intercomm_create.c | Should be using the local_size argument |
| src/mpi/comm/intercomm_merge.c | For the intracomm, we need a consistent context id. That means that one of the two groups needs to use the recvcontext_id and the other must use the context_id |
| src/mpi/comm/comm_create.c | BUBBLE SORT |
| src/mpi/comm/commutil.c | Reusing context ids can lead to a race condition if (as is desirable) MPI_Comm_free does not include a barrier. Consider the following: Process A frees the communicator. Process A creates a new communicator, reusing the just released id Process B sends a message to A on the old communicator. Process A receives the message, and believes that it belongs to the new communicator. Process B then cancels the message, and frees the communicator. The likelyhood of this happening can be reduced by introducing a gap between when a context id is released and when it is reused. An alternative is to use an explicit message (in the implementation of MPI_Comm_free) to indicate that a communicator is being freed; this will often require less communication than a barrier in MPI_Comm_free, and will ensure that no messages are later sent to the same communicator (we may also want to have a similar check when building fault-tolerant versions of MPI). |
| src/mpi/comm/commutil.c | this is an alternative constructor that doesn't use MPIR_Comm_create! |
| src/mpi/comm/commutil.c | No coll_fns functions for the collectives |
| src/mpi/comm/commutil.c | No local functions for the topology routines |
| src/mpi/comm/commutil.c | MT what protects access to this structure and ic_default_collops? |
| src/mpi/comm/commutil.c | we may be able to "rotate" this a half iteration up to the top where we already have to grab the lock |
| src/mpi/comm/commutil.c | we need an internal tag or communication channel. Can we use a different context instead?. Or can we use the tag provided in the intercomm routine? (not on a dup, but in that case it can use the collective context) |
| src/mpi/comm/commutil.c | do we want to copy coll_fns here? |
| src/mpi/init/async.c | We assume that waiting on some request forces progress on all requests. With fine-grained threads, will this still work as expected? We can imagine an approach where a request on a non-conflicting communicator would not touch the remaining requests to avoid locking issues. Once the fine-grained threads code is fully functional, we need to revisit this and, if appropriate, either change what we do in this thread, or delete this comment. |
| src/mpi/init/initthread.c | Move to os-dependent interface? |
| src/mpi/init/initthread.c | (Windows) This severly degrades performance but fixes alignment issues with the datatype code. |
| src/mpi/init/initthread.c | Define these in the interface. Does Timer init belong here? |
| src/mpi/init/initthread.c | Does this need to come before the call to MPID_InitComplete? For some debugger support, MPIR_WaitForDebugger may want to use MPI communication routines to collect information for the debugger |
| src/mpi/init/abort.c | 100 is arbitrary and may not be long enough |
| src/mpi/init/abort.c | It isn't clear that Abort should wait for a critical section, since that could result in the Abort hanging if another routine is hung holding the critical section. Also note the "not thread-safe" comment in the description of MPI_Abort above. |
| src/mpi/init/abort.c | This is not internationalized |
| src/mpi/init/finalize.c | Why is this not one of the finalize callbacks?. Do we need pre and post MPID_Finalize callbacks? |
| src/mpi/init/finalize.c | Should this also be a finalize callback? |
| src/mpi/init/finalize.c | Many of these debugging items could/should be callbacks, added to the finalize callback list |
| src/mpi/init/finalize.c | the memory tracing code block should be a finalize callback |
| src/mpi/init/finalize.c | will cause problems with MPIX_T_finalize when it is used after MPI_Finalize. The init/finalize paths in general need a big overhaul in order to account for the new MPIX_T_ code, and also to examine the feasibility of the proposed "allow calling init/finalize multiple times" in the MPI Forum. |
| src/mpi/init/finalize.c | We'd like to arrange for the mem dump output to go to separate files or to be sorted by rank (note that the rank is at the head of the line) |
| src/mpi/attr/comm_get_attr.c | We could initialize some of these here; only tag_ub is used in the error checking. |
| src/mpi/attr/comm_get_attr.c | Attributes must be visable from all languages |
| src/mpi/attr/attrutil.c | Missing routine description |
| src/mpi/attr/win_set_attr.c | communicator of window? |
| src/mpi/romio/mpi-io/get_extent.c | handle other file data representations |
| src/mpi/romio/mpi-io/seek_sh.c | explain why the barrier is necessary |
| src/mpi/romio/mpi-io/iotestall.c | THIS IS NOT CORRECT (see above). But most applications won't care |
| src/mpi/romio/mpi-io/get_view.c | It is wrong to use MPI_Type_contiguous; the user could choose to re-implement MPI_Type_contiguous in an unexpected way. Either use MPIR_Barrier_impl as in MPICH2 or PMPI_Type_contiguous |
| src/mpi/romio/mpi-io/get_view.c | Ditto for MPI_Type_commit - use NMPI or PMPI |
| src/mpi/romio/mpi-io/get_view.c | Ditto for MPI_Type_xxx - use NMPI or PMPI |
| src/mpi/romio/mpi-io/read_ord.c | Check for error code from ReadStridedColl? |
| src/mpi/romio/mpi-io/write_ordb.c | Check for error code from WriteStridedColl? |
| src/mpi/romio/mpi-io/write_alle.c | we should really ensure that the split_datatype remains valid by incrementing the ref count in the write_allb.c routine and decrement it here after setting the bytes |
| src/mpi/romio/mpi-io/write_ord.c | Check for error code from WriteStridedColl? |
| src/mpi/romio/mpi-io/glue/mpich2/mpio_err.c | These external prototypes should be included from mpich2/src/include/mpiext.h |
| src/mpi/romio/mpi-io/glue/mpich2/mpio_err.c | This is a hack in case no error handler was set |
| src/mpi/romio/adio/ad_ntfs/ad_ntfs_iwrite.c | Validate the args -- has it already been done by the caller ? |
| src/mpi/romio/adio/ad_ntfs/ad_ntfs_iwrite.c | Validate the args -- has it already been done by the caller ? |
| src/mpi/romio/adio/ad_ntfs/ad_ntfs_iwrite.c | Is the timeout in seconds ? |
| src/mpi/romio/adio/ad_ntfs/ad_ntfs_iwrite.c | Validate the args -- has it already been done by the caller ? |
| src/mpi/romio/adio/ad_ntfs/ad_ntfs_iwrite.c | Pass appropriate error code to user |
| src/mpi/romio/adio/ad_ntfs/ad_ntfs_iwrite.c | Pass appropriate error code to user |
| src/mpi/romio/adio/common/ad_init.c | should be checking error code from MPI_Info_create here |
| src/mpi/romio/adio/common/lock.c | This is a temporary hack until we use flock64 where available. It also doesn't fix the broken Solaris header sys/types.h header file, which declars off_t as a UNION ! Configure tests to see if the off64_t is a union if large file support is requested; if so, it does not select large file support. |
| src/mpi/romio/adio/common/lock.c | This should use the error message system, especially for MPICH2 |
| src/mpi/romio/adio/common/req_malloc.c | Insert error here |
| src/mpi/romio/adio/common/strfns.c | Really need a check for varargs.h vs stdarg.h |
| src/mpi/romio/adio/common/strfns.c | Assumes ASCII |
| src/mpi/topo/topoutil.c | thread safe code needs a thread lock here, followed by another test on the keyval to see if a different thread got there first |
| src/mpi/topo/topoutil.c | free the attribute data structure |
| src/mpi/topo/cart_create.c | nranks could be zero (?) |
| src/mpi/topo/dist_gr_neighb_count.c | Why does this routine require a CS? |
| src/mpi/topo/dims_create.c | This routine does not work well with non-powers of two (and probably with collections of different factors. For example, factoring squares like 6*6 or 7*7 in to 2 dimensions doesn't yield the expected results (18,2) and (49,1) in current tests |
| src/mpi/topo/dims_create.c | |
| src/mpi/topo/dist_gr_neighb.c | Why does this routine need a CS |
| src/mpi/datatype/register_datarep.c | UNIMPLEMENTED |
| src/mpi/datatype/type_match_size.c | This routine does not require the allfunc critical section |
| src/mpi/datatype/status_set_elements.c | The device may want something else here |
| src/mpi/datatype/typeutil.c | does the order of this list need to correspond to anything in particular? There are several lists of predefined types sprinkled throughout the codebase and it's unclear which (if any) of them must match exactly. [goodell@ 2009-03-17] |
| src/mpi/datatype/typeutil.c | This is actually an error, since this routine should only be called once |
| src/mpi/datatype/typeutil.c | This needs to be make thread safe |
| src/mpi/debugger/dbginit.c | In MPICH2, the executables may not have the information on the other processes; this is part of the Process Manager Interface (PMI). We need another way to provide this information to a debugger |
| src/mpi/debugger/dll_mpich2.c | include the mpichconf.h file? |
| src/mpi/debugger/dll_mpich2.c | handle size properly (declared as 64 in mpi_interface.h) |
| src/mpi/debugger/tvtest.c | Move these into dbgstub.c |
| src/mpi/timer/mpidtime.c | We need to cleanup the use of the MPID_Generic_wtick prototype |
| src/mpi/timer/mpidtime.c | This should have been fixed in the configure, rather than as a make-time error message |
| src/mpi/timer/mpidtime.c | We need to cleanup the use of the MPID_Generic_wtick prototype |
| src/mpi/errhan/file_set_errhandler.c | check for a valid file handle (fh) before converting to a pointer |
| src/mpi/errhan/file_set_errhandler.c | Is this obsolete now? |
| src/mpi/errhan/file_set_errhandler.c | We need an error return |
| src/mpi/errhan/file_set_errhandler.c | We need an error return |
| src/mpi/errhan/errutil.c | Now that error codes are chained, this does not produce a valid error code since there is no valid ring index corresponding to this code |
| src/mpi/errhan/errutil.c | We probably want to set this to MPI_ERR_UNKNOWN and discard the rest of the bits |
| src/mpi/errhan/errutil.c | Should this check against dynamically-created error classes? |
| src/mpi/errhan/errutil.c | These strings need to be internationalized |
| src/mpi/errhan/errutil.c | This should really be the same as MPI_MAX_ERROR_STRING, or in the worst case, defined in terms of that |
| src/mpi/errhan/errutil.c | Not internationalized |
| src/mpi/errhan/errutil.c | Factor the error routines so that there is a single version of each |
| src/mpi/errhan/errutil.c | Not internationalized |
| src/mpi/errhan/errutil.c | This routine isn't quite right yet |
| src/mpi/errhan/errutil.c | not internationalized |
| src/mpi/errhan/errutil.c | Why was the last test commented out? |
| src/mpi/errhan/errutil.c | Why was the last test commented out? |
| src/mpi/errhan/errutil.c | Where is the documentation for this function? What is it for? |
| src/mpi/errhan/errutil.c | This needs to be made consistent with the different thread levels, since in the "global" thread level, an extra thread mutex is not required. |
| src/mpi/errhan/errutil.c | These functions are not thread safe |
| src/mpi/errhan/errutil.c | default is not thread safe |
| src/mpi/errhan/errutil.c | Is MPI_UNDEFINED valid as a tag? |
| src/mpi/errhan/errutil.c | A check for MPI_IN_PLACE should only be used where that is valid |
| src/mpi/errhan/errutil.c | We may want to use 0x%p for systems that (including Windows) that don't prefix %p with 0x. This must be done with a capability, not a test on particular OS or header files |
| src/mpi/errhan/errutil.c | ERR_OTHER is overloaded; this may mean "OTHER" or it may mean "No additional error, just routine stack info" |
| src/mpi/errhan/errutil.c | Internal error. Generate some debugging information; Fix for the general release |
| src/mpi/errhan/errutil.c | This is wrong. The only way that you can get here without errcode being MPI_SUCCESS is if there is an error in the processing of the error codes. Dropping through into the next level of code (particularly when that code doesn't check for valid error codes!) is erroneous |
| src/mpi/errhan/errutil.c | Not internationalized |
| src/mpi/errhan/errutil.c | Shouldn't str be const char * ? - no, but you don't know that without some documentation |
| src/mpi/errhan/errutil.c | Don't use Snprint to append a string ! |
| src/mpi/errhan/errutil.c | The following code is broken as described above (if the errcode is not valid, then this code is just going to cause more problems) |
| src/mpi/errhan/errutil.c | (Here and elsewhere) Make sure any string is non-null before you use it |
| src/mpi/errhan/errutil.c | Not internationalized |
| src/mpi/errhan/errutil.c | How do we determine that we failed to unwind the stack? |
| src/mpi/errhan/add_error_code.c | verify that errorclass is a dynamic class |
| src/mpi/errhan/dynerrutil.c | Does this need a thread-safe init? |
| src/mpi/errhan/dynerrutil.c | Unallocated error code? |
| src/mpi/errhan/dynerrutil.c | Unallocated error code? |
| src/mpi/errhan/dynerrutil.c | For robustness, we should make sure that the associated string is initialized to null |
| src/mpi/errhan/file_get_errhandler.c | check for a valid file handle (fh) before converting to a pointer |
| src/mpi/errhan/file_get_errhandler.c | Is this obsolete now? |
| src/mpi/errhan/add_error_class.c | why isn't this done in MPIR_Err_add_class? |
| src/mpi/errhan/file_call_errhandler.c | check for a valid file handle (fh) before converting to a pointer |
| src/mpi/pt2pt/ibsend.c | There should be no unreferenced args! |
| src/mpi/pt2pt/ibsend.c | use the memory management macros |
| src/mpi/pt2pt/ibsend.c | should we be setting the request at all in the case of an error? |
| src/mpi/pt2pt/cancel.c | why do we directly access the partner request's cc field? shouldn't our cc_ptr be set to the address of the partner req's cc field? |
| src/mpi/pt2pt/bsend.c | Check for MPID_WOULD_BLOCK? |
| src/mpi/pt2pt/sendrecv_rep.c | should we cancel the pending (possibly completed) receive request or wait for it to complete? |
| src/mpi/pt2pt/mpir_request.c | are Ibsend requests added to the send queue? |
| src/mpi/pt2pt/mpir_request.c | MPIR_SENDQ_FORGET(request_ptr); -- it appears that persistent sends are not currently being added to the send queue. should they be, or should this release be conditional? |
| src/mpi/pt2pt/mpir_request.c | What is this routine for? [BRT] it is used by testall, although looking at testall now, I think the algorithm can be change slightly and eliminate the need for this routine |
| src/mpi/pt2pt/sendrecv.c | Performance for small messages might be better if MPID_Send() were used here instead of MPID_Isend() |
| src/mpi/pt2pt/sendrecv.c | should we cancel the pending (possibly completed) receive request or wait for it to complete? |
| src/mpi/pt2pt/bsendutil.c | Process pending bsend requests in detach |
| src/mpi/pt2pt/bsendutil.c | Unimplemented retry of pending bsend operations |
| src/mpi/pt2pt/greq_start.c | MT this function is not thread safe when using fine-grained threading |
| src/mpi/pt2pt/iprobe.c | Is this correct for intercomms? |
| src/mpi/coll/iscatter.c | how can this be right? shouldn't (sendcount*mask) be the amount sent and curr_cnt be reduced by that? Or is it always true the (curr_cnt/2==sendcount*mask)? |
| src/mpi/coll/ired_scat_block.c | does this algorithm actually work correctly for noncommutative ops? If so, relax restriction in assert and comments... |
| src/mpi/coll/ired_scat_block.c | this version only works for power of 2 procs |
| src/mpi/coll/igather.c | why are there two copies here? |
| src/mpi/coll/red_scat_block.c | should we be checking the op_errno here? |
| src/mpi/coll/red_scat_block.c | this version only works for power of 2 procs |
| src/mpi/coll/bcast.c | it would be nice if we could refactor things to minimize duplication between this and MPIR_Scatter_intra and friends. We can't use MPIR_Scatter_intra as is without inducing an extra copy in the noncontig case. |
| src/mpi/coll/bcast.c | This function uses some heuristsics based off of some testing on a cluster at Argonne. We need a better system for detrmining and controlling the cutoff points for these algorithms. If I've done this right, you should be able to make changes along these lines almost exclusively in this function and some new functions. [goodell@ 2008/01/07] |
| src/mpi/coll/bcast.c | binomial may not be the best algorithm for on-node bcast. We need a more comprehensive system for selecting the right algorithms here. |
| src/mpi/coll/bcast.c | binomial may not be the best algorithm for on-node bcast. We need a more comprehensive system for selecting the right algorithms here. |
| src/mpi/coll/bcast.c | It would be good to have an SMP-aware version of this algorithm that (at least approximately) minimized internode communication. |
| src/mpi/coll/red_scat.c | this version only works for power of 2 procs |
| src/mpi/coll/helper_fns.c | should we cancel the pending (possibly completed) receive request or wait for it to complete? |
| src/mpi/coll/helper_fns.c | For the brief-global and finer-grain control, we must ensure that the global lock is *not* held when this routine is called. (unless we change progress_start/end to grab the lock, in which case we must *still* make sure that the lock is not held when this routine is called). |
| src/mpi/coll/oputil.h | REAL need not be float, nor DOUBLE_PRECISION be double. Fortran types are not synonyms for the C types |
| src/mpi/coll/oputil.h | Is MPI_Fint really OK here? |
| src/mpi/coll/ired_scat.c | does this algorithm actually work correctly for noncommutative ops? If so, relax restriction in assert and comments... |
| src/mpi/coll/ired_scat.c | this version only works for power of 2 procs |
| src/mpi/coll/alltoall.c | This converts the Alltoall to a set of blocking phases. Two alternatives should be considered: 1) the choice of communication pattern could try to avoid contending routes in each phase 2) rather than wait for all communication to finish (waitall), we could maintain constant queue size by using waitsome and posting new isend/irecv as others complete. This avoids synchronization delays at the end of each block (when there are only a few isend/irecvs left) |
| src/mpi/coll/iallgather.c | saving an MPI_Aint into an int |
| src/mpi/coll/reduce.c | does this need to be checked after each uop invocation for predefined operators? |
| src/mpi/coll/allgather.c | saving an MPI_Aint into an int |
| src/mpi/coll/allgather.c | saving an MPI_Aint into an int |
| src/mpi/coll/iscan.c | just use roots_comm->rank instead |
| src/mpi/spawn/comm_disconnect.c | MT should we be checking this? |
| src/mpi/spawn/lookup_name.c | change **fail to something more meaningful |
| src/mpi/spawn/lookup_name.c | change **fail to something more meaningful |
| src/mpi/spawn/open_port.c | If info_ptr is non-null, we should validate it |
| src/mpi/rma/winutil.c | Move this into wincreate (having a separate file is unneccessary, and it leads to a false "missing coverage" report) |
| src/mpi/rma/win_unlock.c | Should this always be cleared, even on failure? |
| src/mpid/ch3/include/mpidpre.h | This header should contain only the definitions exported to the mpiimpl.h level |
| src/mpid/ch3/include/mpidpre.h | Who defines this name |
| src/mpid/ch3/include/mpidpre.h | we should really use the copy of these values that is stored in the MPID_Comm structure |
| src/mpid/ch3/include/mpidpre.h | This ifndef test is a temp until mpidpre is cleaned of all items that do not belong (e.g., all items not needed by the top layers of MPICH2) |
| src/mpid/ch3/include/mpidpre.h | The progress routines will be made into ch3-common definitions, not channel specific. Channels that need more will need to piggy back or otherwise override |
| src/mpid/ch3/include/mpidimpl.h | We want to avoid even storing information about the builtins if we can |
| src/mpid/ch3/include/mpidimpl.h | XXX DJG for TLS hack |
| src/mpid/ch3/include/mpidimpl.h | Why does a send request need the match information? Is that for debugging information? In case the initial envelope cannot be sent? Ditto for the dev.user_buf, count, and datatype fields when the data is sent eagerly. The following fields needed to be set: datatype_ptr status.MPI_ERROR Note that this macro requires that rank, tag, context_offset, comm, buf, datatype, and count all be available with those names (they are not arguments to the routine) |
| src/mpid/ch3/include/mpidimpl.h | We've moved to allow finer-grain critical sections... |
| src/mpid/ch3/include/mpidimpl.h | Determine which of these functions should be exported to all of the MPICH routines and which are internal to the device implementation |
| src/mpid/ch3/include/mpidimpl.h | This duplicates a value in util/sock/ch3usock.h |
| src/mpid/ch3/include/mpidimpl.h | Switch this to use the common debug code |
| src/mpid/ch3/include/mpidimpl.h | This does not belong here |
| src/mpid/ch3/include/mpidimpl.h | These are specific to the RMA code and should be in the RMA header file. |
| src/mpid/ch3/include/mpidimpl.h | These should be defined only when these particular utility packages are used. Best would be to keep these prototypes in the related util/xxx directories, and either copy them into an include directory used only for builds or add (yet another) include path |
| src/mpid/ch3/include/mpidimpl.h | Where should this go? |
| src/mpid/ch3/include/mpidimpl.h | This is a macro! |
| src/mpid/ch3/include/mpidimpl.h | Move these prototypes into header files in the appropriate util directories |
| src/mpid/ch3/include/mpidimpl.h | Make these part of the channel support headers |
| src/mpid/ch3/include/mpidimpl.h | do we need nest checking here? the existing macros won't work unmodified... |
| src/mpid/ch3/include/mpidimpl.h | do we need nest checking here? the existing macros won't work unmodified... |
| src/mpid/ch3/include/mpidimpl.h | incomplete? or already handled by the upper level? |
| src/mpid/ch3/include/mpidimpl.h | does it make sense to have (MPICH_IS_THREADED && _GRANULARITY==_SINGLE) ? |
| src/mpid/ch3/include/mpidpkt.h | This appears to assume that sizeof(int) == 4 (or at least >= 4) |
| src/mpid/ch3/include/mpidpkt.h | Having predefined names makes it harder to add new message types, such as different RMA types. |
| src/mpid/ch3/include/mpidpkt.h | no sync eager |
| src/mpid/ch3/include/mpidpkt.h | should be stream put |
| src/mpid/ch3/include/mpidpkt.h | Add PUT, GET_IMMED packet types |
| src/mpid/ch3/include/mpidpkt.h | Unused |
| src/mpid/ch3/include/mpidpkt.h | Experimental for now |
| src/mpid/ch3/include/mpidpkt.h | Compress datatype/op into a single word (immedate mode) |
| src/mpid/ch3/include/mpidpkt.h | do we need these (use a regular accum packet if we do?) |
| src/mpid/ch3/include/mpidpost.h | mpidpost.h is included by mpiimpl.h . However, mpiimpl.h should refer only to the ADI3 prototypes and should never include prototypes specific to any particular device. Factor the include files to maintain better modularity by providing mpiimpl.h with only the definitions that it needs |
| src/mpid/ch3/include/mpidrma.h | It would be better to setup the packet that will be sent, at least in most cases (if, as a result of the sync/ops/sync sequence, a different packet type is needed, it can be extracted from the information otherwise stored). |
| src/mpid/ch3/include/mpidrma.h | Use enum for RMA op type? |
| src/mpid/ch3/src/ch3u_comm_spawn_multiple.c | Place all of this within the mpid_comm_spawn_multiple file |
| src/mpid/ch3/src/ch3u_comm_spawn_multiple.c | We can avoid these two routines if we define PMI as using MPI info values |
| src/mpid/ch3/src/ch3u_comm_spawn_multiple.c | info may be needed for port name |
| src/mpid/ch3/src/ch3u_comm_spawn_multiple.c | cheating on constness |
| src/mpid/ch3/src/ch3u_comm_spawn_multiple.c | This is *really* awkward. We should either Fix on MPI-style info data structures for PMI (avoid unnecessary duplication) or add an MPIU_Info_getall(...) that creates the necessary arrays of key/value pairs |
| src/mpid/ch3/src/ch3u_comm_spawn_multiple.c | translate the pmi error codes here |
| src/mpid/ch3/src/ch3u_comm_spawn_multiple.c | What does this function do? Who calls it? Can we assume that it is called only dynamic process operations (specifically spawn) are supported? Do we need the concept of a port? For example, could a channel that supported only shared memory call this (it doesn't look like it right now, so this could go into util/sock, perhaps? It might make more sense to have this function provided as a function pointer as part of the channel init setup, particularly since this function appears to access channel-specific storage (MPIDI_CH3_Process) |
| src/mpid/ch3/src/ch3u_comm_spawn_multiple.c | DARIUS |
| src/mpid/ch3/src/mpidi_isend_self.c | Explain this function |
| src/mpid/ch3/src/mpidi_isend_self.c | should there be a simpler version of this for short messages, particularly contiguous ones? See also the FIXME about buffering short messages |
| src/mpid/ch3/src/mpidi_isend_self.c | Insert code here to buffer small sends in a temporary buffer? |
| src/mpid/ch3/src/mpid_init.c | This does not belong here |
| src/mpid/ch3/src/mpid_init.c | the PMI init function should ONLY do the PMI operations, not the process group or bc operations. These should be in a separate routine |
| src/mpid/ch3/src/mpid_init.c | This is a good place to check for environment variables and command line options that may control the device |
| src/mpid/ch3/src/mpid_init.c | Why are pg_size and pg_rank handled differently? |
| src/mpid/ch3/src/mpid_init.c | Why do we add a ref to pg here? |
| src/mpid/ch3/src/mpid_init.c | To allow just the "root" process to request the port and then use MPIR_Bcast_intra to distribute it to the rest of the processes, we need to perform the Bcast after MPI is otherwise initialized. We could do this by adding another MPID call that the MPI_Init(_thread) routine would make after the rest of MPI is initialized, but before MPI_Init returns. In fact, such a routine could be used to perform various checks, including parameter consistency value (e.g., all processes have the same environment variable values). Alternately, we could allow a few routines to operate with predefined parameter choices (e.g., bcast, allreduce) for the purposes of initialization. |
| src/mpid/ch3/src/mpid_init.c | Check that this intercommunicator gets freed in MPI_Finalize if not already freed. |
| src/mpid/ch3/src/mpid_init.c | Document this |
| src/mpid/ch3/src/mpid_init.c | We may want to allow the channel to ifdef out the use of PMI calls, or ask the channel to provide stubs that return errors if the routines are in fact used |
| src/mpid/ch3/src/mpid_init.c | We can allow the channels to tell the PG how to get connection information by passing the pg to the channel init routine |
| src/mpid/ch3/src/mpid_init.c | Who is this for and where does it belong? |
| src/mpid/ch3/src/mpid_init.c | has_args and has_env need to come from PMI eventually... |
| src/mpid/ch3/src/mpid_init.c | The PG code should supply these, since it knows how the pg_ids and other data are represented |
| src/mpid/ch3/src/mpidi_printf.c | What are these routines for? Who uses them? Why are they different from the src/util/dbg routines? |
| src/mpid/ch3/src/mpidi_printf.c | This "unreferenced_arg" is an example of a problem with the API (unneeded level argument) or the code (failure to check the level argument). Inserting these "unreference_arg" macros erroneously suggests that the code is correct with this ununsed argument, and thus commits the grave harm of obscuring a real problem |
| src/mpid/ch3/src/mpidi_printf.c | It would be much better if the routine to print packets used routines defined by packet type, making it easier to modify the handling of packet types (and allowing channels to customize the printing of packets). For example, an array of function pointers, indexed by packet type, could be used. Also, these routines should not use MPIU_DBG_PRINTF, instead they should us a simple fprintf with a style allowance (so that the style checker won't flag the use as a possible problem). This should switch to using a table of functions MPIDI_PktPrintFunctions[pkt->type](stdout,pkt); |
| src/mpid/ch3/src/mpidi_printf.c | Move these RMA descriptions into the RMA code files |
| src/mpid/ch3/src/ch3u_eagersync.c | This sometimes segfaults |
| src/mpid/ch3/src/ch3u_recvq.c | Recvq_lock/unlock removed because it is not needed for the SINGLE_CS approach and we might want a different, non-lock-based approach in a finer-grained thread-sync version. For example, some routines can be implemented in a lock-free fashion (because the user is required to guarantee non-conflicting accesses, such as doing a probe and a receive that matches in different threads). There are a lot of routines here. Do we really need them all? The search criteria can be implemented in a single 64 bit compare on systems with efficient 64-bit operations. The rank and contextid can also in many cases be combined into a single 32-bit word for the comparison (in which case the message info should be stored in the queue in a naturally aligned, 64-bit word. |
| src/mpid/ch3/src/ch3u_recvq.c | If this routine is only used by probe/iprobe, then we don't need to set the cancelled field in status (only set for nonblocking requests) |
| src/mpid/ch3/src/ch3u_recvq.c | Why doesn't this exit after it finds the first match? |
| src/mpid/ch3/src/mpid_startall.c | This bsend header shouldn't be needed (the function prototype should be in mpiimpl.h), to allow a devices MPID_Startall to use the MPIR_Bsend_isend function |
| src/mpid/ch3/src/mpid_startall.c | Consider using function pointers for invoking persistent requests; if we made those part of the public request structure, the top-level routine could implement startall unless the device wanted to study the requests and reorder them |
| src/mpid/ch3/src/mpid_startall.c | Where is the memory registration call in the init routines, in case the channel wishes to take special action (such as pinning for DMA) the memory? This was part of the design. |
| src/mpid/ch3/src/mpid_startall.c | The odd 7th arg (match.context_id - comm->context_id) is probably to get the context offset. Do we really need the context offset? Is there any case where the offset isn't zero? |
| src/mpid/ch3/src/mpid_startall.c | Move the routines that initialize the persistent requests into this file, since startall must be used with all of them |
| src/mpid/ch3/src/ch3u_handle_recv_req.c | MT: this has to be done atomically |
| src/mpid/ch3/src/ch3u_handle_recv_req.c | Temp to avoid SEGV when memory tracing |
| src/mpid/ch3/src/ch3u_handle_recv_req.c | MT: Must be done atomically |
| src/mpid/ch3/src/ch3u_handle_recv_req.c | MT: The setting of the lock type must be done atomically |
| src/mpid/ch3/src/ch3u_handle_recv_req.c | MT: All queue accesses need to be made atomic |
| src/mpid/ch3/src/ch3u_handle_recv_req.c | MT: Must be done atomically |
| src/mpid/ch3/src/ch3u_handle_recv_req.c | MT: The setting of the lock type must be done atomically |
| src/mpid/ch3/src/mpid_finalize.c | This routine needs to be factored into finalize actions per module, In addition, we should consider registering callbacks for those actions rather than direct routine calls. |
| src/mpid/ch3/src/mpid_finalize.c | The correct action here is to begin a shutdown protocol that lets other processes know that this process is now in finalize. Note that only requests that have been freed with MPI_Request_free are valid at this point; other pending receives can be ignored since a valid program should wait or test for them before entering finalize. The easist fix is to allow an MPI_Barrier over comm_world (and any connected processes in the MPI-2 case). Once the barrier completes, all processes are in finalize and any remaining unmatched receives will never be matched (by a correct program; a program with a send in a separate thread that continues after some thread calls MPI_Finalize is erroneous). Avoiding the barrier is hard. Consider this sequence of steps: Send in-finalize message to all connected processes. Include information on whether there are pending receives. (Note that a posted receive with any source is a problem) (If there are many connections, then this may take longer than the barrier) Allow connection requests from anyone who has not previously connected only if there is an possible outstanding receive; reject others with a failure (causing the source process to fail). Respond to an in-finalize message with the number of posted receives remaining. If both processes have no remaining receives, they can both close the connection. Processes with no pending receives and no connections can exit, calling PMI_Finalize to let the process manager know that they are in a controlled exit. Processes that still have open connections must then try to contact the remaining processes. August 2010: The barrier has been removed so that finalize won't hang when another processes has died. This allows processes to finalize and exit while other processes are still executing. This has the following consequences: * If a process tries to send a message to a process that has exited before completing the matching receive, it will now get an error. This is an erroneous program. * If a process finalizes before completing a nonblocking send, the message might not be sent. Similarly, if it finalizes before completing all receives, the sender may get an error. These are erroneous programs. * A process may isend to another process that has already terminated, then cancel the send. The program is not erroneous in this case, but this will result in an error. This can be fixed by not returning an error until the app completes the send request. If the app cancels the request, we need to to search the pending send queue and cancel it, in which case an error shouldn't be generated. |
| src/mpid/ch3/src/mpid_finalize.c | The close actions should use the same code as the other connection close code |
| src/mpid/ch3/src/ch3u_rma_sync.c | MT: this needs to be done atomically because other procs have the address and could decrement it. |
| src/mpid/ch3/src/ch3u_rma_sync.c | We could at least occassionally try to wait on completion of the pending send requests rather than focus on filling the queues. |
| src/mpid/ch3/src/ch3u_rma_sync.c | threshold as parameter |
| src/mpid/ch3/src/ch3u_rma_sync.c | Make this size check efficient and match the packet type |
| src/mpid/ch3/src/ch3u_rma_sync.c | This is still very heavyweight for a small message operation, such as a single word update |
| src/mpid/ch3/src/ch3u_rma_sync.c | Short messages like this shouldn't normally need a request - this should consider using the ch3 call to send a short message and return a request only if the message is not delivered. |
| src/mpid/ch3/src/ch3u_rma_sync.c | This is a heavyweight way to process these sync messages - this should be handled with a special packet type and callback function. |
| src/mpid/ch3/src/ch3u_rma_sync.c | MT: this has to be done atomically |
| src/mpid/ch3/src/ch3u_rma_sync.c | Error return |
| src/mpid/ch3/src/ch3u_rma_sync.c | MT: this has to be done atomically |
| src/mpid/ch3/src/ch3u_rma_sync.c | Only change the handling of completion if post_data_receive reset the handler. There should be a cleaner way to do this |
| src/mpid/ch3/src/ch3u_rma_sync.c | Only change the handling of completion if post_data_receive reset the handler. There should be a cleaner way to do this |
| src/mpid/ch3/src/ch3u_rma_sync.c | use immediate copy because this is short |
| src/mpid/ch3/src/ch3u_rma_sync.c | MT: this has to be done atomically |
| src/mpid/ch3/src/ch3u_rma_sync.c | MT: This may need to be done atomically. |
| src/mpid/ch3/src/ch3u_rma_sync.c | Since we need to add to the tail of the list, we should maintain a tail pointer rather than traversing the list each time to find the tail. |
| src/mpid/ch3/src/ch3u_rma_sync.c | MT: The queuing may need to be done atomically. |
| src/mpid/ch3/src/ch3u_rma_sync.c | MT: This may need to be done atomically. |
| src/mpid/ch3/src/ch3u_rma_sync.c | MT: The queuing may need to be done atomically. |
| src/mpid/ch3/src/ch3u_rma_sync.c | Only change the handling of completion if post_data_receive reset the handler. There should be a cleaner way to do this |
| src/mpid/ch3/src/ch3u_rma_sync.c | It is likely that this cannot happen (never perform a get with a 0-sized item). In that case, change this to an MPIU_Assert (and do the same for accumulate and put) |
| src/mpid/ch3/src/mpid_getpname.c | Make thread safe |
| src/mpid/ch3/src/ch3u_handle_connection.c | Decrement the reference count? Who increments? |
| src/mpid/ch3/src/ch3u_handle_connection.c | The reference count is often already 0. But not always |
| src/mpid/ch3/src/ch3u_handle_connection.c | Who increments the reference count that this is decrementing? |
| src/mpid/ch3/src/ch3u_handle_connection.c | This should be done when the reference count of the vc is first decremented |
| src/mpid/ch3/src/ch3u_handle_connection.c | Debugging |
| src/mpid/ch3/src/ch3u_handle_connection.c | This won't work for dynamic procs |
| src/mpid/ch3/src/ch3u_handle_connection.c | Currently this only handles failed processes in comm_world. We need to fix hydra to include the pgid along with the rank, then we need to create the failed group from something bigger than comm_world. |
| src/mpid/ch3/src/ch3u_port.c | If dynamic processes are not supported, this file will contain no code and some compilers may warn about an "empty translation unit" |
| src/mpid/ch3/src/ch3u_port.c | pg_translation is used for ? |
| src/mpid/ch3/src/ch3u_port.c | Describe the algorithm used here, and what routine is user on the other side of this connection |
| src/mpid/ch3/src/ch3u_port.c | This code is still broken for the following case: If the same process opens connections to the multiple processes, this context ID might get out of sync. |
| src/mpid/ch3/src/ch3u_port.c | we probably need a unique context_id. |
| src/mpid/ch3/src/ch3u_port.c | Explain why |
| src/mpid/ch3/src/ch3u_port.c | Why do we do a dup here? |
| src/mpid/ch3/src/ch3u_port.c | Error, the pg_list is broken |
| src/mpid/ch3/src/ch3u_port.c | We should use the PG destroy function for this, and ensure that the PG fields are valid for that function |
| src/mpid/ch3/src/ch3u_port.c | why is this commented out? |
| src/mpid/ch3/src/ch3u_port.c | How much of this could/should be common with the upper level (src/mpi/comm/ *.c) code? For best robustness, this should use the same routine (not copy/paste code) as in the upper level code. |
| src/mpid/ch3/src/ch3u_port.c | The free and the create routines should be in the same file |
| src/mpid/ch3/src/ch3u_port.c | What is an Accept queue and who uses it? Is this part of the connect/accept support? These routines appear to be called by channel progress routines; perhaps this belongs in util/sock (note the use of a port_name_tag in the dequeue code, though this could be any string). Are the locks required? If this is only called within the progress engine, then the progress engine locks should be sufficient. If a finer grain lock model is used, it needs to be very carefully designed and documented. |
| src/mpid/ch3/src/ch3u_port.c | Use CHKPMEM |
| src/mpid/ch3/src/ch3u_port.c | Stack or queue? |
| src/mpid/ch3/src/mpid_abort.c | Who uses/sets MPIDI_DEV_IMPLEMENTS_ABORT? |
| src/mpid/ch3/src/mpid_abort.c | We should move this into a header file so that we don't need the ifdef. Also, don't use exit (add to coding check) since not safe in windows. To avoid confusion, define a RobustExit? or MPIU_Exit? |
| src/mpid/ch3/src/mpid_abort.c | This routine *or* MPI_Abort should provide abort callbacks, similar to the support in MPI_Finalize |
| src/mpid/ch3/src/mpid_abort.c | Do we want the rank of the input communicator here or the rank of comm world? The message gives the rank but not the communicator, so using other than the rank in comm world does not identify the process, as the message suggests |
| src/mpid/ch3/src/mpid_abort.c | Not internationalized |
| src/mpid/ch3/src/mpid_abort.c | Not internationalized |
| src/mpid/ch3/src/mpid_abort.c | This should not use an ifelse chain. Either define the function by name or set a function pointer |
| src/mpid/ch3/src/mpid_abort.c | What is the scope for PMI_Abort? Shouldn't it be one or more process groups? Shouldn't abort of a communicator abort either the process groups of the communicator or only the current process? Should PMI_Abort have a parameter for which of these two cases to perform? |
| src/mpid/ch3/src/mpid_ssend.c | HOMOGENEOUS SYSTEMS ONLY -- no data conversion is performed |
| src/mpid/ch3/src/mpidi_pg.c | These routines need a description. What is their purpose? Who calls them and why? What does each one do? |
| src/mpid/ch3/src/mpidi_pg.c | straighten out the use of PMI_Finalize - no use after PG_Finalize |
| src/mpid/ch3/src/mpidi_pg.c | This routine needs to make it clear that the pg_id, for example is saved; thus, if the pg_id is a string, then that string is not copied and must be freed by a PG_Destroy routine |
| src/mpid/ch3/src/mpidi_pg.c | it would be good if we could make this assertion. Unfortunately, either: 1) We're not being disciplined and some caller of this function doesn't bother to manage all the refcounts because he thinks he knows better. Annoying, but not strictly a bug. (wdg - actually, that is a bug - managing the ref counts IS required and missing one is a bug.) 2) There is a real bug lurking out there somewhere and we just haven't hit it in the tests yet. |
| src/mpid/ch3/src/mpidi_pg.c | What does DEV_IMPLEMENTS_KVS mean? Why is it used? Who uses PG_To_string and why? |
| src/mpid/ch3/src/mpidi_pg.c | This is a temporary hack for devices that do not define MPIDI_DEV_IMPLEMENTS_KVS FIXME: MPIDI_DEV_IMPLEMENTS_KVS should be removed |
| src/mpid/ch3/src/mpidi_pg.c | This is a hack to avoid including shared-memory queue names in the business card that may be used by processes that were not part of the same COMM_WORLD. To fix this, the shared memory channels should look at the returned connection info and decide whether to use sockets or shared memory by determining whether the process is in the same MPI_COMM_WORLD. |
| src/mpid/ch3/src/mpidi_pg.c | The more general problem is that the connection information needs to include some information on the range of validity (e.g., all processes, same comm world, particular ranks), and that representation needs to be scalable |
| src/mpid/ch3/src/mpidi_pg.c | this should be a failure to call this routine |
| src/mpid/ch3/src/mpidi_pg.c | Turn this into a valid error code create/return |
| src/mpid/ch3/src/mpidi_pg.c | This is a hack, and it doesn't even work |
| src/mpid/ch3/src/mpidi_pg.c | MT: This should be a fetch and increment for thread-safety |
| src/mpid/ch3/src/mpidi_pg.c | This routine should invoke a close method on the connection, rather than have all of the code here |
| src/mpid/ch3/src/mpidi_pg.c | If the reference count for the vc is not 0, something is wrong |
| src/mpid/ch3/src/ch3u_request.c | This makes request creation expensive. We need to trim this to the basics, with additional setup for special-purpose requests (think base class and inheritance). For example, do we really* want to set the kind to UNDEFINED? And should the RMA values be set only for RMA requests? |
| src/mpid/ch3/src/ch3u_request.c | status fields meaningful only for receive, and even then should not need to be set. |
| src/mpid/ch3/src/ch3u_request.c | RMA ops shouldn't need to be set except when creating a request for RMA operations |
| src/mpid/ch3/src/ch3u_request.c | This fails to fail if debugging is turned off |
| src/mpid/ch3/src/ch3u_request.c | We need a lighter-weight version of this to avoid all of the extra checks. One posibility would be a single, no special case (no comm, datatype, or srbuf to check) and a more general (check everything) version. |
| src/mpid/ch3/src/ch3u_request.c | We need a better way to handle these so that we do not always need to initialize these fields and check them when we destroy a request |
| src/mpid/ch3/src/ch3u_request.c | We need a way to call these routines ONLY when the related ref count has become zero. |
| src/mpid/ch3/src/ch3u_request.c | we should drain the data off the pipe here, but we don't have a buffer to drain it into. should this be a fatal error? |
| src/mpid/ch3/src/mpid_cancel_send.c | This should call a channel-provided routine to deliver the cancel message, once the code decides that the request can still be cancelled |
| src/mpid/ch3/src/mpid_cancel_send.c | should be a decr and assert, not a set |
| src/mpid/ch3/src/mpid_cancel_send.c | should be a decr and assert, not a set |
| src/mpid/ch3/src/mpid_cancel_send.c | should be a decr and assert, not a set |
| src/mpid/ch3/src/mpid_cancel_send.c | if send cancellation packets are allowed to arrive out-of-order with respect to send packets, then we need to timestamp send and cancel packets to insure that a cancellation request does not bypass the send packet to be cancelled and erroneously cancel a previously sent message with the same request handle. |
| src/mpid/ch3/src/mpid_cancel_send.c | A timestamp is more than is necessary; a message sequence number should be adequate. |
| src/mpid/ch3/src/mpid_cancel_send.c | Note that this routine is called from within the packet handler. If the message queue mutex is different from the progress mutex, this must be protected within a message-queue mutex |
| src/mpid/ch3/src/mpid_cancel_send.c | This is called within the packet handler |
| src/mpid/ch3/src/mpid_comm_spawn_multiple.c | Correct description of function |
| src/mpid/ch3/src/mpid_send.c | HOMOGENEOUS SYSTEMS ONLY -- no data conversion is performed |
| src/mpid/ch3/src/mpid_send.c | flow control: limit number of outstanding eager messsages containing data and need to be buffered by the receiver |
| src/mpid/ch3/src/mpid_irsend.c | HOMOGENEOUS SYSTEMS ONLY -- no data conversion is performed |
| src/mpid/ch3/src/mpid_rsend.c | HOMOGENEOUS SYSTEMS ONLY -- no data conversion is performed |
| src/mpid/ch3/src/mpid_rsend.c | How does this differ from eager send? It should differ in only a few bits (e.g., indicate that the send is ready and should fail if there is no matching receive) |
| src/mpid/ch3/src/ch3u_handle_send_req.c | Should this test be an MPIU_Assert? |
| src/mpid/ch3/src/ch3u_handle_send_req.c | MT: this has to be done atomically |
| src/mpid/ch3/src/ch3u_eager.c | Why do we set the message type? |
| src/mpid/ch3/src/ch3u_eager.c | We want to set the OnDataAvail to the appropriate function, which depends on whether this is an RMA request or a pt-to-pt request. |
| src/mpid/ch3/src/ch3u_eager.c | The MPICH2 tests do not exercise this branch |
| src/mpid/ch3/src/ch3u_eager.c | the AEU branch gives (rreq->cc==1) but the complete at the bottom of this function will decr it. Is everything going to be cool in this case? No upper layer has a pointer to rreq yet (it's unexpected and freshly allocated) |
| src/mpid/ch3/src/ch3u_eager.c | When eagershort is enabled, provide a preallocated space for short messages (which is used even if eager short is not used), since we don't want to have a separate check to figure out which buffer we're using (or perhaps we should have a free-buffer-pointer, which can be null if it isn't a buffer that we've allocated). |
| src/mpid/ch3/src/ch3u_eager.c | This is not optimized for short messages, which should have the data in the same packet when the data is particularly short (e.g., one 8 byte long word) |
| src/mpid/ch3/src/ch3u_eager.c | an error packet should be sent back to the sender indicating that the ready-send failed. On the send side, the error handler for the communicator can be invoked even if the ready-send request has already completed. |
| src/mpid/ch3/src/ch3u_rndv.c | fill temporary IOV or pack temporary buffer after send to hide some latency. This requires synchronization because the CTS packet could arrive and be processed before the above iStartmsg completes (depending on the progress engine, threads, etc.). |
| src/mpid/ch3/src/ch3u_rndv.c | What if the receive user buffer is not big enough to hold the data about to be cleared for sending? |
| src/mpid/ch3/src/ch3u_rndv.c | Ideally we could specify that a req not be returned. This would avoid our having to decrement the reference count on a req we don't want/need. |
| src/mpid/ch3/src/ch3u_rma_ops.c | There should be no unreferenced args |
| src/mpid/ch3/src/ch3u_rma_ops.c | This needs to be fixed for heterogeneous systems |
| src/mpid/ch3/src/ch3u_rma_ops.c | If we wanted to validate the transfer as within range at the origin, we'd also need the window size. |
| src/mpid/ch3/src/ch3u_rma_ops.c | For short operations, should we use a (per-thread) pool? |
| src/mpid/ch3/src/ch3u_rma_ops.c | For contig and very short operations, use a streamlined op |
| src/mpid/ch3/src/ch3u_rma_ops.c | For contig and very short operations, use a streamlined op |
| src/mpid/ch3/src/mpid_issend.c | HOMOGENEOUS SYSTEMS ONLY -- no data conversion is performed |
| src/mpid/ch3/src/mpid_issend.c | fill temporary IOV or pack temporary buffer after send to hide some latency. This requires synchronization because the CTS packet could arrive and be processed before the above iStartmsg completes (depending on the progress engine, threads, etc.). |
| src/mpid/ch3/src/ch3u_buffer.c | Explain the contents of this file |
| src/mpid/ch3/src/ch3u_buffer.c | Explain this routine . Used indirectly by mpid_irecv, mpid_recv (through MPIDI_CH3_RecvFromSelf) and in mpidi_isend_self.c |
| src/mpid/ch3/src/ch3u_buffer.c | DJG really? shouldn't we at least decr+assert? |
| src/mpid/ch3/src/mpid_recv.c | in the common case, we want to simply complete the message and make as few updates as possible. Note in addition that this routine is used only by MPI_Recv (a blocking routine; the intent of the interface (which returns a request) was to simplify the handling of the case where the message was not found in the unexpected queue. |
| src/mpid/ch3/src/mpid_recv.c | We do not need to add a datatype reference if the request is blocking. This is currently added because of the actions that are taken when a request is freed. (specifically, the datatype and comm both have their refs decremented, and are freed if the refs are zero) |
| src/mpid/ch3/src/ch3u_comm.c | This algorithm assumes that the number of processes in group is very small (like 1). So doing a linear search for them in comm is better than sorting the procs in comm and group then doing a binary search |
| src/mpid/ch3/src/ch3u_comm.c | This won't work for dynamic procs |
| src/mpid/ch3/src/mpid_vc.c | What should this do? See proc group and vc discussion |
| src/mpid/ch3/src/mpid_vc.c | Need a better way to define how vc's are closed that takes into account pending operations on vcs, including close events received from other processes. |
| src/mpid/ch3/src/mpid_vc.c | this is still bogus, the VCRT may contain a mix of dynamic and non-dynamic VCs, so the ref_count isn't guaranteed to have started at 2. The best thing to do might be to avoid overloading the reference counting this way and use a separate check for dynamic VCs (another flag? compare PGs?) |
| src/mpid/ch3/src/mpid_vc.c | the correct test is ACTIVE or REMOTE_CLOSE |
| src/mpid/ch3/src/mpid_vc.c | These routines belong in a different place |
| src/mpid/ch3/src/mpid_vc.c | a quick check on the min/max values of the lpid for this process group could help speed this search |
| src/mpid/ch3/src/mpid_vc.c | For testing, we just test in place |
| src/mpid/ch3/src/mpid_vc.c | We need a cleaner way to handle this case than using an ifdef. We could have an empty version of MPID_PG_BCast in ch3u_port.c, but that's a rather crude way of addressing this problem. Better is to make the handling of local and remote PIDS for the dynamic process case part of the dynamic process "module"; devices that don't support dynamic processes (and hence have only COMM_WORLD) could optimize for that case |
| src/mpid/ch3/src/mpid_vc.c | Printf for debugging |
| src/mpid/ch3/src/mpid_vc.c | We need a better abstraction for initializing the thread state for an object |
| src/mpid/ch3/src/mpid_vc.c | this routine can't handle the dynamic process case at this time. This will require more support from the process manager. |
| src/mpid/ch3/src/mpid_vc.c | 'PMI_process_mapping' only applies for the original PG (MPI_COMM_WORLD) |
| src/mpid/ch3/src/mpid_vc.c | need a better algorithm -- this one does O(N^2) strncmp()s! |
| src/mpid/ch3/src/mpid_port.c | We should instead ask the mpid_pg routines to give us a connection string. There may need to be a separate step to restrict us to a connection information that is only valid for connections between processes that are started separately (e.g., may not use shared memory). We may need a channel-specific function to create an exportable connection string. |
| src/mpid/ch3/src/mpid_isend.c | HOMOGENEOUS SYSTEMS ONLY -- no data conversion is performed |
| src/mpid/ch3/src/mpid_isend.c | The routines MPID_Isend, MPID_Issend, MPID_Irsend are nearly identical. It would be better if these did roughly: MPID_Irsend -> always eager send (with ready mode for error detection) MPID_Issend -> always rendezvous send MPID_Isend -> chose eager/rendezvous based on a threshold (and consider making the threshold configurable at either compile time (for best low-latency performance) or run-time (for application tuning). Then the 3 routines share little code, particularly if the eager/rendezvous implementations are in their own routine |
| src/mpid/ch3/src/mpid_isend.c | flow control: limit number of outstanding eager messsages containing data and need to be buffered by the receiver |
| src/mpid/ch3/src/mpid_isend.c | fill temporary IOV or pack temporary buffer after send to hide some latency. This requires synchronization because the CTS packet could arrive and be processed before the above iStartmsg completes (depending on the progress engine, threads, etc.). |
| src/mpid/ch3/src/mpid_iprobe.c | The routine CH3U_Recvq_FU is used only by the probe functions; it should atomically return the flag and status rather than create a request. Note that in some cases it will be possible to atomically query the unexpected receive list (which is what the probe routines are for). |
| src/mpid/ch3/src/mpid_iprobe.c | It would be helpful to know if the Progress_poke operation causes any change in state; we could then avoid a second test of the receive queue if we knew that nothing had changed |
| src/mpid/ch3/src/mpid_comm_disconnect.c | How can we get to a state where we are still connecting a VC but the MPIR_Comm_release will find that the ref count decrements to zero (it may be that some operation fails to increase/decrease the reference count. A patch could be to increment the reference count while connecting, then decrement it. But the increment in the reference count should come from the step that caused the connection steps to be initiated. Possibility: if the send queue is not empty, the ref count should be higher. |
| src/mpid/ch3/src/mpid_comm_disconnect.c | This doesn't work yet |
| src/mpid/ch3/src/mpid_comm_disconnect.c | Describe what more might be required |
| src/mpid/ch3/src/ch3u_handle_recv_pkt.c | We can turn this into something like MPIU_Assert(pkt->type <= MAX_PACKET_TYPE); mpi_errno = MPIDI_CH3_ProgressFunctions[pkt->type](vc,pkt,rreqp); in the progress engine itself. Then this routine is not necessary. |
| src/mpid/ch3/src/ch3u_handle_recv_pkt.c | We want to set the OnDataAvail to the appropriate function, which depends on whether this is an RMA request or a pt-to-pt request. |
| src/mpid/ch3/src/ch3u_handle_recv_pkt.c | Set OnDataAvail to 0? If not, why not? |
| src/mpid/ch3/src/ch3u_handle_recv_pkt.c | to improve performance, allocate temporary buffer from a specialized buffer pool. |
| src/mpid/ch3/src/ch3u_handle_recv_pkt.c | to avoid memory exhaustion, integrate buffer pool management with flow control |
| src/mpid/ch3/src/ch3u_handle_recv_pkt.c | We want to set the OnDataAvail to the appropriate function, which depends on whether this is an RMA request or a pt-to-pt request. |
| src/mpid/ch3/src/ch3u_handle_recv_pkt.c | to improve performance, allocate temporary buffer from a specialized buffer pool. |
| src/mpid/ch3/src/ch3u_handle_recv_pkt.c | to avoid memory exhaustion, integrate buffer pool management with flow control |
| src/mpid/ch3/src/ch3u_handle_recv_pkt.c | we still need to implement flow control. As a reminder, we don't mark these parameters as unused, because a full implementation of this routine will need to make use of all 4 parameters |
| src/mpid/ch3/src/ch3u_handle_recv_pkt.c | This should be initialized by a separate, RMA routine. That would allow different RMA implementations. We could even do lazy initialization (make this part of win_create) |
| src/mpid/ch3/util/unordered/unordered.c | This should probably be *rreqp = NULL? |
| src/mpid/ch3/util/unordered/unordered.c | processing send cancel requests requires that we be aware of pkts in the reorder queue |
| src/mpid/ch3/util/sock/findinterfaces.c | THIS IS WRONG. INSTEAD, SPECIFY THE SPECIFIC FEATURE LEVEL NEEDED, AND THEN ONLY IF A CONFIGURE TEST SHOWS THAT IT IS REQUIRED THESE NEED TO BE SET FOR ALL COMPILATIONS TO AVOID HAVING DIFFERENT FILES COMPILED WITH DIFFERENT AND INCOMPATIBLE HEADER FILES. WHAT IS APPARENTLY NEEDED (SEE /usr/include/features.h ON A LINUX SYSTEM) IS EITHER _BSD_SOURCE OR _SVID_SOURCE; THIS MUST BE SET CONSISTENTLY FOR ALL FILES COMPILED AS PART OF MPICH2 |
| src/mpid/ch3/util/sock/ch3u_init_sock.c | Why are these unused? |
| src/mpid/ch3/util/sock/ch3u_init_sock.c | Get the size from the process group |
| src/mpid/ch3/util/sock/ch3u_init_sock.c | This should probably be the same as MPIDI_VC_InitSock. If not, why not? |
| src/mpid/ch3/util/sock/ch3u_init_sock.c | Note that MPIDI_CH3_VC_Init sets state, sendq_head and tail. so this should be MPIDI_CH3_VC_Init( &pg_p->vct[p] ); followed by MPIDI_VC_InitSock( ditto ); In fact, there should be a single VC_Init call here |
| src/mpid/ch3/util/sock/ch3u_init_sock.c | Why isn't this MPIDI_VC_Init( vc, NULL, 0 )? |
| src/mpid/ch3/util/sock/ch3u_init_sock.c | This doesn't belong here, since the pg is not created in this routine |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | We need a little security here to avoid having a random port scan crash the process. Perhaps a "secret" value for each process could be published in the key-val space and subsequently sent in the open pkt. |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | Describe what these routines do |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | Clean up use of private packets (open/accept) |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | This size is unchanging, so get it only once (at most); we might prefer for connections to simply point at the single process group to which the remote process belong |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | Why does the name include "to_root"? |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | Describe the algorithm for the connection logic |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | where does this vc get freed? |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | There may need to be an additional routine here, to ensure that the channel is initialized for this pair of process groups (this process and the remote process to which the vc will connect). |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | To avoid this global (MPIDI_CH3I_sock_set) which is used only in ch3_progress.c and ch3_progress_connect.c in the channels, this should be a call into the channel, asking it to setup the socket for a connection and return the connection. That will keep the socket set out of the general ch3 code, even if this is the socket utility functions. |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | These are small routines; we may want to bring them together into a more specific post-connection-for-sock |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | This is a hack to allow Windows to continue to use the host description string instead of the interface address bytes when posting a socket connection. This should be fixed by changing the Sock_post_connect to only accept interface address. Note also that Windows does not have the inet_pton routine; the Windows version of this routine will need to be identified or written. See also channels/sock/ch3_progress.c |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | We should start switching to getaddrinfo instead of gethostbyname |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | We don't make use of the ifname in Windows in order to provide backward compatibility with the (undocumented) host description string used by the socket connection routine MPIDU_Sock_post_connect. We need to change to an interface-address (already resolved) based description for better scalability and to eliminate reliance on fragile DNS services. Note that this is also more scalable, since the DNS server may serialize address requests. On most systems, asking for the host info of yourself is resolved locally (i.e., perfectly parallel). Regrettably, not all systems do this (e.g., some versions of FreeBSD). |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | What does the above comment mean? |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | separate out the accept and connect sides to make it easier to follow the logic |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | Is there an assumption about conn->state? |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | where does this vc get freed? |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | Possible ambiguous state (two ways to get to OPEN_LSEND) |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | is this the correct assert? |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | Should conn->vc be freed? Who allocated? Why not? |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | Should probably reduce ref count on conn->vc |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | What happens to the state of the associated VC? Why isn't it changed? Is there an assert here, such as conn->vc->conn != conn (there is another connection chosen for the vc)? |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | What does post close do here? |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | This should really be combined with handle_conn_event |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | This routine is called when? What is valid in conn? |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | the connect side of this sets conn->vc to NULL. Why is this different? The code that checks CONN_STATE_CLOSING uses conn == NULL to identify intentional close, which this appears to be. |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | What does this do? |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | This is a hack to allow Windows to continue to use the host description string instead of the interface address bytes when posting a socket connection. This should be fixed by changing the Sock_post_connect to only accept interface address. |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | What does this do? |
| src/mpid/ch3/util/sock/ch3u_connect_sock.c | This function also used in channels/sock/src/ch3_progress.c |
| src/mpid/ch3/channels/sock/src/ch3_progress.c | Move thread stuff into some set of abstractions in order to remove ifdefs |
| src/mpid/ch3/channels/sock/src/ch3_progress.c | Was (USE_THREAD_IMPL == MPICH_THREAD_IMPL_NOT_IMPLEMENTED), which really meant not-using-global-mutex-thread model . This was true for the single threaded case, but was probably not intended for that case |
| src/mpid/ch3/channels/sock/src/ch3_progress.c | should be appropriately abstracted somehow |
| src/mpid/ch3/channels/sock/src/ch3_progress.c | Cleanly shutdown other socks and free connection structures. (close protocol?) |
| src/mpid/ch3/channels/sock/src/ch3_progress.c | should be appropriately abstracted somehow |
| src/mpid/ch3/channels/sock/src/ch3_progress.c | the following should be handled by the close protocol |
| src/mpid/ch3/channels/sock/src/ch3_progress.c | Test for runtime thread level (here or where used) |
| src/mpid/ch3/channels/sock/src/ch3_progress.c | should be appropriately abstracted somehow |
| src/mpid/ch3/channels/sock/src/ch3_progress.c | Test for runtime thread level |
| src/mpid/ch3/channels/sock/src/ch3_progress.c | should be appropriately abstracted somehow |
| src/mpid/ch3/channels/sock/src/ch3_progress.c | (a) what does this do and where is it used and (b) we could replace it with a #define for the single-method case |
| src/mpid/ch3/channels/sock/src/ch3_progress.c | This function also used in ch3u_connect_sock.c |
| src/mpid/ch3/channels/sock/src/ch3_progress.c | Is dequeue/get next the operation we really want? |
| src/mpid/ch3/channels/sock/src/ch3_progress.c | What is this routine for? |
| src/mpid/ch3/channels/sock/src/ch3_isendv.c | the current code only agressively writes the first IOV. Eventually it should be changed to agressively write as much as possible. Ideally, the code would be shared between the send routines and the progress engine. |
| src/mpid/ch3/channels/sock/src/ch3_isendv.c | Shouldn't the vc->state also change? |
| src/mpid/ch3/channels/sock/src/ch3_isend.c | Shouldn't the vc->state also change? |
| src/mpid/ch3/channels/sock/include/mpidi_ch3_impl.h | Any of these used in the ch3->channel interface should be defined in a header file in ch3/include that defines the channel interface |
| src/mpid/ch3/channels/sock/include/mpidi_ch3_pre.h | These should be removed |
| src/mpid/ch3/channels/sock/include/mpidi_ch3_pre.h | Are the following packet extensions? Can the socket connect/accept packets be made part of the util/sock support? |
| src/mpid/ch3/channels/sock/include/mpidi_ch3_pre.h | Explain these; why is this separate from the VC state? |
| src/mpid/ch3/channels/sock/include/mpidi_ch3_post.h | We need to document all of these parameters. There should be no ifdef that is not documented, other than those set by configure |
| src/mpid/ch3/channels/nemesis/src/ch3_init.c | Circular dependency. Before calling MPIDI_CH3_Init, MPID_Init calls InitPG which calls MPIDI_PG_Create which calls MPIDI_CH3_VC_Init. But MPIDI_CH3_VC_Init needs nemesis to be initialized. We can't call MPIDI_CH3_Init before initializing the PG, because it needs the PG. There is a hook called MPIDI_CH3_PG_Init which is called from MPIDI_PG_Create before MPIDI_CH3_VC_Init, but we don't have the pg_rank in that function. Maybe this shouldn't really be a FIXME, since I believe that this issue will be moot once nemesis is a device. So what we do now, is do nothing if MPIDI_CH3_VC_Init is called before MPIDI_CH3_Init, and call MPIDI_CH3_VC_Init from inside MPIDI_CH3_Init after initializing nemesis |
| src/mpid/ch3/channels/nemesis/src/ch3_init.c | where does this vc get freed? ANSWER (goodell@) - ch3u_port.c FreeNewVC (but the VC_Destroy is in this file) |
| src/mpid/ch3/channels/nemesis/src/ch3_istartmsg.c | we are sometimes called from within the progress engine, we shouldn't be calling the progress engine again |
| src/mpid/ch3/channels/nemesis/src/ch3_progress.c | should be appropriately abstracted somehow |
| src/mpid/ch3/channels/nemesis/src/ch3_progress.c | should be appropriately abstracted somehow |
| src/mpid/ch3/channels/nemesis/src/ch3_progress.c | should be appropriately abstracted somehow |
| src/mpid/ch3/channels/nemesis/src/ch3i_comm.c | need a better solution here. e.g., allocate some barrier_vars on the first barrier for the life of the communicator (as is the case now), others must be allocated for each barrier, then released. If we run out of barrier_vars after that, then use msg_barrier. |
| src/mpid/ch3/channels/nemesis/include/mpidi_ch3_impl.h | used for debugging |
| src/mpid/ch3/channels/nemesis/include/mpidi_ch3_impl.h | ch3 assumes there is a field called sendq_head in the ch portion of the vc. This is unused in nemesis and should be set to NULL |
| src/mpid/ch3/channels/nemesis/nemesis/include/mpid_nem_queue.h | is a barrier needed here because of the control-only dependency? |
| src/mpid/ch3/channels/nemesis/nemesis/include/mpid_nem_queue.h | We shouldn't really be using the MPID_Thread_mutex_* code but the MPIDU_Process_locks code is a total mess right now. In the long term we need to resolve this, but in the short run it should be safe on most (all?) platforms to use these instead. Usually they will both boil down to a pthread_mutex_t and and associated functions. |
| src/mpid/ch3/channels/nemesis/nemesis/include/mpid_nem_defs.h | This definition should be gotten from mpidi_ch3_impl.h |
| src/mpid/ch3/channels/nemesis/nemesis/include/mpid_nem_post.h | Do not export these definitions all the way into mpiimpl.h (which will happen if this is included in mpidpost.h or mpidpre.h) |
| src/mpid/ch3/channels/nemesis/nemesis/include/mpid_nem_datatypes.h | We are using this as an interprocess lock in the queue code, although it's not strictly guaranteed to work for this scenario. These should really use the "process locks" code, but it's in such terrible shape that it doesn't really work for us right now. Also, because it has some inline assembly it's not really a fair comparison for studying the impact of atomic instructions. [goodell@ 2009-01-16] |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/elan/elan_init.c | create a real error string for this |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/elan/elan_init.c | create a real error string for this |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/elan/elan_init.c | Check to make sure that it's OK to terminate the connection without making sure that all sends have been sent |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/newmad/newmad_poll.c | this might not work in the case of multiple netmods |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/newmad/newmad_init.c | create a real error string for this |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/newmad/newmad_init.c | Check to make sure that it's OK to terminate the connection without making sure that all sends have been sent |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/wintcp_poll.c | Get rid of these dummy funcs |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.h | This structure can be moved to sock wrappers once MPIU_Sock_fd_t becomes more opaque OR we replace MPIU_Sock_fd_t with an opaque MPIU_Sock_t Also, look into the MSMPI code on how to organize this private struct members into different states, listen_state/accept_state/read_state/write_state, depending on the type of socket. |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.h | Check whether the elements of sockconn is aligned correctly |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.h | Remove the index. We no longer need it |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.h | CONTAINING_RECORD is only defined for windows - Use offsetof() to define CONTAINING_RECORD() on unix |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.h | bc actually contains port_name info |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Why should the listen sock conn be global ? |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Tune the skip poll values since we now have a fast executive (instead of select) |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | We won't need a common place holder for these handlers. Remove it eventually. |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | We need to check whether a non-linear growth of the sc_tbl will be better for performance for large jobs |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Use if-else and get rid of goto |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | The index could be invalid in the case of an error |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | need to handle error condition better |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Post a write instead of assuming a write would succeed Looks like we should be *usually* ok here since the conn is just established and socket send buffer > sizeof iov |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Post a write instead of assuming a write would succeed Looks like we should be *usually* ok here since the conn is just established and socket send buffer > sizeof iov |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | We should ideally do a shutdown() not close () |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | We should ideally do a shutdown() not close () |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Remove the assumption that we will be able to read the complete id/tmp_vc info in a single read... |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Just retry if we don't read the complete header |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | We might want to use sc->tmp_buf - after hdr for reading the pg_id |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Post a read if read fails - instead of aborting |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Z1 |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | is it needed ? |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Don't assume read will read the whole pg_id/tmpvc |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Z1 |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | We are assuming the command packets are only sent during connection phase - enough sock buffer for the calls to succeed |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Z1 |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | We need to set addr and port using bc. If a process is dynamically spawned, vc->pg is NULL. In that case, same procedure is done in MPID_nem_newtcp_module_connect_to_root() |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Post a close and let the quiescent handler take care of adding sc to freeq |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | How can the sc->fd be invalid here ? |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | XXX DJG do we need to do anything here to ensure that the final close(TRUE) packet has made it into a writev call? The code might have a race for queued messages. |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | WINTCP ASYNC MPIU_Assert(VC_FIELD(vc, sc_ref_count) == 0); |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | We are calling the cntd success handler explicitly here. Get rid of one of these states |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Danger add error string actual string : "**cannot send idinfo" |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Don't assume that the whole hdr is read |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Remove this restriction |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Add more states instead of setting handlers explicitly.. OR get rid of all states |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Is this reqd ? - there is a *accept_fail_handler |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Implement connect retry ... |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Is this needed ? On accept side the sc won't have a vc right now |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Re-introduce after we get rid of explicit calls to handlers MPIU_Assert(nb > 0); |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Get rid of found_better_sc() in the C_CNTD state. Always have the connection ACK/NACK by the listen side There are 2 cases here: 1) A connect() is in progress - Send a NACK - Let the other connection succeed 2) A connection already exists - Send a NACK |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | When is snd_nak true ? |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Create post thread here |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Kill post thread here |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | We also need to tune the number of times we need to wait for an event. eg: SOCK_PROGRESS_EVENTS_MAX = 2 x No_of_fds_in_ex_set |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Try to avoid calling the rd handler explicitly |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Get rid of static allocn of listen sock and post a close |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/wintcp_init.c | Move all externs to say socksm_globals.h |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/wintcp_init.c | All MPI util funcs should return an MPI error code |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/wintcp_init.c | No error code returned ! |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/wintcp_init.c | WINTCP ASYNC MPID_nem_newtcp_module_g_lstn_sc.handler = MPID_nem_newtcp_module_state_listening_handler; |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/wintcp_finalize.c | Move all externs to say socksm_globals.h |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/wintcp_finalize.c | Why don't we have a finalize for sm - MPID_nem_newtcp_module_finalize_sm() - ? |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/wintcp_finalize.c | Shouldn't the order of finalize() be the reverse order of init() ? i.e., *finalize_sm(); *poll_finalize(); *send_finalize(); |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/wintcp_impl.h | WINTCP ASYNC int MPID_nem_newtcp_module_conn_wr_enable (struct MPIDI_VC *const vc); int MPID_nem_newtcp_module_conn_wr_disable (struct MPIDI_VC *const vc); |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/wintcp_impl.h | WINTCP ASYNC MPID_NEM_NEWTCP_MODULE_SOCK_STATUS_t MPID_nem_newtcp_module_check_sock_status(MPIU_SOCKW_Waitset_sock_hnd_t fd_ws_hnd); |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/wintcp_impl.h | WINTCP ASYNC int MPID_nem_newtcp_module_recv_handler (MPIU_SOCKW_Waitset_sock_hnd_t fd_ws_hnd, sockconn_t *sc); |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/wintcp_impl.h | WINTCP ASYNC int MPID_nem_newtcp_module_state_listening_handler(MPIU_SOCKW_Waitset_sock_hnd_t fd_ws_hnd, sockconn_t *const l_sc); |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/wintcp_utility.c | At least on windows there is no guarantee that a successful socket call resets the socket error code We will assume that sock is connected for now and let the state machine handle errors |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/tcp/socksm.c | trace/log all the state transitions |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/tcp/socksm.c | need to handle error condition better |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/tcp/socksm.c | Z1 |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/tcp/socksm.c | Z1 |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/tcp/socksm.c | is it needed ? |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/tcp/socksm.c | Z1 |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/tcp/socksm.c | Z1 |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/tcp/socksm.c | Z1 |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/tcp/socksm.c | We need to set addr and port using bc. If a process is dynamically spawned, vc->pg is NULL. In that case, same procedure is done in MPID_nem_tcp_connect_to_root() |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/tcp/socksm.c | XXX DJG do we need to do anything here to ensure that the final close(TRUE) packet has made it into a writev call? The code might have a race for queued messages. |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/tcp/tcp_getip.c | This should use the standard debug/logging routines and macros |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/tcp/socksm.h | should plfd really be const const? Some handlers may need to change the plfd entry. |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/tcp/socksm.h | bc actually contains port_name info |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/mx/mx_init.c | create a real error string for this |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/mx/mx_init.c | create a real error string for this |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/mx/mx_init.c | Check to make sure that it's OK to terminate the connection without making sure that all sends have been sent |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/mx/mx_cancel.c | The vc is only needed to find which function to call |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/mx/mx_cancel.c | this test is probably not correct with multiple netmods |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/mx/mx_poll.c | this might not work in the case of multiple netmods |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/mx/mx_poll.c | |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/mx/mx_poll.c | this function can't return an error because it's called from a recvq function that doesn't check for errors. For now, I'm replacing the chkandjump with an assertp. |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/mx/mx_poll.c | how can we report MPI_ERR_TRUNC in this case? |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/mx/mx_send.c | match_info is used uninitialized |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/gm/gm_init.c | create a real error string for this |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/gm/gm_init.c | create a real error string for this |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/gm/gm_init.c | this is only valid for processes in COMM_WORLD |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/gm/gm_init.c | Check to make sure that it's OK to terminate the connection without making sure that all sends have been sent |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/nd/nd_impl.h | Declare all MPICH headers under extern, if possible |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/nd/nd_impl.h | Change #define const to const int |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/nd/nd_impl.h | Check if we can pack the structures in ND more tightly |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/nd/nd_impl.h | We might need a list of completion queues here if we exhaust the number of endpoints on a single completion queue |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/nd/nd_impl.h | Decide on these SENDQ/RECVQ sizes more judiciously |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/nd/nd_impl.h | Change this value |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/nd/nd_impl.h | Only use/send the valid mw_datas |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/nd/nd_impl.h | Use this for multi-mws |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/nd/nd_impl.h | Define the number of max conn dynamically |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/nd/nd_impl.h | Use this for Send() etc after extending Executive |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/nd/nd_impl.h | REMOVE ME ! |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/nd/nd_impl.h | Currently one cannot block for specific ops |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/nd/nd_sm.h | Use inline buffering - see ND_SGE doc - to improve latency |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/nd/nd_sm.h | we could piggyback data on a CACK |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/psm/psm_init.c | Check to make sure that it's OK to terminate the connection without making sure that all sends have been sent |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_lmt_vmsplice.c | we should write our own function that isn't dependent on the in-request iov array. This will let us use IOVs that are larger than MPID_IOV_LIMIT. |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_lmt_vmsplice.c | set the error status of the req and complete it, rather than POP |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_lmt_vmsplice.c | set the error status of the req and complete it, rather than POP |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_lmt_vmsplice.c | need to handle the case where a VC is terminated due to a process failure. We need to remove any outstanding LMT ops for this VC. |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_lmt.c | where does this request get freed? |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_lmt.c | where does this request get freed? |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_lmt_dma.c | randomly chosen |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_lmt_dma.c | The knem module iovec is potentially different from the system iovec. This causes all sorts of fun if you don't realize it and use the system iovec directly instead. Eventually we need to either unify them or avoid this extra copy. |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_lmt_dma.c | The knem module iovec is potentially different from the system iovec. This causes all sorts of fun if you don't realize it and use the system iovec directly instead. Eventually we need to either unify them or avoid this extra copy. |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_lmt_dma.c | we should write our own function that isn't dependent on the in-request iov array. This will let us use IOVs that are larger than MPID_IOV_LIMIT. |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_ckpt.c | we're only handling processes in our pg, so no dynamic connections |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_init.c | DARIUS set these to default for now |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_init.c | DARIUS -- enable this assert once these functions are implemented |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_init.c | ch3 assumes there is a field called sendq_head in the ch portion of the vc. This is unused in nemesis and should be set to NULL |
| src/mpid/ch3/channels/nemesis/nemesis/src/mpid_nem_barrier.c | this is not a scalable algorithm because everyone is polling on the same cacheline |
| src/mpid/ch3/channels/sctp/include/mpidi_ch3_pre.h | These should be removed |
| src/mpid/ch3/channels/sctp/include/mpidi_ch3_pre.h | Are the following packet extensions? Can the socket connect/accept packets be made part of the util/sock support? |
| src/mpid/ch3/channels/sctp/include/mpidi_ch3_pre.h | We need a little security here to avoid having a random port scan crash the process. Perhaps a "secret" value for each process could be published in the key-val space and subsequently sent in the open pkt. |
| src/mpid/ch3/channels/sctp/include/mpidi_ch3_pre.h | this is called in mpid_vc.c |
| src/mpid/ch3/channels/sctp/include/mpidi_ch3_post.h | We need to document all of these parameters. There should be no ifdef that is not documented, other than those set by configure |
| src/mpid/ch3/channels/sctp/include/mpidi_ch3_impl.h | Any of these used in the ch3->channel interface should be defined in a header file in ch3/include that defines the channel interface |
| src/mpid/ch3/channels/sctp/src/ch3_progress.c | should be appropriately abstracted somehow |
| src/mpid/ch3/channels/sctp/src/ch3_progress.c | should be appropriately abstracted somehow |
| src/mpid/ch3/channels/sctp/src/ch3_progress.c | is it possible for tmp_vc to be non-NULL and pkt to be an accept pkt? |
| src/mpid/ch3/channels/sctp/src/ch3_progress.c | what if not a full pkt (!MSG_EOR)? |
| src/mpid/ch3/channels/sctp/src/ch3_progress.c | why would the vc ref_count be zero w/o being destroyed yet? A check against zero like this is usually not thread-safe... |
| src/mpid/ch3/channels/sctp/src/ch3_progress.c | done elsewhere so add this to a function |
| src/mpid/ch3/channels/sctp/src/ch3_progress.c | define error code |
| src/mpid/ch3/channels/sctp/src/ch3_progress.c | should be appropriately abstracted somehow |
| src/mpid/ch3/channels/sctp/src/ch3_progress.c | should be appropriately abstracted somehow |
| src/mpid/ch3/channels/sctp/src/ch3_progress.c | TODO : change error codes to have SCTP errors |
| src/mpid/ch3/channels/sctp/src/ch3_progress.c | atoi doesn't detect errors (e.g., non-digits) |
| src/mpid/ch3/channels/sctp/src/ch3_progress.c | atoi doesn't detect errors (e.g., non-digits) |
| src/mpid/ch3/channels/sctp/src/ch3_progress.c | might need to be more sophisticated with multiple fd's |
| src/mpid/ch3/channels/sctp/src/sctp_common.c | assumes success |
| src/mpid/ch3/channels/sctp/src/ch3_init.c | define error code |
| src/mpid/ch3/channels/sctp/src/ch3_init.c | define error code |
| src/mpid/ch3/channels/sctp/src/sctp_util.c | Is this documented? How does it work if the process manager cannot give each process a different value for an environment name? What if a different interface is needed? |
| src/mpid/ch3/channels/sctp/src/sctp_util.c | We should start switching to getaddrinfo instead of gethostbyname |
| src/mpid/ch3/channels/sctp/src/sctp_util.c | We don't make use of the ifname in Windows in order to provide backward compatibility with the (undocumented) host description string used by the socket connection routine MPIDU_Sock_post_connect. We need to change to an interface-address (already resolved) based description for better scalability and to eliminate reliance on fragile DNS services. Note that this is also more scalable, since the DNS server may serialize address requests. On most systems, asking for the host info of yourself is resolved locally (i.e., perfectly parallel). |
| src/mpid/ch3/channels/sctp/src/sctp_util.c | This is a hack to allow Windows to continue to use the host description string instead of the interface address bytes when posting a socket connection. This should be fixed by changing the Sock_post_connect to only accept interface address. Note also that Windows does not have the inet_pton routine; the Windows version of this routine will need to be identified or written. See also channels/sock/ch3_progress.c |
| src/mpid/ch3/channels/sctp/src/ch3_istartmsg.c | change error code |
| src/mpid/ch3/channels/sctp/src/ch3_isendv.c | check specific sendQ and send_active |
| src/mpid/ch3/channels/sctp/src/ch3_isendv.c | the current code only agressively writes the first IOV. Eventually it should be changed to agressively write as much as possible. Ideally, the code would be shared between the send routines and the progress engine. |
| src/mpid/ch3/channels/sctp/src/ch3_isendv.c | enqueue x |
| src/mpid/ch3/channels/dllchan/include/mpidi_ch3_impl.h | Old code from sock below here |
| src/mpid/ch3/channels/dllchan/include/mpidi_ch3_impl.h | Any of these used in the ch3->channel interface should be defined in a header file in ch3/include that defines the channel interface |
| src/mpid/ch3/channels/dllchan/include/mpidi_ch3_pre.h | These should be removed |
| src/mpid/ch3/channels/dllchan/include/mpidi_ch3_pre.h | Are the following packet extensions? Can the socket connect/accept packets be made part of the util/sock support? |
| src/mpid/ch3/channels/dllchan/include/mpidi_ch3_pre.h | We need a little security here to avoid having a random port scan crash the process. Perhaps a "secret" value for each process could be published in the key-val space and subsequently sent in the open pkt. |
| src/mpid/ch3/channels/dllchan/include/mpidi_ch3_pre.h | Explain these; why is this separate from the VC state? |
| src/mpid/ch3/channels/dllchan/include/mpidi_ch3_post.h | We need to document all of these parameters. There should be no ifdef that is not documented, other than those set by configure |
| src/mpid/ch3/channels/dllchan/src/ch3impl.c | As a workaround for the use of PG_Destroy *after* CH3_Finalize is called, this entry point is set to something benign, and the VC_Destroy that PG_Destroy may invoke, is also set to ignore. |
| src/mpid/common/sock/mpidu_sock.h | This is not the right way to add error values to an MPICH module. Note that (a) the last class values are not respected by the error handling code, (b) the entire point of codes and classes is to provide a natural grouping of codes to a class, (c) this approach can only be used by one module and hence breaks any component design, and (d) this is what the MPI dynamic error codes and classes was designed for. |
| src/mpid/common/sock/iocp/sock.c | Is this correct for resource errors like WSAENOBUFS or an interrupted operation? |
| src/mpid/common/sock/iocp/sock.c | does this data need to be handled immediately? |
| src/mpid/common/sock/poll/socki_util.i | These need to separate the operations from the thread-related synchronization to ensure that the code that is independent of threads is always the same. Also, the thread-level check needs to be identical to all others, and there should be an option, possibly embedded within special thread macros, to allow runtime control of the thread level |
| src/mpid/common/sock/poll/socki_util.i | Does this need a runtime check on whether threads are in use? |
| src/mpid/common/sock/poll/socki_util.i | Low usage operations like this should be a function for better readability, modularity, and code size |
| src/mpid/common/sock/poll/socki_util.i | Are these really optional? Based on their definitions, it looks like they should only be used when debugging the code. |
| src/mpid/common/sock/poll/socki_util.i | Should this use the CHKPMEM macros (perm malloc)? |
| src/mpid/common/sock/poll/socki_util.i | We need an abstraction for the thread sync operations |
| src/mpid/common/sock/poll/socki_util.i | move last element into current position and update sock associated with last element. |
| src/mpid/common/sock/poll/socki_util.i | Shouldn't this be an mpi error code? |
| src/mpid/common/sock/poll/socki_util.i | Who allocates eventq tables? Should there be a check that these tables are empty first? |
| src/mpid/common/sock/poll/socki_util.i | Is this the name that we want to use (this was chosen to match the original, undocumented name) |
| src/mpid/common/sock/poll/sock_post.i | Provide an overview for the functions in this file |
| src/mpid/common/sock/poll/sock_post.i | It would be better to include a special formatting clue for system error messages (e.g., %dSE; in the recommended revision for error reporting (that is, value (errno) is an int, but should be interpreted as an System Error string) |
| src/mpid/common/sock/poll/sock_post.i | What does this routine do? Why does it take a host description instead of an interface name or address? |
| src/mpid/common/sock/poll/sock_post.i | strtok may change the contents of host_description. Shouldn't the host description be a const char [] and not modified by this routine? |
| src/mpid/common/sock/poll/sock_post.i | For ipv6, we should use getaddrinfo |
| src/mpid/common/sock/poll/sock_post.i | Set error |
| src/mpid/common/sock/poll/sock_post.i | What does this function do? |
| src/mpid/common/sock/poll/sock_wait.i | return code? If an error occurs do we return it instead of the error specified in the event? |
| src/mpid/common/sock/poll/sock_misc.i | This routine is misnamed; it is really get_interface_name (in the case where there are several networks available to the calling process, this picks one but even in the current code can pick a different interface if a particular environment variable is set) . This routine is used in smpd so we should not change its name yet. |
| src/mpid/common/sock/poll/sock_misc.i | Is this documented? How does it work if the process manager cannot give each process a different value for an environment name? What if a different interface is needed? |
| src/mpid/common/sock/poll/sock_misc.i | What is this for? It appears to be used in debug printing and in smpd |
| src/mpid/common/sock/poll/sock_misc.i | This function violates the internationalization design by using English language strings rather than the error translation mechanism. This unnecessarily breaks the goal of allowing internationalization. Read the design documentation and if there is a problem, raise it rather than ignoring it. |
| src/mpid/common/sock/poll/sock_misc.i | This appears to only be used in smpd |
| src/mpid/common/sock/poll/sock_misc.i | It appears that this function was used instead of making use of the existing MPI-2 features to extend MPI error classes and code, of to export messages to non-MPI application (smpd?) |
| src/mpid/common/sock/poll/mpidu_socki.h | !!! |
| src/mpid/common/sock/poll/sock_set.i | Move the thread-specific operations into thread-specific routines (to allow for alternative thread sync models and for runtime control of thread level) |
| src/mpid/common/sock/poll/sock.c | What do these mean? Why is 32 a good size (e.g., is it because 32*32 = 1024 if these are bits in a 4-byte int? In that case, should these be related to a maximum processor count or an OS-defined fd limit? |
| src/mpid/common/sock/poll/sock.c | Why aren't these static |
| src/mpid/common/sock/poll/sock.c | Why are these files included in this way? Why not make them either separate files or simply part of (one admittedly large) source file? |
| src/mpid/common/sock/poll/sock_init.i | The usual missing documentation (what are these routines for? preconditions? who calls? post conditions? |
| src/mpid/common/sock/poll/sock_init.i | Who calls? When? Should this be a finalize handler instead? |
| src/mpid/common/sock/poll/sock_immed.i | Why is this the _immed file (what does immed stand for?) |
| src/mpid/common/sock/poll/sock_immed.i | What do any of these routines do? What are the arguments? Special conditions (see the FIXME on len = SSIZE_MAX)? preconditions? postconditions? |
| src/mpid/common/sock/poll/sock_immed.i | What does this function do? What are its arguments? It appears to execute a nonblocking accept call |
| src/mpid/common/sock/poll/sock_immed.i | Either use the syscall macro or correctly wrap this in a test for EINTR |
| src/mpid/common/sock/poll/sock_immed.i | There should be a simpler macro for reporting errno messages |
| src/mpid/common/sock/poll/sock_immed.i | Who sets the socket buffer size? Why isn't the test made at that time? |
| src/mpid/common/sock/poll/sock_immed.i | There's normally no need to check that the socket buffer size was set to the requested size. This should only be part of some more verbose diagnostic output, not a general action |
| src/mpid/common/sock/poll/sock_immed.i | There's normally no need to check that the socket buffer size was set to the requested size. This should only be part of some more verbose diagnostic output, not a general action |
| src/mpid/common/sock/poll/sock_immed.i | Cut and paste code is a disaster waiting to happen. Particularly in any non-performance critical section, create a separate routine instead of using cut and paste. |
| src/mpid/common/sock/poll/sock_immed.i | multiple passes should be made if len > SSIZE_MAX and nb == SSIZE_MAX |
| src/mpid/common/sock/poll/sock_immed.i | This is a scary test/assignment. It needs an explanation (presumably that this routine will be called again if len is shortened. However, in that case, the description of the routine (which is also missing!!!!) needs to be very clear about this requirement. |
| src/mpid/common/sock/poll/sock_immed.i | multiple passes should be made if len > SSIZE_MAX and nb == SSIZE_MAX |
| src/mpid/common/sock/poll/sock_immed.i | We need (1) a standardized test for including multithreaded code and (2) include support for user requests for a lower-level of thread safety. Finally, things like this should probably be implemented as an abstraction (e.g., wakeup_progress_threads?) rather than this specific code. |
| src/mpid/common/thread/mpid_thread.h | Define other error codes. For now, any non-zero value is an error. |
| src/mpid/common/datatype/mpid_type_create_resized.c | ??? |
| src/mpid/common/datatype/mpid_ext32_segment.h | !!! TODO!!! FOO!!! DO THIS!!! DETECT ME!!! Include mpiimpl.h up here too, taking it out of mpid_ext32_segment.c Consider using MPIU_INT64_T etc. types instead of the EIGHT_BYTE_BASIC_TYPE stuff, or put #defines at the top of this file assigning them in a simple manner. Doing that might require that we create MPIU_UINT64_T types (etc), because it looks like we really want to have unsigned types for the various convert functions below. |
| src/mpid/common/datatype/mpid_ext32_segment.h | This is necessary with some compilers or compiler settings |
| src/mpid/common/datatype/mpid_ext32_segment.h | Who defines __BYTE_ORDER or __BIG_ENDIAN? They aren't part of C |
| src/mpid/common/datatype/mpid_ext32_segment.h | The else test assumes that the byte order is little endian, whereas it may simply have been undetermined. This should instead use either a configure-time test (for which there are macros) or a runtime test and not use this non-portable check |
| src/mpid/common/datatype/mpid_ext32_segment.h | "BLENDIAN" is a non-conforming name - it could conflict with some other definition in a non-mpich2 header file |
| src/mpid/common/datatype/mpid_contents_support.c | Is this routine complete? Why is it needed? If it is needed, it must have a comment that describes why it is needed and the arguments must have ATTRIBUTE((unused)) |
| src/mpid/common/datatype/mpid_datatype.h | calculating this value this way is foolish, we should make this more automatic and less error prone |
| src/mpid/common/datatype/dataloop/segment_count.c | The blkidx and index functions are not used since they optimize the count by coalescing contiguous segments, while functions using the count do not optimize in the same way (e.g., flatten code) |
| src/mpid/common/sched/mpid_sched.c | this requires VA_ARGS macros |
| src/mpid/common/sched/mpid_sched.c | is this right when comm/datatype GC is used? |
| src/mpid/common/sched/mpid_sched.h | open questions: - Should the schedule hold a pointer to the nbc request and the nbc request hold a pointer to the schedule? This could cause MT issues. |
| src/util/info/info_dup.c | multithreaded |
| src/util/info/info_get.c | The real fix is to change MPIU_Strncpy to set the null at the end (always!) and return an error if it had to truncate the result. |
| src/util/param/param_vals.c | any MT issues here? |
| src/util/multichannel/mpi.c | Remove dlls without a channel token string |
| src/util/multichannel/mpi.c | Get rid of dlls without a channel substring in the name |
| src/util/multichannel/mpi.c | Should we allow for short wrapper names like 'mpe'? |
| src/util/wrappers/mpiu_shm_wrappers.h | Add underscore to the end of funcs/macro names not to be exposed to user |
| src/util/wrappers/mpiu_shm_wrappers.h | Reduce the conditional checks for wrapper-internal utility funcs/macros. |
| src/util/wrappers/mpiu_shm_wrappers.h | What if val is a non-null terminated string ? |
| src/util/wrappers/mpiu_shm_wrappers.h | Don't print ENGLISH strings on error. Define the error strings in errnames.txt |
| src/util/wrappers/mpiu_shm_wrappers.h | Pass (void **)shm_addr_ptr instead of (char **) shm_addr_ptr since a util func should be generic Currently not passing (void **) to reduce warning in nemesis code which passes (char **) ptrs to be attached to a seg |
| src/util/wrappers/mpiu_shm_wrappers.h | Close local handle only when closing the shm handle |
| src/util/wrappers/mpiu_sock_wrappers.h | Replace wrapper funcs implemented as complex macros with static inline funcs |
| src/util/wrappers/mpiu_sock_wrappers.h | Add more comments |
| src/util/wrappers/mpiu_sock_wrappers.h | Ensure this header is only loaded when needed |
| src/util/wrappers/mpiu_sock_wrappers.h | Try receiving data with AcceptEx() call to improve perf |
| src/util/wrappers/mpiu_sock_wrappers.h | This is cheating... Expand dynamically - we need to take care of updating sock Handles if we expand dynamically. |
| src/util/wrappers/mpiu_sock_wrappers.h | Cheating - Expand dynamically |
| src/util/wrappers/mpiu_sock_wrappers.h | Try not to use MPIU_Assert(). Utils should not depend on a device impl of assert |
| src/util/instrm/instr.c | This should at least issue the writes in process order |
| src/util/dbg/dbg_control.c | Delete this file (functionality switched to MPIU_DBG_MSG interface |
| src/util/dbg/dbg_printf.c | This should use MPIU_Param_get_string |
| src/util/dbg/dbg_printf.c | put everything on one line until a \n is found |
| src/util/dbg/dbg_printf.c | |
| src/util/dbg/dbg_printf.c | Need to know how many MPI_COMM_WORLDs are known |
| src/util/dbg/dbg_printf.c | This is a hack to handle the common case of two worlds |
| src/util/dbg/dbg_printf.c | Get world number |
| src/util/thread/mpiu_thread.c | faster allocation, or avoid it all together? |
| src/util/thread/mpiu_thread.c | convert error to an MPIU_THREAD_ERR value |
| src/util/thread/mpiu_thread.c | faster allocation, or avoid it all together? |
| src/util/thread/mpiu_thread.c | convert error to an MPIU_THREAD_ERR value |
| src/util/mem/handlemem.c | The alloc_complete routine should be removed. It is used only in typeutil.c (in MPIR_Datatype_init, which is only executed from within the MPI_Init/MPI_Init_thread startup and hence is guaranteed to be single threaded). When used by the obj_alloc, it adds unnecessary overhead, particularly when MPI is single threaded |
| src/util/logging/rlog/rlogtime.c | This name needs to be changed to ensure no conflicts with user-defined globals |
| src/util/logging/rlog/printrlog.c | Add a structured comment for the man page generate to create the basic documentation on this routine, particularly since this routine only prints a subset of information by default |
| src/util/logging/rlog/printrlog.c | This should also check for the GNU-standard --help, --usage, and -h options. |
| src/util/logging/rlog/printrlog.c | What is the default behavior with just an rlogfile? |
| src/util/logging/rlog/TraceInput/trace_input.c | Improve failure reporting |
| src/util/procmap/local_proc.c | this is including a ch3 include file! Implement these functions at the device level. |
| src/util/procmap/local_proc.c | this could/should be a list of ranks on the local node, which should take up much less space on a typical thin(ish)-node system. |
| src/util/ex/ex.c | Change ExCreateSet() to allow adding the set to a list & allow sending this list to MPIU_ExProcessCompletions() MPIU_ExSetHandle_t MPIU_ExCreateSet(MPIU_ExCreateSetList_t list) |
| src/util/ex/ex.c | Should'nt there be a retry count ? |
| src/pm/smpd/mp_parse_command_line.c | We don't detect overflow case in atoi |
| src/pm/smpd/mp_parse_command_line.c | How do we tell the difference between an error parsing the file and parsing the last entry? |
| src/pm/smpd/smpd_implthread.h | SMPD internally calls the utils defined for the SMPD device eg: SMPDU_Sock_wait() -- Remove this dependency by copying the required socket code to smpd |
| src/pm/smpd/smpd_handle_spawn.c | This will turn on exit code printing for all processes. Implement a new mechanism for only printing codes for an individual process group. |
| src/pm/smpd/smpd_handle_spawn.c | Tell MPICH that the spawned processes will not make any SMPD calls, including SMPD_Init - so don't to a comm_accept or it will hang! |
| src/pm/smpd/smpd_connect.c | this insertion needs to be thread safe |
| src/pm/smpd/smpd_connect.c | cleanup sspi structures |
| src/pm/smpd/smpd_connect.c | this could overflow |
| src/pm/smpd/smpd_connect.c | Insert code here to parse a compressed host string |
| src/pm/smpd/smpd_connect.c | this could overflow |
| src/pm/smpd/smpd_connect.c | this could overflow |
| src/pm/smpd/smpd_service.c | Remove hardcoded values -- eg: Length of error Mesg, 256 |
| src/pm/smpd/smpd_handle_command.c | smpd_get_hostname() does the same thing |
| src/pm/smpd/smpd_handle_command.c | We shouldn't abort if a process fails to launch as part of a spawn command |
| src/pm/smpd/smpd_handle_command.c | mismatch btw size of connect_to->host and max host name len |
| src/pm/smpd/smpd_handle_command.c | decrypt the password |
| src/pm/smpd/smpd_handle_command.c | This assumes that the server knows to post a write of the delegate command because it knows that no buffer will be returned by the iter command |
| src/pm/smpd/smpd_handle_command.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_handle_command.c | username and password needed to map a drive |
| src/pm/smpd/smpd_handle_command.c | create a new return class, for now use SMPD_DBS_RETURN |
| src/pm/smpd/smpd_handle_command.c | decrypt the password |
| src/pm/smpd/smpd_handle_command.c | How do we determine whether to provide user credentials or impersonate the current user? |
| src/pm/smpd/smpd_handle_command.c | Assign the appropriate src/dst for singinit/die |
| src/pm/smpd/smpd_iov.h | How is IOV_LIMIT chosen? |
| src/pm/smpd/smpd_host_util.c | If these are not NULL should they be freed? |
| src/pm/smpd/smpd.c | Get rid of this hack - we already create local KVS for all singleton clients by default |
| src/pm/smpd/mpiexec.c | Get rid of this hack - we already create local KVS for all singleton clients by default |
| src/pm/smpd/smpd_database.c | Should this cleanup be done here ? This lets one forget to do a smpd_dbs_destroy() for every smpd_dbs_create() |
| src/pm/smpd/smpd_database.c | Replace SMPD_DBS_SUCCESS/... error codes with SMPD_SUCCESS/... |
| src/pm/smpd/smpd_database.c | Use routines below to replace dbs_first(), dbs_next() routines |
| src/pm/smpd/smpd_database.c | Go for a macro here ? |
| src/pm/smpd/smpd_database.c | Desc error msgs ? |
| src/pm/smpd/smpd_launch_process.c | The return vals of these functions are not checked ! |
| src/pm/smpd/smpd_state_machine.c | what does 0 bytes written mean? Does it mean that no bytes could be written at that moment or does it mean that there is an error on the socket? |
| src/pm/smpd/smpd_state_machine.c | the smpd password needs to be able to be set |
| src/pm/smpd/smpd_state_machine.c | Instead of encoding a pointer to the context, add an integer id to the context structure and look up the context based on this id from a global list of contexts. |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | This assumes that the server knows to post a write of the delegate command because it knows that no buffer will be returned by the iter command |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Instead of encoding a pointer to the context, add an integer id to the context structure and look up the context based on this id from a global list of contexts. |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Add code to cleanup sspi structures |
| src/pm/smpd/smpd_state_machine.c | Instead of encoding a pointer to the context, add an integer id to the context structure and look up the context based on this id from a global list of contexts. |
| src/pm/smpd/smpd_state_machine.c | Instead of encoding a pointer to the context, add an integer id to the context structure and look up the context based on this id from a global list of contexts. |
| src/pm/smpd/smpd_state_machine.c | Reintroduce event.error == SMPDU_SOCK_ERR_CONN_CLOSED once we have a better error code |
| src/pm/smpd/smpd_state_machine.c | should we send a message to the root process manager to close stdin to the root process? |
| src/pm/smpd/smpd.h | Use an enum instead enum SMPD_RetVal{ SMPD_ERR_INVALID_USER=-3, SMPD_ABORT, SMPD_FAIL, SMPD_SUCCESS, SMPD_DBS_RETURN, SMPD_CONNECTED, SMPD_CLOSE, SMPD_EXIT, SMPD_LAST_RETVAL }; |
| src/pm/smpd/smpd.h | Do something sensible like mpitypedefs.h |
| src/pm/smpd/smpd.h | Remove this |
| src/pm/smpd/smpd.h | Remove this |
| src/pm/smpd/smpd.h | Cleanup this struct after we start using spn list handles everywhere |
| src/pm/smpd/mpiexec_rsh.c | Why is this func defined here ? - shouldn't this be in smpd_util*.lib ? |
| src/pm/smpd/mpiexec_rsh.c | Do we need a generic handle type for smpd thread/proc ? |
| src/pm/smpd/smpd_printf.c | For now ... translate correctly later |
| src/pm/smpd/smpd_user_data.c | function not implemented |
| src/pm/smpd/smpd_affinitize.c | Is this defined for win32 ? case RelationProcessorPackage : return prioPackage; |
| src/pm/smpd/sock/iocp/smpd_util_sock.c | Use WSADuplicateSocket() instead |
| src/pm/smpd/sock/iocp/smpd_util_sock.c | Use WSADuplicateSocket instead |
| src/pm/smpd/sock/iocp/smpd_util_sock.c | Do we need this any more ? - PM sock util is single threaded |
| src/pm/smpd/sock/iocp/smpd_util_sock.c | Use WSADuplicateSocket() instead |
| src/pm/smpd/sock/iocp/smpd_util_sock.c | Is this correct for resource errors like WSAENOBUFS or an interrupted operation? |
| src/pm/smpd/sock/iocp/smpd_util_sock.c | Why don't we check for errors during post ? |
| src/pm/smpd/sock/iocp/smpd_util_sock.c | Wait on progress engine instead |
| src/pm/smpd/sock/iocp/smpd_util_sock.c | does this data need to be handled immediately? |
| src/pm/smpd/sock/iocp/smpd_util_sock.c | Provide a rich set of error codes as in mpid/common/sock |
| src/pm/smpd/sock/poll/smpd_sock_misc.i | This routine is misnamed; it is really get_interface_name (in the case where there are several networks available to the calling process, this picks one but even in the current code can pick a different interface if a particular environment variable is set) . This routine is used in smpd so we should not change its name yet. |
| src/pm/smpd/sock/poll/smpd_sock_misc.i | Is this documented? How does it work if the process manager cannot give each process a different value for an environment name? What if a different interface is needed? |
| src/pm/smpd/sock/poll/smpd_sock_misc.i | What is this for? It appears to be used in debug printing and in smpd |
| src/pm/smpd/sock/poll/smpd_sock_misc.i | This function violates the internationalization design by using English language strings rather than the error translation mechanism. This unnecessarily breaks the goal of allowing internationalization. Read the design documentation and if there is a problem, raise it rather than ignoring it. |
| src/pm/smpd/sock/poll/smpd_sock_misc.i | This appears to only be used in smpd |
| src/pm/smpd/sock/poll/smpd_sock_misc.i | It appears that this function was used instead of making use of the existing MPI-2 features to extend MPI error classes and code, of to export messages to non-MPI application (smpd?) |
| src/pm/smpd/sock/poll/smpd_sock_wait.i | return code? If an error occurs do we return it instead of the error specified in the event? |
| src/pm/smpd/sock/poll/smpd_util_sock.c | What do these mean? Why is 32 a good size (e.g., is it because 32*32 = 1024 if these are bits in a 4-byte int? In that case, should these be related to a maximum processor count or an OS-defined fd limit? |
| src/pm/smpd/sock/poll/smpd_util_sock.c | Why aren't these static |
| src/pm/smpd/sock/poll/smpd_util_sock.c | Why are these files included in this way? Why not make them either separate files or simply part of (one admittedly large) source file? |
| src/pm/smpd/sock/poll/smpd_socki_util.i | These need to separate the operations from the thread-related synchronization to ensure that the code that is independent of threads is always the same. Also, the thread-level check needs to be identical to all others, and there should be an option, possibly embedded within special thread macros, to allow runtime control of the thread level |
| src/pm/smpd/sock/poll/smpd_socki_util.i | Low usage operations like this should be a function for better readability, modularity, and code size |
| src/pm/smpd/sock/poll/smpd_socki_util.i | Are these really optional? Based on their definitions, it looks like they should only be used when debugging the code. |
| src/pm/smpd/sock/poll/smpd_socki_util.i | Should this use the CHKPMEM macros (perm malloc)? |
| src/pm/smpd/sock/poll/smpd_socki_util.i | move last element into current position and update sock associated with last element. |
| src/pm/smpd/sock/poll/smpd_socki_util.i | Shouldn't this be an mpi error code? |
| src/pm/smpd/sock/poll/smpd_socki_util.i | Who allocates eventq tables? Should there be a check that these tables are empty first? |
| src/pm/smpd/sock/poll/smpd_socki_util.i | Is this the name that we want to use (this was chosen to match the original, undocumented name) |
| src/pm/smpd/sock/poll/smpd_sock_post.i | Provide an overview for the functions in this file |
| src/pm/smpd/sock/poll/smpd_sock_post.i | It would be better to include a special formatting clue for system error messages (e.g., %dSE; in the recommended revision for error reporting (that is, value (errno) is an int, but should be interpreted as an System Error string) |
| src/pm/smpd/sock/poll/smpd_sock_post.i | What does this routine do? Why does it take a host description instead of an interface name or address? |
| src/pm/smpd/sock/poll/smpd_sock_post.i | strtok may change the contents of host_description. Shouldn't the host description be a const char [] and not modified by this routine? |
| src/pm/smpd/sock/poll/smpd_sock_post.i | For ipv6, we should use getaddrinfo |
| src/pm/smpd/sock/poll/smpd_sock_post.i | Set error |
| src/pm/smpd/sock/poll/smpd_sock_post.i | Use the parameter interface and document this |
| src/pm/smpd/sock/poll/smpd_sock_post.i | What does this function do? |
| src/pm/smpd/sock/poll/smpd_sock_set.i | Move the thread-specific operations into thread-specific routines (to allow for alternative thread sync models and for runtime control of thread level) |
| src/pm/smpd/sock/poll/smpd_sock_immed.i | Why is this the _immed file (what does immed stand for?) |
| src/pm/smpd/sock/poll/smpd_sock_immed.i | What do any of these routines do? What are the arguments? Special conditions (see the FIXME on len = SSIZE_MAX)? preconditions? postconditions? |
| src/pm/smpd/sock/poll/smpd_sock_immed.i | What does this function do? What are its arguments? It appears to execute a nonblocking accept call |
| src/pm/smpd/sock/poll/smpd_sock_immed.i | Either use the syscall macro or correctly wrap this in a test for EINTR |
| src/pm/smpd/sock/poll/smpd_sock_immed.i | There should be a simpler macro for reporting errno messages |
| src/pm/smpd/sock/poll/smpd_sock_immed.i | Who sets the socket buffer size? Why isn't the test made at that time? |
| src/pm/smpd/sock/poll/smpd_sock_immed.i | multiple passes should be made if len > SSIZE_MAX and nb == SSIZE_MAX |
| src/pm/smpd/sock/poll/smpd_sock_immed.i | This is a scary test/assignment. It needs an explanation (presumably that this routine will be called again if len is shortened. However, in that case, the description of the routine (which is also missing!!!!) needs to be very clear about this requirement. |
| src/pm/smpd/sock/poll/smpd_sock_immed.i | multiple passes should be made if len > SSIZE_MAX and nb == SSIZE_MAX |
| src/pm/smpd/sock/poll/smpd_sock_immed.i | We need (1) a standardized test for including multithreaded code and (2) include support for user requests for a lower-level of thread safety. Finally, things like this should probably be implemented as an abstraction (e.g., wakeup_progress_threads?) rather than this specific code. |
| src/pm/smpd/sock/poll/smpd_sock_init.i | The usual missing documentation (what are these routines for? preconditions? who calls? post conditions? |
| src/pm/smpd/sock/poll/smpd_sock_init.i | Who calls? When? Should this be a finalize handler instead? |
| src/pm/smpd/sock/poll/smpd_util_socki.h | !!! |
| src/pm/remshell/mpiexec.c | The singleton code goes here |
| src/pm/remshell/mpiexec.c | This should be part of the PMI initialization in the clients |
| src/pm/gforker/mpiexec.c | The following should be a single routine in pmiport |
| src/pm/gforker/mpiexec.c | This should be part of the PMI initialization in the clients |
| src/pm/util/env.c | This should be getGenv (so allocation/init in one place) |
| src/pm/util/process.c | kill children |
| src/pm/util/process.c | kill children |
| src/pm/util/process.c | Should initialize with the number of created processes |
| src/pm/util/process.c | Indicate whether all processes have exited. Then mpiexec programs can decide (probably based on a debugging flag) what to do if they have not all exited. |
| src/pm/util/process.c | This is an ugly hack to ensure that the macros to manipulate the cpu_set_t are defined - without these, you can't use the affinity routines |
| src/pm/util/process.c | How do we bind the threads when we don't have direct access to them? |
| src/pm/util/pmiserv.h | We need to specify a minimum length for both key and value. The "business cards" used by the ch3 channel implementations for sockets and for sockets+shared memory can exceed 128 characters. |
| src/pm/util/pmiserv.h | This is a temporary declaration (non-conforming name). Who added this, and why? |
| src/pm/util/simple_pmiutil2.c | Decide what role PMIU_printf should have (if any) and select the appropriate MPIU routine |
| src/pm/util/simple_pmiutil2.c | Make this an optional output |
| src/pm/util/cmnargs.c | Get values from the environment first. Command line options override the environment |
| src/pm/util/cmnargs.c | Move this routine else where; perhaps a pmutil.c? |
| src/pm/util/cmnargs.c | handle sign, invalid input |
| src/pm/util/cmnargs.c | handle negative increments, and e not s + k incr |
| src/pm/util/pmiport.c | simple_pmi should *never* start mpiexec with a bogus interface name |
| src/pm/util/ioloop.c | Occassionally, data from stdout has been lost when the job is exiting. I don't know whether data is being lost because the writer is discarding it or the reader (mpiexec) is failing to finish reading from all of the sockets before exiting. |
| src/pm/util/ioloop.c | an EINTR may also mean that a process has exited (SIGCHILD). If all processes have exited, we may want to exit |
| src/pm/util/pmiserv.c | We may need to record some information, such as the curPMIGroup, in the pState or pmiprocess entry |
| src/pm/util/pmiserv.c | What should be the defaults for the spawned env? Should the default be the env ov the spawner? |
| src/pm/util/pmiserv.c | Otherwise, we have a problem |
| src/pm/util/pmiserv.c | call user-specified info handler, if any. Unspecified info keys are ignored |
| src/pm/util/pmiserv.c | Make this an optional output |
| src/pm/util/rm.c | need path and external designation of file names |
| src/pm/hydra/mpl/include/mpltrmem.h | Consider an option of specifying __attribute__((malloc)) for gcc - this lets gcc-style compilers know that the returned pointer does not alias any pointer prior to the call. |
| src/pm/hydra/mpl/src/mplstr.c | Assumes ASCII |
| src/pm/hydra/mpl/src/mpltrmem.c | We should use generalized parameter handling here to allow use of the command line as well as environment variables |
| src/pm/hydra/mpl/src/mpltrmem.c | why do we skip the first few ints? [goodell@] |
| src/pm/hydra/mpl/src/mpltrmem.c | why is this +1 here? |
| src/pm/hydra/mpl/src/mplenv.c | We need to provide a way to signal this error |
| src/pm/hydra/utils/sock/sock.c | Comparing the hostname to "127.*" does not seem like a good way of checking if a hostname is remotely accessible |
| src/pm/hydra/tools/topo/hwloc/hwloc/include/hwloc.h | agregate nbobjs from different levels? |
| src/pm/hydra/tools/topo/hwloc/hwloc/include/hwloc/rename.h | hwloc_ffsl may be a macro, but it may not be defined yet |
| src/pm/hydra/tools/topo/hwloc/hwloc/include/hwloc/rename.h | hwloc_flsl may be a macro, but it may not be defined yet |
| src/pm/hydra/tools/topo/hwloc/hwloc/include/hwloc/helper.h | what if !root->cpuset? |
| src/pm/hydra/tools/topo/hwloc/hwloc/include/hwloc/helper.h | agregate nbobjs from different levels? |
| src/pm/hydra/tools/topo/hwloc/hwloc/include/hwloc/helper.h | what if !root->cpuset? |
| src/pm/hydra/tools/topo/hwloc/hwloc/include/hwloc/helper.h | what if !root->cpuset? |
| src/pm/hydra/tools/topo/hwloc/hwloc/include/hwloc/helper.h | what if !root->cpuset? |
| src/pm/hydra/tools/topo/hwloc/hwloc/src/cpuset.c | add hwloc_bitmap_alloc_size() + hwloc_bitmap_init_allocated() to avoid malloc? |
| src/pm/hydra/tools/topo/hwloc/hwloc/src/topology-linux.c | drop |
| src/pm/hydra/tools/topo/hwloc/hwloc/src/topology-linux.c | get the right DMI info of each machine |
| src/pm/hydra/tools/topo/hwloc/hwloc/src/topology-solaris.c | drop |
| src/pm/hydra/tools/bootstrap/utils/bscu_wait.c | We rely on gettimeofday here. This needs to detect the timer type available and use that. Probably more of an MPL functionality than Hydra's. |
| src/pm/hydra/tools/bootstrap/external/pbs_wait.c | We rely on gettimeofday here. This needs to detect the timer type available and use that. Probably more of an MPL functionality than Hydra's. |
| src/pm/hydra/tools/demux/demux.c | The below read() check for STDIN validity is somehow interfering in the case where the application is run in the background, but expects some STDIN. The correct behavior is to close the STDIN socket for the application in that case. However, this seems to hang. I'm still investigating this case. In the meanwhile, I have commented out this check. PLEASE DO NOT DELETE. All other cases dealing with STDIN seem to be working correctly. |
| src/pm/hydra/pm/pmiserv/pmiserv_utils.c | For dynamic processes, we give a process mapping that does not provide any locality information. This is required as our calculation of the locality below is static and is only valid for PGID 0 |
| src/pm/hydra/pm/pmiserv/pmiserv_cb.c | If the list of dead processes does not fit inside a single value length, set it as a multi-line value. |
| src/pm/hydra/pm/pmiserv/pmip_cb.c | This should really be a "FIXME" for the client, since there's not much we can do on the server side. We initialize to whatever PMI version we detect while reading the PMI command, instead of relying on what the init command gave us. This part of the code should not know anything about PMI-1 vs. PMI-2. But the simple PMI client-side code in MPICH2 is so hacked up, that commands can arrive out-of-order and this is necessary. This was discussed in the group and we felt that it is unsafe to change the order of the PMI command arrival in the client code (even if we are really correcting it), since other PMs might rely on the "incorrect order of commands". |
| src/pm/hydra/pm/pmiserv/pmip_cb.c | This dual memcpy is crazy and needs to be fixed. Single memcpy should be possible, but we need to be a bit careful not to corrupt the buffer. |
| src/pm/hydra/pm/pmiserv/pmip_cb.c | This code needs to change from sending the SIGUSR1 signal to a PMI-2 notification message. |
| src/pm/hydra/pm/pmiserv/pmip_cb.c | This code needs to change from sending the signal to a PMI-2 notification message. |
| src/pm/hydra/pm/pmiserv/pmip_pmi_v2.c | We cannot abort when we are not able to send data downstream. The upper layer needs to handle this based on whether we want to abort or not. |
| src/pm/hydra/pm/pmiserv/pmip_pmi_v2.c | allow for multiple ranks per PMI ID |
| src/pm/hydra/pm/pmiserv/pmip.c | Have a non-zero delay in retries; possibly have a larger delay for larger ranks |
| src/pm/hydra/pm/pmiserv/pmip_pmi_v1.c | We cannot abort when we are not able to send data downstream. The upper layer needs to handle this based on whether we want to abort or not. |
| src/pm/hydra/pm/pmiserv/pmip_pmi_v1.c | allow for multiple ranks per PMI ID |
| src/pmi/smpd/smpd_ipmi.c | Send the actual exe name |
| src/pmi/smpd/smpd_ipmi.c | Can we send a pid_t as an int ? |
| src/pmi/smpd/smpd_ipmi.c | Currently only used for singleton init -- mostly only one pair of (key, val) . Inefficient for large number of (key,val)s |
| src/pmi/smpd/smpd_ipmi.c | Switch to PMI v2 to recognize non-MPICH2 mpiexecs |
| src/pmi/smpd/smpd_ipmi.c | Reduce size of rank_str & size_str |
| src/pmi/smpd/smpd_ipmi.c | Make sure the newly created mpiexec process is also killed in the case of an error |
| src/pmi/smpd/smpd_ipmi.c | On failure do we have a local KVS ? |
| src/pmi/smpd/smpd_ipmi.c | Do we need a check for PMI_KVS to determine if client is singleton ? |
| src/pmi/smpd/smpd_ipmi.c | hack - Is there a better way ? |
| src/pmi/smpd/smpd_ipmi.c | Get rid of this hack - we already create local KVS for all singleton clients by default |
| src/pmi/smpd/smpd_ipmi.c | Test only for singleton init proc |
| src/pmi/smpd/smpd_ipmi.c | We don't support user environment infos for spawn() |
| src/pmi/smpd/smpd_ipmi.c | Should it be an error to call setup_name_service twice? |
| src/pmi/smpd/smpd_ipmi.c | Why is this func defined here ? - shouldn't this be in smpd_util*.lib ? |
| src/pmi/pmi2/simple2pmi.c | Why is setvbuf commented out? |
| src/pmi/pmi2/simple2pmi.c | What if the output should be fully buffered (directed to file)? unbuffered (user explicitly set?) |
| src/pmi/pmi2/simple2pmi.c | overall need a better interface for building commands! Need to be able to append commands, and to easily accept integer valued arguments. Memory management should stay completely out of mind when writing a new PMI command impl like this! |
| src/pmi/pmi2/simple2pmi.c | we are somehow still leaking some of this memory |
| src/pmi/pmi2/simple2pmi.c | casts are here to suppress legitimate constness warnings |
| src/pmi/pmi2/simple2pmi.c | Check for valid integer after : |
| src/pmi/pmi2/simple_pmiutil.c | Decide what role PMI2U_printf should have (if any) and select the appropriate PMI2U routine |
| src/pmi/pmi2/simple_pmiutil.c | Make this an optional output |
| src/pmi/simple/simple_pmi.c | Why is setvbuf commented out? |
| src/pmi/simple/simple_pmi.c | What if the output should be fully buffered (directed to file)? unbuffered (user explicitly set?) |
| src/pmi/simple/simple_pmi.c | Why does this depend on their being a port??? |
| src/pmi/simple/simple_pmi.c | What is this for? |
| src/pmi/simple/simple_pmi.c | This should use a cmd/response rather than a expecting the server to set a value in this and only this case |
| src/pmi/simple/simple_pmi.c | And it most ceratainly should not happen *before* the initialization handshake |
| src/pmi/simple/simple_pmi.c | This is something that the PM should tell the process, rather than deliver it through the environment |
| src/pmi/simple/simple_pmi.c | My name should be cached rather than re-acquired, as it is unchanging (after singleton init) |
| src/pmi/simple/simple_pmi.c | We need to support a distinct kvsname for each process group |
| src/pmi/simple/simple_pmi.c | This should have used rc and msg |
| src/pmi/simple/simple_pmi.c | Do correct error reporting |
| src/pmi/simple/simple_pmi.c | Do correct error reporting |
| src/pmi/simple/simple_pmi.c | (protocol design flaw): command line arguments may contain both = and <space> (and even tab!). |
| src/pmi/simple/simple_pmi.c | This mixes init with get maxes |
| src/pmi/simple/simple_pmi.c | This is an example of an incorrect fix - writeline can change the second argument in some cases, and that will break the const'ness of request. Instead, writeline should take a const item and return an error in the case in which it currently truncates the data. |
| src/pmi/simple/simple_pmi.c | Use a valid hostname |
| src/pmi/simple/simple_pmi.c | We need to support a distinct kvsname for each process group |
| src/pmi/simple/simple_pmi.c | Check for valid integer after : |
| src/pmi/simple/simple_pmiutil.c | Decide what role PMIU_printf should have (if any) and select the appropriate MPIU routine |
| src/pmi/simple/simple_pmiutil.c | Make this an optional output |
| src/nameserv/file/file_nameserv.c | Determine if the directory exists before trying to create it |
| src/nameserv/file/file_nameserv.c | An error. Ignore most ? For example, ignore EEXIST? |
| src/nameserv/file/file_nameserv.c | This should use internationalization calls |