[PATCH] Add cleanup routines.
Dynamically loaded library handles are saved in a list and dlclosed() on
exit. The list of struct ibv_driver *, as well as the global
struct ibv_device ** list are free()d.
Signed-off-by: Hannes Weisbach hannes_weisb...@gmx.net
---
src/device.c | 10 ++
src/init.c | 51 ++-
2 files changed, 56 insertions(+), 5 deletions(-)
diff --git a/src/device.c b/src/device.c
index beb7b3c..d5b76bb 100644
--- a/src/device.c
+++ b/src/device.c
@@ -305,3 +305,13 @@ void __ibv_ack_async_event(struct ibv_async_event *event)
}
}
default_symver(__ibv_ack_async_event, ibv_ack_async_event);
+
+FINI static void ibverbs_deinit()
+{
+ size_t i;
+ for (i = 0; i num_devices; i++) {
+ /* driver callback needed. May not be malloc'd memory */
+ free(device_list[i]);
+ }
+ free(device_list);
+}
diff --git a/src/init.c b/src/init.c
index d0e4b1c..2a8bca4 100644
--- a/src/init.c
+++ b/src/init.c
@@ -67,6 +67,11 @@ struct ibv_driver_name {
struct ibv_driver_name *next;
};
+struct ibv_so_list {
+ void *dlhandle;
+ struct ibv_so_list *next;
+};
+
struct ibv_driver {
const char *name;
ibv_driver_init_funcinit_func;
@@ -77,6 +82,7 @@ struct ibv_driver {
static struct ibv_sysfs_dev *sysfs_dev_list;
static struct ibv_driver_name *driver_name_list;
static struct ibv_driver *head_driver, *tail_driver;
+static struct ibv_so_list *so_list;
static int find_sysfs_devs(void)
{
@@ -193,7 +199,14 @@ void verbs_register_driver(const char *name,
verbs_driver_init_func init_func)
static void load_driver(const char *name)
{
char *so_name;
- void *dlhandle;
+ struct ibv_so_list *elem;
+ struct ibv_so_list **list;
+
+ elem = malloc(sizeof(*elem));
+ if(!elem)
+ return;
+
+ elem-next = NULL;
#define __IBV_QUOTE(x) #x
#define IBV_QUOTE(x) __IBV_QUOTE(x)
@@ -205,16 +218,25 @@ static void load_driver(const char *name)
name) 0) {
fprintf(stderr, PFX Warning: couldn't load driver '%s'.\n,
name);
- return;
+ goto err;
}
- dlhandle = dlopen(so_name, RTLD_NOW);
- if (!dlhandle) {
+ elem-dlhandle = dlopen(so_name, RTLD_NOW);
+ if (!elem-dlhandle) {
fprintf(stderr, PFX Warning: couldn't load driver '%s': %s\n,
name, dlerror());
- goto out;
+ goto err;
}
+ for (list = so_list; *list; list = (*list)-next)
+ ;
+
+ *list = elem;
+
+ goto out;
+
+err:
+ free(elem);
out:
free(so_name);
}
@@ -573,3 +595,22 @@ out:
return num_devices;
}
+
+FINI static void unload_drivers()
+{
+ struct ibv_driver *driver;
+ struct ibv_so_list * list;
+
+ for (driver = head_driver; driver;) {
+ struct ibv_driver *tmp = driver;
+ driver = driver-next;
+ free(tmp);
+ }
+
+ for (list = so_list; list;) {
+ struct ibv_so_list *tmp = list;
+ list = list-next;
+ dlclose(tmp-dlhandle);
+ free(tmp);
+ }
+}
--
1.8.5.5
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Clean index map on exit
This patch adds the function idm_free() to free all entries of an index
map. A call to this function is added in the ucma_cleanup destructor.
The ucma_idm struct index_map is cleaned.
Signed-off-by: Hannes Weisbach hannes_weisb...@gmx.net
---
src/cma.c | 1 +
src/indexer.c | 8
src/indexer.h | 1 +
3 files changed, 10 insertions(+)
diff --git a/src/cma.c b/src/cma.c
index 0cf4203..a533bf9 100644
--- a/src/cma.c
+++ b/src/cma.c
@@ -139,6 +139,7 @@ static void ucma_cleanup(void)
ibv_close_device(cma_dev_array[cma_dev_cnt].verbs);
}
+ idm_free(ucma_idm);
fastlock_destroy(idm_lock);
free(cma_dev_array);
cma_dev_cnt = 0;
diff --git a/src/indexer.c b/src/indexer.c
index c8e8bce..4d1fd77 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -164,3 +164,11 @@ void *idm_clear(struct index_map *idm, int index)
entry[idx_entry_index(index)] = NULL;
return item;
}
+
+void idm_free(struct index_map *idm)
+{
+ size_t i;
+ for (i = 0; i IDX_ARRAY_SIZE; i++) {
+ free(idm-array[i]);
+ }
+}
diff --git a/src/indexer.h b/src/indexer.h
index 0c5f388..829d46b 100644
--- a/src/indexer.h
+++ b/src/indexer.h
@@ -89,6 +89,7 @@ struct index_map
int idm_set(struct index_map *idm, int index, void *item);
void *idm_clear(struct index_map *idm, int index);
+void idm_free(struct index_map *idm);
static inline void *idm_at(struct index_map *idm, int index)
{
--
1.8.5.5
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add cleanup routines.
Am 11.04.2014 um 19:00 schrieb Yann Droneaud ydrone...@opteya.com:
Hi,
Thanks for your quick reply.
Please adds some explanation, in particular the purpose of the changes.
Your commit message only explain how, but you should also explain
why.
Ok.
In C, empty prototype declare a function which accept any parameter. So
perhaps void ibverbs(void) is what you mean.
Yup, thanks.
+{
+size_t i;
+for (i = 0; i num_devices; i++) {
+/* driver callback needed. May not be malloc'd memory */
Seems dangerous. Such interrogation must be explicitly added in the
commit message.
Maybe it's better to leave it as a TODO and only free the device_list
itself?
+for (driver = head_driver; driver;) {
+struct ibv_driver *tmp = driver;
+driver = driver-next;
+free(tmp);
+}
+
Is it safe here to free the driver ?
The struct itself is no longer needed. All the driver libraries are
loaded and ibverbs_init() is call-once, so no new libraries are
dlopen()ed (and call ibv_register_driver()).
It's probably nicer first to dlclose() all libraries and then free all
struct ibv_drivers.
+for (list = so_list; list;) {
+struct ibv_so_list *tmp = list;
+list = list-next;
+dlclose(tmp-dlhandle);
+free(tmp);
+}
+}
Why not store the dlopen() handle in the struct ibv_driver itself ?
This way only one list has to be scanned.
That was my first idea, but the struct ibv_driver doesn't exist at
that time. First the (driver) library is dlopen()d and calls
ibv_register_driver(). Only there is the ibv_driver struct allocated,
but the handle is gone.
Best regards,
Hannes
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Clean index map on exit
Is the array always full ?
No, but it has to be zero-initialized and free(NULL) is fine.
idm_set() does:
if (!idm-array[idx_array_index(index)]) {
if (idm_grow(idm, index) 0)
(idm_grow does calloc().)
So I concluded unused entries have to be zero already and I don't put
a new constraint on struct index_map.
is the maximum index of the array
IDX_ARRAY_SIZE or could it be lower ?
It's fixed:
struct index_map
{
void **array[IDX_ARRAY_SIZE];
};
And what about idm, is it free()'d somewhere else ?
The owner has to take care of its struct index_map. I can only free
the resources the map itself holds.
For what it's worth: all struct index_maps are declared statically.
Again, thank you for your quick feedback.
Best regards,
Hannes
--
To unsubscribe from this list: send the line unsubscribe linux-rdma in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RDMA read does not update local memory
Hello, I found out that some code does madvise(MADV_DONTNEED) on the allocated memory. I think that this unmaps the memory from the user space process. If the process touches the memory again, a COW mapping or even a new, zeroed out page is mapped in the process. Either way, the relation between the physical page, where the data from the RDMA read ends up and the virtual page is broken. I realize that calling madvise(MADV_DONTNEED) doesn’t have any meaning for RDMA-registered memory. I feel however that my observed behavior is a bug and one of the following should happen: . madvise() on pinned memory/I/O memory should fail, i.e. return EINVAL, EIO or ENOMEM (I didn’t test what happens when you call madvise(MADV_DONTNEED) on I/O mem.) . RDMA actions on madvise()’d memory should fail. Best regards, Hannes Weisbach-- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RDMA read does not update local memory
Am 30.01.2014 um 22:36 schrieb Anuj Kalia anujkaliai...@gmail.com: Hannes, Have you tried marking the memory that is being read as volatile“? Thanks for the suggestion, but I don’t think that this solves the problem, because when I do: memset(…); rdma_post_read(…); /* wait */ memcmp(…); The compiler may not optimize across rdma_post_read() (or rather ibv_post_send() and finally ioctl()), because these function may have side-effects (which they do). Anyway, I implemented a little helper memcmpv(volatile unsigned char * s1, volatile unsiged char * s2, size_t size) to compare the memory as volatile, but the memory still has the preset 0x55 instead of the contents of the remote memory. May that memset() be still in the cache, so that the memory accesses of the CPU hit the cache instead of main memory? I can’t imagine the cache isn’t invalidated on reception of remote data. Best regards, Hannes-- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RDMA read does not update local memory
Am 31.01.2014 um 15:26 schrieb Anuj Kalia anujkaliai...@gmail.com: Are you sure that the compiler knows that post_send() has side effects? No, thats why I tested with the mentioned memcmpv. I examined the emitted code with objdump and the compiler issued the mov instructions to read the bytes from memory - still didn’t work. I’ll leave the memcpyv in for the moment just to be sure. Best regards, Hannes-- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RDMA read does not update local memory
Hello all, I’m developing an (userspace) RDMA-application under Linux and came across a problem, I’m not able to solve. On machine A I had a chunk of memory registered with rdma_reg_read(), so others can read from that memory later. Machine B has memory which is also registered with rdma_reg_read(). Machine B tells machine A about that memory and the rkey. Machine A rdma_post_read()s then from that memory on machine B in it’s own memory. The problem now is, that - although no error is reported - the contents of machine A’s memory does not change. I memset() it to 0x55 before the rdma_post_read() for debugging purposes and instead of having the contents I expect (an ASCII string), the memory is still 0x55 after the work completion is signaled. Thus, I’ve allocated different chunks of memory using malloc() and posix_memalign() with different sizes and registered them with rdma_reg_read(), to see if that makes a difference. The rdma_post_read() works on all tested memory, except one chunk. I don’t know what is different about that memory, where my mistake might be or how to debug this. Please note, that I check all return values for errors (there are none) and also the work completion reports IBV_SUCCESS. I hope you can give me some hints on how to proceed. I apologize if my problem description was too brief; please ask for details if I left important information out. Best regards, Hannes Weisbach-- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
non-const pointer void * addr in rdma_reg_* and rdma_post_[send|write]
Hi, I just started working with librdmacm and I was wondering if there is a specific reason why rdma_reg_* functions and rdma_post_send/write functions take the local memory address as non-const pointer void * addr. These functions shouldn't and don't change the memory pointed to by addr. I think this should be made explicit by using the type const void * for addr. In case you agree, I would volunteer to make the necessary changes. Best regards, Hannes Weisbach-- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
