| File | FIXME Note |
|---|---|
| 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/win_set_attr.c | communicator of window? |
| 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/async.c | The CS_ENTER/EXIT code should be abstracted out correctly, instead of relying on the #if protection 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 | This severly degrades performance but fixes alignment issues with the datatype code. |
| src/mpi/init/initthread.c | The call to err init should be within an ifdef HAVE_ ERROR_CHECKING block (as must all uses of Err_create_code) |
| src/mpi/init/initthread.c | the default behavior for all MPI routines is to abort. This isn't always convenient, because there's no other way to get this routine to simply return. But we should provide some sort of control for that and follow the default defined by the standard |
| src/mpi/init/initthread.c | Define these in the interface. Does Timer init belong here? |
| src/mpi/init/initthread.c | Can we get away without locking every time. Now, we need a MPID_CS_ENTER/EXIT around MPI_Init and MPI_Init_thread. Progress may be called within MPI_Init, e.g., by a spawned child process. Within progress, the lock is released and reacquired when blocking. If the lock isn't acquired before then, the release in progress is incorrect. Furthermore, if we don't release the lock after progress, we'll deadlock the next time this process tries to acquire the lock. MPID_CS_ENTER/EXIT functions are used here instead of MPIU_THREAD_CS_ENTER/EXIT because MPIR_ThreadInfo.isThreaded hasn't been initialized yet. |
| src/mpi/init/init.c | Can we get away without locking every time. Now, we need a MPID_CS_ENTER/EXIT around MPI_Init and MPI_Init_thread. Progress may be called within MPI_Init, e.g., by a spawned child process. Within progress, the lock is released and reacquired when blocking. If the lock isn't acquired before then, the release in progress is incorrect. Furthermore, if we don't release the lock after progress, we'll deadlock the next time this process tries to acquire the lock. MPID_CS_ENTER/EXIT functions are used here instead of MPIU_THREAD_SINGLE_CS_ENTER/EXIT because MPIR_ThreadInfo.isThreaded hasn't been initialized yet. |
| 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 | Both the memory tracing and debug nesting code blocks should be finalize callbacks |
| 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/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_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.c | Bubble sort |
| 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 local functions for the collectives |
| src/mpi/comm/commutil.c | No local functions for the topology routines |
| src/mpi/comm/commutil.c | TEMP for current yield definition |
| 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/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/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/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/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 | 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 | These functions are not thread safe |
| src/mpi/errhan/errutil.c | default is not thread safe |
| 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 | 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 | 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 flag wasn't documented in the release specs, and in any event shouldn't be controlled through source-code changes (i.e., make it either a configure option or a runtime option) |
| src/mpi/errhan/errutil.c | This is wrong. The only way that you can get here without errcode beign 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 * ? |
| 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 | Remove the bogus fn argument from all uses of this routine |
| src/mpi/errhan/errutil.c | How do we determine that we failed to unwind the stack? |
| 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_code.c | verify that errorclass is a dynamic class |
| src/mpi/errhan/file_call_errhandler.c | check for a valid file handle (fh) before converting to a pointer |
| src/mpi/errhan/file_call_errhandler.c | Is this obsolete now? |
| src/mpi/errhan/comm_set_errhandler.c | Add a check that this is a comm errhandler. This test should be a macro so that it can be used in all calls, along with similar calls for win and file |
| src/mpi/datatype/type_match_size.c | Should make use of Fortran optional types (e.g., MPI_INTEGER2) |
| src/mpi/datatype/register_datarep.c | UNIMPLEMENTED |
| 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/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 NMPI_Barrier 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_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/mpioimpl.h | We should be more restricted in what we include from MPICH2 into ROMIO |
| src/mpi/romio/mpi-io/write_ordb.c | Check for error code from WriteStridedColl? |
| src/mpi/romio/mpi-io/seek_sh.c | explain why the barrier is necessary |
| src/mpi/romio/mpi-io/write_ord.c | Check for error code from WriteStridedColl? |
| src/mpi/romio/mpi-io/get_extent.c | handle other file data representations |
| src/mpi/romio/mpi-io/iotestall.c | THIS IS NOT CORRECT (see above). But most applications won't care |
| src/mpi/romio/mpi-io/close.c | It is wrong to use MPI_Barrier; the user could choose to re-implement MPI_Barrier in an unexpected way. Either use NMPI_Barrier as in MPICH2 or PMPI_Barrier |
| 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/strfns.c | Really need a check for varargs.h vs stdarg.h |
| src/mpi/romio/adio/common/strfns.c | Assumes ASCII |
| src/mpi/romio/adio/common/ad_init.c | should be checking error code from MPI_Info_create here |
| src/mpi/romio/adio/common/req_malloc.c | Insert error 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/topo/cart_create.c | nranks could be zero (?) |
| src/mpi/topo/dist_gr_neighb.c | Why does this routine need 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_count.c | Why does this routine require a CS? |
| 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/debugger/dll_mpich2.c | include the mpichconf.h file? |
| src/mpi/debugger/dll_mpich2.c | This function is not yet implemented |
| src/mpi/debugger/dll_mpich2.c | handle size properly (declared as 64 in mpi_interface.h) |
| 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/dbginit.c | Make this thread-safe |
| src/mpi/debugger/dbginit.c | (MT) Ensure thread-safe |
| src/mpi/debugger/dbginit.c | (MT) Ensure thread-safe |
| src/mpi/debugger/tvtest.c | Move these into dbgstub.c |
| src/mpi/pt2pt/ibsend.c | There should be no unreferended 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/greq_start.c | MT this function is not thread safe when using fine-grained threading |
| src/mpi/pt2pt/bsend.c | Check for MPID_WOULD_BLOCK? |
| 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/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/mpir_request.c | Why do we need to get the thread-private storage here? |
| src/mpi/pt2pt/iprobe.c | Is this correct for intercomms? |
| src/mpi/pt2pt/bsendutil.c | This is the wrong error text (notimpl) |
| src/mpi/pt2pt/bsendutil.c | |
| src/mpi/spawn/comm_disconnect.c | MT should we be checking this? |
| src/mpi/spawn/open_port.c | If info_ptr is non-null, we should validate it |
| 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/coll/red_scat.c | should we be checking the op_errno here? |
| src/mpi/coll/red_scat.c | this version only works for power of 2 procs |
| src/mpi/coll/reduce.c | does this need to be checked after each uop invocation for predefined operators? |
| src/mpi/coll/reduce.c | does this need to be checked after each uop invocation for predefined operators? |
| src/mpi/coll/allreduce.c | mpi_errno is error CODE, not necessarily the error class MPI_ERR_OP. In MPICH2, we can get the error class with errorclass = mpi_errno & ERROR_CLASS_MASK; |
| 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 | Is MPI_Fint really OK here? |
| src/mpi/coll/bcast.c | move to somewhere else |
| src/mpi/coll/bcast.c | it would be nice if we could refactor things to minimize duplication between this and MPIR_Scatter and friends. We can't use MPIR_Scatter 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/allgather.c | saving an MPI_Aint into an int |
| src/mpi/coll/allgather.c | saving an MPI_Aint into an int |
| src/mpi/timer/mpidtime.c | We need to cleanup the use of the MPID_Generic_wtick prototype |
| 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 | the precise meaning of this field is unclear, comments/docs about it should be added |
| 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/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 | Unused |
| src/mpid/ch3/include/mpidpkt.h | Experimental for now |
| 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 | This must be used within an MPIDCOMM critical section. |
| 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 | MPIDI_PG_Get_vc_set_active is a macro, not a routine |
| 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 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 | Currently forcing the PER_OBJECT case to do a global lock. We need a better way of fixing this. |
| 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/util/shmbase/ch3_shm.c | |
| 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_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 and channels/ssm/ch3_progress_connect.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. See also channels/ssm/ch3_progress_connect.c |
| 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/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/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/shmproc/shmproc.c | Do we need these routines for all shmem or only for some options? |
| src/mpid/ch3/util/shmproc/shmproc.c | Now what? Why not continue to read? |
| src/mpid/ch3/util/shm/ch3i_bootstrapq.c | Are these needed here (these were defined within an unrelated ifdef in mpidimpl.h) |
| src/mpid/ch3/util/shm/ch3i_bootstrapq.c | These routines allocate memory but rarely free it. They need a finalize handler to remove the allocated memory. A clear description of the purpose of these routines and their scope of use would make that easier. |
| src/mpid/ch3/util/shm/ch3i_bootstrapq.c | These routines should *first* decide on the method of shared memory and queue support, then implement the Bootstrap interface in terms of those routines, if necessary using large (but single) ifdef regions rather than the constant ifdef/endif code used here. In addition, common service routines should be defined so that the overall code can simply |
| src/mpid/ch3/util/shm/ch3i_bootstrapq.c | These routines need a description and explanation. For example, where is the bootstrap q pointer exported? Is there more than one bootstrap queue? |
| src/mpid/ch3/util/shm/ch3i_bootstrapq.c | What does this routine do? |
| src/mpid/ch3/util/shm/ch3i_bootstrapq.c | This memory is not freed |
| src/mpid/ch3/util/shm/ch3i_shm_bootstrapq.c | What is this routine for? Why does this routine duplicate so much of the code in the MPIDI_CH3I_SHM_Get_mem function (but not exactly; e.g., is there a reason that this routine and the Get_mem version use slightly different arguments to the shm_open routine and take different action on failure? |
| src/mpid/ch3/util/shm/ch3u_connect_sshm.c | Packet types used for establishing connections |
| src/mpid/ch3/util/shm/ch3u_connect_sshm.c | What does this routine do? |
| src/mpid/ch3/util/shm/ch3u_connect_sshm.c | It appears that this routine is used as part of the initial "get the peers connected" code in comm_connect/accept and spawn |
| src/mpid/ch3/util/shm/ch3u_connect_sshm.c | where does this vc get freed? |
| src/mpid/ch3/util/shm/ch3u_connect_sshm.c | Do we want to free the vc instead? Or put this into the fail block? |
| src/mpid/ch3/util/shm/ch3u_connect_sshm.c | Non conforming (not internationalizable) error message. why not just MPIU_ERR_POP? |
| src/mpid/ch3/util/shm/ch3u_finalize_sshm.c | Should this be registered as a finalize handler? Should there be a corresponding abort handler? |
| src/mpid/ch3/util/shm/ch3u_init_sshm.c | This file contains too many ifdefs. A cleaner design is needed; examples may be found in MPICH1. For example, separate files for each type of shared memory and common routines implementing a shared memory abstraction |
| src/mpid/ch3/util/shm/ch3u_init_sshm.c | Why is this commented out? |
| src/mpid/ch3/util/shm/ch3u_init_sshm.c | This code probably reflects a bug caused by commenting out the above code |
| src/mpid/ch3/util/shm/ch3u_init_sshm.c | MPIU_g_nLockSpinCount is in a different module (mpid/common/locks) where it is used (and defined) only in some cases. Commenting out this bogus access for now; uncommenting it may cause link failures. Update - see FIXME in mpidu_process_locks - there are serious problem with how configuration information is passed to mpidu_process_locks.h , leading to *different* choices in the files in mpid/ch3 that include it (like this one) and in mpidu_process_locks.c (!). For that reason, I've left this statement in the code, but once mpidu_process_locks is fixed, this will need to be as well. |
| src/mpid/ch3/util/shm/ch3u_init_sshm.c | Is this initialization correct (it should have been done when the pg was created. If not, then the process by which channels and PG's are initialized needs to be documented and explained! An alternate approach to having this done after the PG is created is to define a method initialization routine that is called after the pg is created but before the final, channel-specific virtual connection initialization is done. |
| 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/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/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/include/mpidi_ch3_pre.h | we should really use the copy of these values that is stored in the MPID_Comm structure |
| 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_defs.h | This definition should be gotten from mpidi_ch3_impl.h |
| 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_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/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_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_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_finalize.c | I removed this PMI_Barrier in PMI2. The barrier seems not to be necessary, but I'm keeping this here until I'm convinced that this isn't here to handle some timing-related bug. DARIUS |
| 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_barrier.c | this is not a scalable algorithm because everyone is polling on the same cacheline |
| 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/tcp_getip.c | This more-or-less duplicates the code in ch3/util/sock/ch3u_getintefaces.c , which is already more thoroughly tested and more portable than the code here. |
| 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/tcp/tcp_impl.h | How do we find the max length of a bc? |
| 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/socksm.c | retry 'n' number of retries before signalling an error to VC layer. |
| 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/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/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_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_send.c | match_info is used uninitialized |
| 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/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/wintcp/socksm.c | trace/log all the state transitions |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | All 'sc's in the sc_tbl should be treated alike. Using the info that (i=0) is a listening fd is an unnecessary/ugly dependency |
| 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 | Z1 |
| 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 | Z1 |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Z1 |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Remove this function - no longer used |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.c | Remove this func - no longer used |
| 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 | 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 | retry 'n' number of retries before signalling an error to VC layer. |
| 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 | Rename the status type to look like a non-MACRO |
| 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/socksm.h | Remove assumption that mpi_errno is defined in caller |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.h | should plfd really be const const? Some handlers may need to change the plfd entry. |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/socksm.h | bc actually contains port_name info |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/wintcp/wintcp_init.c | Why happens on an error ? |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/newmad/newmad_send.c | issend ! |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/newmad/newmad_send.c | issend ! |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/newmad/newmad_probe.c | |
| src/mpid/ch3/channels/nemesis/nemesis/netmod/newmad/newmad_probe.c | |
| 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/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/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/shm/src/ch3_finalize.c | This should be used in abort as well |
| src/mpid/ch3/channels/shm/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/shm/src/ch3_init.c | the vc's must be set to active for the close protocol to work in the shm channel |
| src/mpid/ch3/channels/shm/src/ch3_init.c | Should the malloc be within the init? |
| src/mpid/ch3/channels/shm/src/ch3_init.c | Need to free this request when the vc is removed |
| src/mpid/ch3/channels/shm/src/ch3_init.c | Should these also be set in the VC_Init, or is VC_Init moot (never called because this channel does not support dynamic processes?) |
| src/mpid/ch3/channels/shm/src/ch3_init.c | Figure out whether this is a common feature of process groups (and thus make it part of the general PG_Init) or something else. Avoid a "get" routine because of the danger in using "get" where "dup" is required. |
| src/mpid/ch3/channels/shm/src/ch3_init.c | Move this code to the general init pg for shared memory |
| src/mpid/ch3/channels/shm/src/ch3_init.c | This should call a routine from the ch3/util/shm directory to initialize the use of shared memory for processes WITHIN this process group |
| src/mpid/ch3/channels/shm/include/mpidi_ch3_impl.h | We need to ensure read/write barriers for correctness, if the processor might possibly reorder |
| src/mpid/ch3/channels/shm/include/mpidi_ch3_impl.h | The ssm channel uses these names instead. We'll switch to them eventually (since there is a great deal of commonality between the ssm and shm code, and that commonality should be reflected in common support routines) |
| src/mpid/ch3/channels/ssm/src/ch3_progress_connect.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. See also channels/sock/ch3_progress.c |
| src/mpid/ch3/channels/ssm/src/ch3_progress_sock.c | This is nowhere set to true. The name is non-conforming if it is not static |
| src/mpid/ch3/channels/ssm/src/ch3_progress_sock.c | the following should be handled by the close protocol |
| src/mpid/ch3/channels/ssm/src/ch3_progress_sock.c | where does this vc get freed? |
| src/mpid/ch3/channels/ssm/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/ssm/src/ch3_progress.c | It looks like this is a relic of code to occassional call sleep to implement a yield. As no code defined the use sleep yield macro, that code was removed, exposing this odd construction. |
| src/mpid/ch3/channels/ssm/src/ch3_progress.c | copied from sock # if (USE_THREAD_IMPL == MPICH_THREAD_IMPL_GLOBAL_MUTEX) { MPID_Thread_cond_create(&MPIDI_CH3I_progress_completion_cond, NULL); } # endif |
| src/mpid/ch3/channels/ssm/src/ch3_progress.c | Cleanly shutdown other socks and MPIU_Free connection structures. (close protocol?) |
| src/mpid/ch3/channels/ssm/src/ch3_progress.c | copied from sock # if (USE_THREAD_IMPL == MPICH_THREAD_IMPL_GLOBAL_MUTEX) { MPID_Thread_cond_destroy(&MPIDI_CH3I_progress_completion_cond, NULL); } # endif |
| src/mpid/ch3/channels/ssm/src/ch3_progress.c | What is this routine for? |
| src/mpid/ch3/channels/ssm/src/ch3_progress.c | This is a temp to free memory allocated in this file |
| src/mpid/ch3/channels/ssm/src/ch3_isend.c | Integrate this with ch3_isendv.c so that there are fewer files, and so that common code is in one place and not duplicated (or almost duplicated) across files |
| src/mpid/ch3/channels/ssm/src/ch3_isend.c | For fault tolerance, retry the operation before marking the connection as failed |
| src/mpid/ch3/channels/ssm/src/ch3_init.c | These VC debug print routines belong in mpid_vc.c, with channel-specific code in an appropriate file. Much of that channel-specific code might belong in ch3/util/sock or ch3/util/shm |
| src/mpid/ch3/channels/ssm/src/ch3_init.c | This doesn't allocate and only inits one field. Is this now part of the channel-specific hook for channel-specific VC info? |
| src/mpid/ch3/channels/ssm/src/ch3_init.c | This should call a routine from the ch3/util/shm directory to initialize the use of shared memory for processes WITHIN this process group |
| src/mpid/ch3/channels/ssm/include/mpidi_ch3_impl.h | We should only include the ones of these that we use within this channel |
| src/mpid/ch3/channels/ssm/include/mpidi_ch3_impl.h | Note the use of the USE_SYSV_MQ to only include this header includes when needed. The other headers should be similarly organized |
| src/mpid/ch3/channels/ssm/include/mpidi_ch3_impl.h | We need to move these queue routines out of the header files to allow dynamic loading/selection of channels, and to provide better modularity of the ch3 code |
| src/mpid/ch3/channels/ssm/include/mpidi_ch3_impl.h | Why 100? |
| src/mpid/ch3/channels/ssm/include/mpidi_ch3_impl.h | These should be known only by the code that is managing the business cards |
| src/mpid/ch3/channels/ssm/include/mpidi_ch3_post.h | These sizes need to be documented at least, and it should be easier to change them, at least at configure/build time |
| src/mpid/ch3/channels/ssm/include/mpidi_ch3_post.h | Are these the same definitions used by all channels? If so, they should be defined in the same place (and it would make sense for the progress routines in the ch3 device to have the same interface independent of channel) |
| src/mpid/ch3/channels/ssm/include/ch3i_progress.h | We need an Undefined for the tick type |
| src/mpid/ch3/channels/ssm/include/ch3i_progress.h | These common definitions (accessing the timer) should be in a single file |
| src/mpid/ch3/channels/ssm/include/ch3i_progress.h | What are these used for (possibly one of the spinwait variations?) Document the use |
| 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/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 | Try to get the environment variable that uses the rank in comm world, i.e., MPICH_INTERFACE_HOSTNAME_R_%d. For this, we'll need to know the rank for this process. |
| 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 and channels/ssm/ch3_progress_connect.c |
| src/mpid/ch3/channels/sctp/src/ch3_istartmsg.c | change error code |
| src/mpid/ch3/channels/sctp/src/sctp_common.c | assumes success |
| 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 | 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/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/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 | 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/sock/include/mpidi_ch3_pre.h | Explain these; why is this separate from the VC state? |
| 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_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/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 | Cleanly shutdown other socks and free connection structures. (close protocol?) |
| 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 | (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/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_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/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/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_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 | Who sets? Why? Where is this defined? Document. Why is this not done as part of the VC initialization? |
| src/mpid/ch3/src/ch3u_port.c | A hook should not be such a specific function; instead, it should invoke a function pointer defined in the channel interface structure |
| src/mpid/ch3/src/ch3u_port.c | If we fail to connect, someone needs to free this newcomm |
| src/mpid/ch3/src/ch3u_port.c | why is this commented out? |
| src/mpid/ch3/src/ch3u_port.c | If this is really needed, make it a destroy callback on the process group rather than an SSHM-specific item |
| src/mpid/ch3/src/ch3u_port.c | Need to understand this and either remove or make common to all channels |
| 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 | remove this ifdef - method on connection? |
| src/mpid/ch3/src/ch3u_port.c | should this be an MPIU_CALL macro? |
| 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/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_handle_connection.c | What is this routine for? It appears to be used only in ch3_progress, ch3_progress_connect, or ch3_progress_sock files. Is this a general operation, or does it belong in util/sock ? It appears to be used in multiple channels, but probably belongs in mpid_vc, along with the vc exit code that is currently in MPID_Finalize |
| src/mpid/ch3/src/ch3u_handle_connection.c | The only event is event_terminated. Should this have a different name/expected function? |
| 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 | Remove this IFDEF |
| src/mpid/ch3/src/ch3u_handle_connection.c | This situation has been seen with the ssm device, when running on a single process. It may indicate that the close protocol, for shared memory connections, can occasionally reach this state when both sides start closing the connection. We will act as if this is duplicate information can be ignored (rather than triggering the Assert in the next case) |
| src/mpid/ch3/src/ch3u_handle_connection.c | Debugging |
| 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 can't do this because fence_cnt must be updated collectively |
| src/mpid/ch3/src/ch3u_rma_sync.c | We can't do this because fence_cnt must be updated collectively |
| src/mpid/ch3/src/ch3u_rma_sync.c | MT: this has to be done atomically |
| src/mpid/ch3/src/ch3u_rma_sync.c | We can't do this because fence_cnt must be updated collectively |
| 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 | 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 | 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_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/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/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. |
| src/mpid/ch3/src/mpid_finalize.c | Using MPI_Barrier on MPI_COMM_WORLD here is dangerous. It is fine, of course, for correct programs, but incorrect programs, for examples, ones that call MPI_Barrier(MPI_COMM_WORLD) in some but not all processes will show unexpected symptoms (e.g., the otherwise Unmatched MPI_Barrier calls will match this barrier, and then MPI_Finalize will hang. To fix this, we need a separate Barrier operation, either an independent Barrier or an independent communicator that is not used by any other (user) routine. |
| 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/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 shouldn't have any references to the channel-specific fields in this part of the code. This case should actually not be needed; if there is a pending send element, the top-level state should not be inactive |
| 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 | need to handle oddeven cliques DARIUS |
| 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_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/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/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 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/mpid_irsend.c | HOMOGENEOUS SYSTEMS ONLY -- no data conversion is performed |
| 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 | 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 | info may be needed for port name |
| 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_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 | Remove this IFDEF |
| src/mpid/ch3/src/mpidi_pg.c | If the reference count for the vc is not 0, something is wrong |
| 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 | 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_irecv.c | if the user buffer is contiguous, just move the data without using a separate routine call |
| src/mpid/ch3/src/mpid_ssend.c | HOMOGENEOUS SYSTEMS ONLY -- no data conversion is performed |
| 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/ch3u_eagersync.c | This sometimes segfaults |
| 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/mpid_port.c | MPIU_xxx routines should return regular mpi error codes |
| 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/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 | 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/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/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/ch3u_handle_recv_req.c | MT: this has to be done atomically |
| src/mpid/ch3/src/ch3u_handle_recv_req.c | What is this variable for (it is never referenced)? |
| 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/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/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 | is that a ch3 or an mpid critical section? |
| 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/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_send.c | HOMOGENEOUS SYSTEMS ONLY -- no data conversion is performed |
| src/mpid/ch3/src/mpid_send.c | this is a fatal error because a sequence number has already been allocated. If sequence numbers are not being used then this could be a recoverable error. A check needs to be added that sets the error to fatal or recoverable depending on the use of sequence numbers. |
| 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_comm_spawn_multiple.c | Correct description of function |
| 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 | It makes sense to save the rank (and size) of the communicator in the window structure to speed up these operations, or to save a pointer to the communicator structure, rather than just the handle |
| src/mpid/ch3/src/ch3u_rma_ops.c | Where does this memory get freed? |
| src/mpid/ch3/src/ch3u_rma_ops.c | It makes sense to save the rank (and size) of the communicator in the window structure to speed up these operations |
| src/mpid/ch3/src/ch3u_rma_ops.c | It makes sense to save the rank (and size) of the communicator in the window structure to speed up these operations, or to save a pointer to the communicator structure, rather than just the handle |
| src/mpid/common/locks/mpidu_process_locks.c | The build scheme for this file is broken, and this definition illustrates the problem. The issue is that some files include mpidu_process_locks.h with *different* definitions than are used to compile this file! That shows up when this variable, which is only used for busy locks, is only conditionally defined when busy locks are defined. The problem is that code in, for example, the ch3:ssm channel includes mpidu_process_locks.h with USE_BUSY_LOCKS defined, but the configure in the mpid/common/locks directory will never* define USE_BUSY_LOCKS |
| src/mpid/common/locks/mpidu_process_locks.c | Why is this here? Is this a misnamed use-inline-locks? |
| src/mpid/common/locks/mpidu_process_locks.h | This inclusion is disgusting but necessary for now. Including mpidimpl.h will get us USE_BUSY_LOCKS from the ssm channel (or any other device that defines it), that's why we do it. Unfortunately, it sucks in the whole MPI headers ball of wax at the same time, and implies a potential circular dependency. Doing the right thing here basically means a total gut job, which is out of scope at this time. [goodell@ 2008-03-19] |
| src/mpid/common/locks/mpidu_process_locks.h | It should be sufficient to include mpidpre.h (updating the ssm etc devices if necessary). That's still not great, but better than all of mpidimpl.h, which should not be used outside of the device code. WDG - 8/8/08 |
| src/mpid/common/locks/mpidu_process_locks.h | Including mpidimpl.h includes mpiimpl.h, which is a superset of mpishared.h. mpishared.h should only be included when the regular mpiimpl.h headers are not used. |
| src/mpid/common/locks/mpidu_process_locks.h | First use the configure ifdefs to decide on an approach for locks. Then put all lock code in one place, or at least guarded by the same "USE_xxx" ifdef. It is nearly impossible with the current code to determine, for example, what is the definition of MPIDU_Process_lock_t. (Specifically, for the Intel compiler on an x86, it appears to be missing a volatile, needed when using the _InterlockedExchange inline function |
| src/mpid/common/locks/mpidu_process_locks.h | This uses an invalid prefix |
| 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/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_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_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/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/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/mpidu_socki.h | !!! |
| 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 | Use the parameter interface and document this |
| src/mpid/common/sock/poll/sock_post.i | What does this function do? |
| src/mpid/common/sock/poll/sock_misc.i | Who uses this and why? |
| 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/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/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/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_datatype.h | calculating this value this way is foolish, we should make this more automatic and less error prone |
| src/mpid/common/datatype/mpid_type_create_resized.c | ??? |
| src/mpid/common/datatype/mpid_datatype_contents.c | Why is this here? The value is never used. Should it be? |
| 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/dataloop/segment_ops.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/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/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/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/trace_input.c | Improve failure reporting |
| src/util/logging/rlog/rlogtime.c | This name needs to be changed to ensure no conflicts with user-defined globals |
| src/util/param/param.c | This is the code from EnvGetRange |
| src/util/param/param.c | We need to provide a way to signal this error |
| src/util/multichannel/mpi.c | Should we allow for short wrapper names like 'mpe'? |
| 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 | 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/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_process_wrappers.h | Currently this functionality is also implemented in mpidu_process_locks.h . Should we use MPIU_Thread_yield() ? |
| src/util/wrappers/mpiu_process_wrappers.h | Allow fallbacks with sleep/select |
| 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/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/dbg/dbg_control.c | Delete this file (functionality switched to MPIU_DBG_MSG interface |
| 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/mem/trmem.c | We should use generalized parameter handling here to allow use of the command line as well as environment variables |
| src/util/mem/trmem.c | why do we skip the first few ints? [goodell@] |
| src/util/mem/safestr.c | Assumes ASCII |
| src/util/mem/trmem.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/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/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/pmiport.c | simple_pmi should *never* start mpiexec with a bogus interface name |
| 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/rm.c | need path and external designation of file names |
| src/pm/util/env.c | This should be getGenv (so allocation/init in one place) |
| 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/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/smpd/smpd_iov.h | How is IOV_LIMIT chosen? |
| 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_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_printf.c | For now ... translate correctly later |
| 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/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_user_data.c | function not implemented |
| src/pm/smpd/smpd_launch_process.c | The return vals of these functions are not checked ! |
| src/pm/smpd/smpd.c | Get rid of this hack - we already create local KVS for all singleton clients by default |
| src/pm/smpd/smpd_read_write.c | why does this return success on truncation? |
| src/pm/smpd/smpd_read_write.c | Error on truncation? if (*str != '\0') { num_decoded = n; return SMPD_FAIL; } |
| src/pm/smpd/smpd_handle_spawn.c | create a mechanism to timeout spawned processes and only those spawned processes |
| 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/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_affinitize.c | Is this defined for win32 ? case RelationProcessorPackage : return prioPackage; |
| 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/mpiexec.c | Get rid of this hack - we already create local KVS for all singleton clients by default |
| 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_host_util.c | If these are not NULL should they be freed? |
| 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/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_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_util_socki.h | !!! |
| src/pm/smpd/sock/poll/smpd_sock_misc.i | Who uses this and why? |
| 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_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_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_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_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_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/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/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/remshell/mpiexec.c | |
| src/pm/hydra/ui/mpiexec/mpiexec.c | We are assuming a working model for the process manager here. We need to get how many processes the PM has launched instead of assuming this. For example, it is possible to have a PM implementation that launches separate "new" proxies on a different port and kills the original proxies using them. |
| src/pm/hydra/ui/utils/uiu.c | Should the timers be initialized? |
| src/pm/hydra/tools/bind/hwloc/hwloc/include/hwloc.h | agregate nbobjs from different levels? |
| src/pm/hydra/tools/bind/hwloc/hwloc/include/hwloc/helper.h | agregate nbobjs from different levels? |
| src/pm/hydra/tools/bind/hwloc/hwloc/src/topology-linux.c | gather page_size_kB as well? MaMI needs it |
| src/pm/hydra/tools/bind/hwloc/hwloc/src/topology-linux.c | get the right DMI info of each machine |
| src/pm/hydra/tools/bind/hwloc/hwloc/src/topology-linux.c | gather page_size_kB as well? MaMI needs it |
| src/pm/hydra/tools/bind/hwloc/hwloc/src/traversal.c | add an invalid value ? |
| src/pm/hydra/utils/timer/timer.c | Here we assume that the timer is gettimeofday. |
| src/pm/hydra/include/hydra_base.h | Is time.h available everywhere? We should probably have multiple timer options. |
| src/pm/hydra/include/hydra_utils.h | This should eventually become MPL_malloc and friends |
| src/pm/hydra/include/hydra_utils.h | HYD_time should be OS specific |
| src/pm/hydra/pm/pmiserv/pmi_handle.h | PMI-1 uses 256, PMI-2 uses 1024; we use the MAX |
| src/pm/hydra/pm/pmiserv/pmi_proxy_cb.c | shutdown should be handled more cleanly. That is, check if there are other processes still running and kill them before exiting. |
| src/pm/hydra/pm/pmiserv/pmi_serv_cb.c | Instead of doing this from this process itself, fork a bunch of processes to do this. |
| src/pm/hydra/pm/pmiserv/pmi_serv_cb.c | Instead of doing this from this process itself, fork a bunch of processes to do this. |
| src/pm/hydra/pm/pmiserv/pmi_proxy_utils.c | This function performs minimal error checking as it is not supposed to be called by the user, but rather by the process management server. It will still be helpful for debugging to add some error checks. |
| src/pm/hydra/pm/pmiserv/pmi_common.h | This structure only provides the PMI_ID, as we currently only support single job environments with no dynamic processes. When there are multiple jobs or dynamic processes, we will need a process group ID as well. |
| src/pm/hydra/pm/pmiserv/pmi_handle_v2.c | This should be an output parameter. We need to change the structure of the PMI function table to be able to take additional arguments, and not just the ones passed on the wire. |
| src/pm/hydra/mpl/src/mplstr.c | This is an approximate form, which works for most cases, but might not work for all. |
| src/pm/hydra/mpl/src/mplstr.c | Assumes ASCII |
| 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 | (protocol design flaw): command line arguments may contain both = and <space> (and even tab!). |
| src/pmi/pmi2/simple2pmi.c | Use a valid hostname |
| src/pmi/pmi2/simple2pmi.c | We need to support a distinct kvsname for each process group |
| src/pmi/pmi2/simple2pmi.c | Check for valid integer after : |
| src/pmi/pmi2/simple2pmi.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/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 | What is this function? How is it defined and used? |
| src/pmi/simple/simple_pmi.c | Check for tempbuf too short |
| 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 | Check for tempbuf too short |
| 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 |