mpi-forum / mpi-issues

Tickets for the MPI Forum
http://www.mpi-forum.org/
67 stars 7 forks source link

MPI_Win_lock lock_type not clarified enough in documentation #694

Open zerothi opened 1 year ago

zerothi commented 1 year ago

Problem

The MPI specification is, to me, not clear enough on the implications on using lock_type in combination with assert values.

For instance a code like the following

  MPI_Win win;
  if (rank == 0) {
    MPI_Win_create(buffer, BUF_SIZE*sizeof(int), sizeof(int), MPI_INFO_NULL, MPI_COMM_WORLD, &win);
  } else {
    // on all other processes we expose nothing
    MPI_Win_create(NULL, 0, 1, MPI_INFO_NULL, MPI_COMM_WORLD, &win);
  }

  if (rank != 0) {
    int target = 0;
    MPI_Win_lock(MPI_LOCK_SHARED,target,MPI_MODE_NOCHECK,win);
    MPI_Accumulate(buffer, BUF_SIZE, MPI_INT, target, 0, BUF_SIZE, MPI_INT, MPI_SUM, win);
    MPI_Win_unlock(target,win);
  }

From the description in mpi40-report.pdf Sec. 12.5.3 I only have:

Starts an RMA access epoch. The window at the process with rank rank can be accessed by RMA operations on win during that epoch. Multiple RMA access epochs (with calls to MPI_WIN_LOCK) can occur simultaneously; however, each access epoch must target a different process.

(I have bolded the problematic sentence)

To me this looks like my above code would be faulty since all rank!=0 access the same target without checking for coherence (NOCHECK). But it isn't clear from the specification whether the MPI implementor should ensure this, or I should.

Later in the same section I read

Advice to users. There may be additional overheads associated with using MPI_WIN_LOCK and MPI_WIN_LOCK_ALL concurrently on the same window. These overheads could be avoided by specifying the assertion MPI_MODE_NOCHECK when possible (see Section 12.5.5). (End of advice to users.)

This to me sounds like I should ensure coherence. Since the NOCHECK will put coherence on the shoulders of the code-writer, and not on the MPI-implementor.

All-in-all I think the clarity of the lock_type is unclear in conjunction with the assert statements.

Proposal

In Sec 12.5.3 it would be ideal if we could clear out the lock_type definitions.

Changes to the Text

In Sec 12.5.3 it would be ideal if we could clear out the lock_type definitions:

lock_type defines whether other processors may access the window at the same time.

  • MPI_LOCK_EXCLUSIVE ensures window can only be accessed by a single process at a time
  • MPI_LOCK_SHARED allows all processors to access the window at the same time

Impact on Implementations

None.

Impact on Users

Users would be able to read the MPI specification and fully grasp what lock_type variables and assert variables do to a Win_lock call.

References and Pull Requests

devreal commented 1 year ago

In the upcoming 4.1 text we moved around the description of locks to provide a better description at the beginning of Section 12.5.3. The beginning of the section of locks now reads:

Locks are used to protect accesses to the locked target window effected by RMA calls issued between the lock and unlock calls, and to protect load/store accesses to a locked local or shared memory window executed between the lock and unlock calls. Accesses that are protected by an exclusive lock (acquired using MPI_LOCK_EXCLUSIVE) will not be concurrent at the window site with other accesses to the same window that are lock protected. Accesses that are protected by a shared lock (acquired using MPI_LOCK_SHARED) will not be concurrent at the window site with accesses protected by an exclusive lock to the same window.

I acknowledge that the phrasing in the last subclause is not clear:

From the description in mpi40-report.pdf Sec. 12.5.3 I only have:

Starts an RMA access epoch. The window at the process with rank rank can be accessed by RMA operations on win during that epoch. Multiple RMA access epochs (with calls to MPI_WIN_LOCK) can occur simultaneously; however, each access epoch must target a different process.

I guess it should really read:

however, each access epoch opened by a process must target a different process.

Would that make it clearer?

To me this looks like my above code would be faulty since all rank!=0 access the same target without checking for coherence (NOCHECK). But it isn't clear from the specification whether the MPI implementor should ensure this, or I should.

AFAICT, your code is still correct. The description of the NOCHECK assert reads:

no other process holds, or will attempt to acquire, a conflicting lock, while the caller holds the window lock. This is useful when mutual exclusion is achieved by other means, but the coherence operations that may be attached to the lock and unlock calls are still required.

The shared locks are not conflicting. This should also answer your question on coherence:

This to me sounds like I should ensure coherence. Since the NOCHECK will put coherence on the shoulders of the code-writer, and not on the MPI-implementor.

The shared lock will provide coherence (i.e., releasing the shared lock provides the usual completion guarantees) but it's the application's task to ensure synchronization.

Does that answer your questions?

zerothi commented 1 year ago

Many thanks for your inputs here!

Locks are used to protect accesses to the locked target window effected by RMA calls issued between the lock and unlock calls, and to protect load/store accesses to a locked local or shared memory window executed between the lock and unlock calls. Accesses that are protected by an exclusive lock (acquired using MPI_LOCK_EXCLUSIVE) will not be concurrent at the window site with other accesses to the same window that are lock protected. Accesses that are protected by a shared lock (acquired using MPI_LOCK_SHARED) will not be concurrent at the window site with accesses protected by an exclusive lock to the same window.

This is much better. This, to me, reads like it is the MPI implementors responsibility to ensure the locks access concurrency.

