shmem_transport_get expects registered target buffer while shmem_get does not?

[zhongchen530]

Not sure if this is the right place to ask. But I was very curious about the implementation details of this repository, so I decided to give it a try anyway. As I was reading through the repo, I saw that the definitions of all shmem_get invoke shmem_internal_get which in turn invoke shmem_transport_get. It is clear that shmem_transport_get assumes the target buffer to be registered memory because we see GET_MR_DESC(shmem_transport_ofi_get_mr_desc_index(target)). It is also clear from the specification that the target buffer of shmem_get should be arbitrary. Isn't the symmetric heap and static data segment the only regions that are registered？I can't see how this would work unless remote-virtual-addressing and scalable_mr are both enabled.

Any answer would be greatly appreciated. Thanks in advanced!

In data_c.c4,

#define SHMEM_DEF_GET_N(NAME,SIZE)                         \
  void SHMEM_FUNCTION_ATTRIBUTES                           \
  SHMEM_FUNC_PROTOTYPE(get##NAME, void *target,            \
                       const void *source, size_t nelems,  \
                       int pe)                             \
    SHMEM_ERR_CHECK_INITIALIZED();                         \
    SHMEM_ERR_CHECK_PE(pe);                                \
    SHMEM_ERR_CHECK_CTX(ctx);                              \
    SHMEM_ERR_CHECK_SYMMETRIC(source, (SIZE) * nelems);    \
    SHMEM_ERR_CHECK_NULL(target, nelems);                  \
    SHMEM_ERR_CHECK_OVERLAP(target, source, (SIZE)*nelems, \
                           (SIZE) * nelems, 0,             \
                           (shmem_internal_my_pe == pe));  \
    shmem_internal_get(ctx, target, source, (SIZE)*nelems, \
                       pe);                                \
    shmem_internal_get_wait(ctx);                          \
  }

In shmem_comm.h,

static inline
void
shmem_internal_get(shmem_ctx_t ctx, void *target, const void *source, size_t len, int pe)
{
    if (len == 0) return;

    if (shmem_shr_transport_use_read(ctx, target, source, len, pe)) {
        shmem_shr_transport_get(ctx, target, source, len, pe);
    } else {
        shmem_transport_get((shmem_transport_ctx_t *)ctx, target, source, len, pe);
    }
}

Finally in transport_ofi.h,

static inline
void shmem_transport_get(shmem_transport_ctx_t* ctx, void *target, const void *source, size_t len, int pe)
{
    int ret = 0;
    uint64_t dst = (uint64_t) pe;
    uint64_t polled = 0;
    uint64_t key;
    uint8_t *addr;

    shmem_transport_ofi_get_mr(source, pe, &addr, &key);

    SHMEM_TRANSPORT_OFI_CTX_LOCK(ctx);
    if (len <= shmem_transport_ofi_max_msg_size) {

        SHMEM_TRANSPORT_OFI_CNTR_INC(&ctx->pending_get_cntr);
        do {
            ret = fi_read(ctx->ep,
                          target,
                          len,
                          GET_MR_DESC(shmem_transport_ofi_get_mr_desc_index(target)),
                          GET_DEST(dst),
                          (uint64_t) addr,
                          key,
                          NULL);
        } while (try_again(ctx, ret, &polled));
    }
    else {
        uint8_t *frag_target = (uint8_t *) target;
        uint64_t frag_source = (uint64_t) addr;
        size_t frag_len = len;

        while (frag_target < ((uint8_t *) target) + len) {
            frag_len = MIN(shmem_transport_ofi_max_msg_size,
                           (size_t) (((uint8_t *) target) + len - frag_target));
            polled = 0;

            SHMEM_TRANSPORT_OFI_CNTR_INC(&ctx->pending_get_cntr);

            do {
                ret = fi_read(ctx->ep,
                              frag_target, frag_len,
                              GET_MR_DESC(shmem_transport_ofi_get_mr_desc_index(target)),
                              GET_DEST(dst), frag_source,
                              key, NULL);
            } while (try_again(ctx, ret, &polled));

            frag_source += frag_len;
            frag_target += frag_len;
        }
    }
    SHMEM_TRANSPORT_OFI_CTX_UNLOCK(ctx);
}

[davidozog]

Not sure if this is the right place to ask.

It seems like a great place to me to ask such a question. Thanks for starting the first "discussion" for SOS on Github!

It is clear from the specification that the target buffer of shmem_get should be arbitrary.

For shmem_get and the other "Get" routines, the target buffer in SOS actually corresponds to the dest argument, which can be arbitrary. It's the source buffer which needs to be remotely accessible (i.e., in the symmetric heap or data segment). This is a bit confusing because some might think of the source buffer for "Get" as the "target" of an RMA operation.. but SOS code isn't written that way - the "target" is where the result of the Get ends up. So perhaps the variable naming could be improved?

It is clear that shmem_transport_get assumes the target buffer to be registered memory because we see GET_MR_DESC(shmem_transport_ofi_get_mr_desc_index(target)).

Anyway, the short answer is that local dest ("target") buffer usually does not need to be explicitly registered with libfabric, so GET_MR_DESC usually returns NULL, but SOS sometimes needs to specify a registered memory descriptor... especially with the addition of GPU support, say via Intel SHMEM.

In some more detail, there are recent changes here to better support GPU buffers (specifically, with providers that set FI_MR_LOCAL, if I'm not mistaken). Please note that GET_MR_DESC currently returns NULL unless USE_FI_HMEM is defined. Briefly, my understanding is that NICs typically need all buffers to be registered, even the local dest buffer; but libfabric only requires a-priori/explicit registration of the remote buffer (source in this case). I believe the utility provider registers memory internally on-the-fly if needed (@j-xiong can possibly correct me where I'm wrong). In this "shmem_get" case, without specifying exactly which memory registration dest could be in (if it happens to already be registered in either the heap, data, or external GPU segment), SOS over libfabric has problems... This argument to fi_read was previously just NULL before commit 934f44a, but now we're looking at a recent bug-fix where the code might be a bit confusing in its current state.

By the way, this is where the remote address registration (corresponding to source) is determined:

    shmem_transport_ofi_get_mr(source, pe, &addr, &key);

On a similar note, perhaps we should rename the target variable to dest or something like that to better reflect the spec, which explains that dest is the local buffer that need not be remotely accessible (but might still need to have an MR descriptor in some cases).

Does that help?

[zhongchen530]

Ahh, so the key here is that fi_read in libfabric only requires remote buffer to be registered with fi_mr_reg. I mistakenly thought that local buffer would need to be registered as well as it is the case for lower level RDMA API such as verbs. Thank you so much for answering my question!

[zhongchen530]

I wonder whether the lack of need to register local buffer for fi_read is universal for all providers in libfabric? Is it a universal requirement of libfabric. I can't seem to find any resource that support this. Any resource will be greatly appreciated! Thanks!

[davidozog]

Sure thing!

The requirements around local buffer registration do depend on the provider (and possibly the associated utility provider, like RXM). The mode bit to read about is FI_MR_LOCAL on the fi_mr man page: https://ofiwg.github.io/libfabric/main/man/fi_mr.3.html

In the case of SOS, the interplay of FI_MR_LOCAL with FI_HMEM_MR also comes into play. So it might help to also read about FI_MR_HMEM on the fi_mr manpage above.

You can see which providers require FI_MR_LOCAL (and which support FI_HMEM) in the provider support matrices in the libfabric wiki, for example:
https://github.com/ofiwg/libfabric/wiki/Provider-Feature-Matrix-v1.18.x

[zhongchen530]

Thank you so much!