Gleb> Hello I've sent this mail a week ago and had no response. I
Gleb> hope this is not because of lack of interest in user space
Gleb> support :) Anyway I repost it one more time to get the
Gleb> feedback and to find the way to include this patch to openib
Gleb> ASAP and not wait till madvise defines will propogate to
Gleb> libc.
Sorry, I missed it the first time around.
In general this seems good, but I have a few quick comments:
> +#ifndef MADV_DONTFORK
> +#define MADV_DONTFORK 10
> +#endif
> +#ifndef MADV_DOFORK
> +#define MADV_DOFORK 11
> +#endif
This should probably be in the only file that uses it, memory.c. And
I think it's cleanest to use autoconf to check if MADV_DONTFORK and
MADV_DOFORK are available.
> --- libibverbs/include/infiniband/verbs.h (revision 7112)
> +++ libibverbs/include/infiniband/verbs.h (working copy)
> @@ -289,6 +289,8 @@
> uint32_t handle;
> uint32_t lkey;
> uint32_t rkey;
> + void *addr;
> + size_t length;
> };
This breaks ABI, right?
> --- libmthca/src/verbs.c (revision 7112)
> +++ libmthca/src/verbs.c (working copy)
> @@ -134,6 +134,9 @@
> return NULL;
> }
>
> + mr->addr = addr;
> + mr->length = length;
> +
> return mr;
> }
What's the reason to set addr and length here? Doesn't libibverbs
already do it?
> if (!cq->buf)
> goto err;
>
> + madvise(cq->buf, cqe * MTHCA_CQ_ENTRY_SIZE, MADV_DONTFORK);
> cq->mr = __mthca_reg_mr(to_mctx(context)->pd, cq->buf,
> cqe * MTHCA_CQ_ENTRY_SIZE,
> 0, IBV_ACCESS_LOCAL_WRITE);
> @@ -247,6 +251,7 @@
> mthca_dereg_mr(cq->mr);
>
> err_buf:
> + madvise(cq->buf, cqe * MTHCA_CQ_ENTRY_SIZE, MADV_DOFORK);
> free(cq->buf);
It seems it would be better to put the DONTFORK call into
mthca_alloc_cq_buf(), and the DOFORK into a new mthca_free_cq_buf() call.
Actually, to handle the QP and SRQ cases too it's probably better to
have wrappers for posix_memalign() and free() to keep this
encapsulated in one place.
- R.
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