I acknowledge that the phrasing in the last subclause is not clear:

From the description in mpi40-report.pdf Sec. 12.5.3 I only have:

Starts an RMA access epoch. The window at the process with rank rank can be accessed by RMA operations on win during that epoch. Multiple RMA access epochs (with calls to MPI_WIN_LOCK) can occur simultaneously; however, each access epoch must target a different process.

I guess it should really read:

however, each access epoch opened by a process must target a different process.

Would that make it clearer?

So you mean that a win_lock must not lock its own window? How should one ensure that one's own window can then be updated (if necessary)? Say in the middle of some loop where everybody may access everybody's window. Flush + barriers?

To me this looks like my above code would be faulty since all rank!=0 access the same target without checking for coherence (NOCHECK). But it isn't clear from the specification whether the MPI implementor should ensure this, or I should.

AFAICT, your code is still correct. The description of the NOCHECK assert reads:

no other process holds, or will attempt to acquire, a conflicting lock, while the caller holds the window lock. This is useful when mutual exclusion is achieved by other means, but the coherence operations that may be attached to the lock and unlock calls are still required.

The shared locks are not conflicting. This should also answer your question on coherence:

So now I get confused about the re-phrased paragraph (top entry). There it seems to me that the implementors are in charge of obeying access concurrency, thereby making NOCHECK doing nothing for LOCK_SHARED?
Would this then be a problem:

if (rank != 0) {
    int target = 0;
    MPI_Win_lock(MPI_LOCK_EXCLUSIVE,target,MPI_MODE_NOCHECK,win);
    MPI_Accumulate(buffer, BUF_SIZE, MPI_INT, target, 0, BUF_SIZE, MPI_INT, MPI_SUM, win);
    MPI_Win_unlock(target,win);
  }

since everybody will try to aquire the lock?

devreal commented 1 year ago

This is much better. This, to me, reads like it is the MPI implementors responsibility to ensure the locks access concurrency.

So you mean that a win_lock must not lock its own window? How should one ensure that one's own window can then be updated (if necessary)? Say in the middle of some loop where everybody may access everybody's window. Flush + barriers?

No, this is not what the modified text would say:

however, each access epoch [opened by a process] must target a different process.

In other words:

however, at each process, each access epoch must target a different process.

The shared locks are not conflicting. This should also answer your question on coherence: So now I get confused about the re-phrased paragraph (top entry). There it seems to me that the implementors are in charge of obeying access concurrency, thereby making NOCHECK doing nothing for LOCK_SHARED?

I'm not sure what you mean by obeying access concurrency (or ensure the locks access concurrency in your first comment): if provided, the NOCHECK assert tells the implementation that there will not be any conflicting locks, i.e., in the case of a shared lock that there will be no exclusive locks on the same window at the same process taken by another process, or in the case of an exclusive lock that there will be no exclusive lock or shared lock taken on the same window at the same process taken by another process (all of those are would be conflicting). It can thus drop the synchronization guarantees. However, the implementation must still ensure coherence, i.e., visibility of updates after the lock has been released.

For example, two processes communicate through a window with locks using NOCHECK and synchronize using P2P communication. The implementation has to make sure that the data put into the window by one process is visible to the other process once the lock has been released.

Your new example is indeed erroneous, since these exclusive locks are conflicting. Whether or not the implementation will barf at it is undefined. A possible outcome is that there simply won't be any synchronization between them through the lock, i.e., it will behave as if every process was taking a shared lock. If your application relies on any synchronization for correctness this could result in a wrong result.

zerothi commented 1 year ago

This is much better. This, to me, reads like it is the MPI implementors responsibility to ensure the locks access concurrency.

So you mean that a win_lock must not lock its own window? How should one ensure that one's own window can then be updated (if necessary)? Say in the middle of some loop where everybody may access everybody's window. Flush + barriers?

No, this is not what the modified text would say:

however, each access epoch [opened by a process] must target a different process.

In other words:

however, at each process, each access epoch must target a different process.

I would then rephrase to:

however, multiple access epochs at different processors must have different (unique) targets.

The shared locks are not conflicting. This should also answer your question on coherence: So now I get confused about the re-phrased paragraph (top entry). There it seems to me that the implementors are in charge of obeying access concurrency, thereby making NOCHECK doing nothing for LOCK_SHARED?

I'm not sure what you mean by obeying access concurrency (or ensure the locks access concurrency in your first comment): if provided, the NOCHECK assert tells the implementation that there will not be any conflicting locks, i.e., in the case of a shared lock that there will be no exclusive locks on the same window at the same process taken by another process, or in the case of an exclusive lock that there will be no exclusive lock or shared lock taken on the same window at the same process taken by another process (all of those are would be conflicting). It can thus drop the synchronization guarantees. However, the implementation must still ensure coherence, i.e., visibility of updates after the lock has been released.

For example, two processes communicate through a window with locks using NOCHECK and synchronize using P2P communication. The implementation has to make sure that the data put into the window by one process is visible to the other process once the lock has been released.

Your new example is indeed erroneous, since these exclusive locks are conflicting. Whether or not the implementation will barf at it is undefined. A possible outcome is that there simply won't be any synchronization between them through the lock, i.e., it will behave as if every process was taking a shared lock. If your application relies on any synchronization for correctness this could result in a wrong result.

Thanks, I think I now understand it. I think that your example about exclusive locks combined with shared locks would be ideal to list in the report. It would clarify the meaning, IMHO.