On 2011/03/25, at 01:05, Steven Dake wrote:
> Great design
>
> Curious why not just have the free list as a static global in totembuf.c
> (ie only have one list)?
The udp and udpu drivers certainly can and probably should share the same free
list. However I was hoping to use the same implementation for the iba, and the
thought was that the buffers are a different size so it should have its own
free list. Alternatively, we could just use 10k buffers for everything (this
feels a bit wrong to me). Or store a list of free lists internally and look up
the one with the correct buffer size when we go to allocate.
However, it sounds like I am going to have to dig a lot deeper into the
Infiniband stuff before I will even know if this implementation will work for
the iba driver ;)
> new C and H files need an appropriate license at the header. Just cut
> and paste from an existing header file (changing the copyright date).
>
> More comments are inline:
>
> On 03/24/2011 06:57 PM, Zane Bitter wrote:
>> (Work in Progress)
>>
>> TODO:
>> * Implement the same change in the udpu driver
>> * Add locks for thread safety
>> * Copyright boilerplate &c.
>> ---
>> exec/Makefile.am | 2 +
>> exec/totembuf.c | 81
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> exec/totembuf.h | 13 +++++++++
>> exec/totemudp.c | 11 ++++++-
>> 4 files changed, 104 insertions(+), 3 deletions(-)
>> create mode 100644 exec/totembuf.c
>> create mode 100644 exec/totembuf.h
>>
>> diff --git a/exec/Makefile.am b/exec/Makefile.am
>> index 39a7213..8ee1e07 100644
>> --- a/exec/Makefile.am
>> +++ b/exec/Makefile.am
>> @@ -37,7 +37,7 @@ INCLUDES = -I$(top_builddir)/include
>> -I$(top_srcdir)/include $(nss_CFLAGS) $(rd
>>
>> TOTEM_SRC = coropoll.c totemip.c totemnet.c totemudp.c \
>> totemudpu.c totemrrp.c totemsrp.c totemmrp.c \
>> - totempg.c crypto.c wthread.c tsafe.c
>> + totempg.c totembuf.c crypto.c wthread.c tsafe.c
>> if BUILD_RDMA
>> TOTEM_SRC += totemiba.c
>> endif
>> diff --git a/exec/totembuf.c b/exec/totembuf.c
>> new file mode 100644
>> index 0000000..7fce433
>> --- /dev/null
>> +++ b/exec/totembuf.c
>> @@ -0,0 +1,81 @@
>> +
>> +#include <config.h>
>> +#include <corosync/list.h>
>> +
>> +#include <stddef.h>
>> +#include <stdlib.h>
>> +#include <assert.h>
>> +
>> +#include "totembuf.h"
>> +
>> +
>> +#define TOTEMBUF_FROM_BUFFER(BUF) ((struct totembuf *)((char *)(BUF) -
>> offsetof(struct totembuf, buffer)))
>> +
>> +
>> +struct totembuf_list {
>> + struct list_head list_free;
>> + size_t size;
>> +};
>> +
>> +struct totembuf {
>> + struct list_head list_free;
>> + struct totembuf_list *owner;
>> + int ref_count;
>> + char buffer[0];
>> +};
>> +
>> +
>
> This could probably just be a file scoped global (and as a result not
> return totembuf_list struct.
>
>> +struct totembuf_list *totembuf_list_init (size_t buffer_size)
>> +{
>> + struct totembuf_list *free_list = malloc (sizeof (struct
>> totembuf_list));
>> + if (!free_list) {
>> + return NULL;
>> + }
>> +
>> + free_list->size = buffer_size;
>> + list_init (&free_list->list_free);
>> +
>> + return free_list;
>> +}
>> +
>> +void *totembuf_alloc (struct totembuf_list *free_list)
>> +{
>> + struct totembuf *block;
>> +
>> + if (list_empty (&free_list->list_free)) {
>> + block = malloc (sizeof (struct totembuf) + free_list->size);
>> + if (block == NULL) {
>> + return (NULL);
>> + }
>> + } else {
>> + block = list_entry (free_list->list_free.next, struct totembuf,
>> list_free);
>> + list_del (&block->list_free);
>> + assert (block->ref_count == 0);
>> + }
>> +
>> + block->ref_count = 1;
>> + block->owner = free_list;
>> +
>> + return (block->buffer);
>> +}
>> +
>> +void *totembuf_retain (void *ptr)
>> +{
>> + struct totembuf *block = TOTEMBUF_FROM_BUFFER(ptr);
>> + block->ref_count++;
>> + return (block->buffer);
>> +}
>> +
>> +void totembuf_release (void *ptr)
>> +{
>> + struct totembuf *block = TOTEMBUF_FROM_BUFFER(ptr);
>> +
>> + block->ref_count--;
>> + if (block->ref_count == 0) {
>> + struct totembuf_list *free_list = block->owner;
>> + assert (free_list != NULL);
>> + block->owner = NULL;
>> + list_add_tail (&block->list_free, &free_list->list_free);
>> + }
>> +}
>> +
>> diff --git a/exec/totembuf.h b/exec/totembuf.h
>> new file mode 100644
>> index 0000000..8226329
>> --- /dev/null
>> +++ b/exec/totembuf.h
>> @@ -0,0 +1,13 @@
>> +
>> +#ifndef TOTEMBUF_H_DEFINED
>> +#define TOTEMBUF_H_DEFINED
>> +
>> +struct totembuf_list;
>> +
>> +struct totembuf_list *totembuf_list_init (size_t buffer_size);
>> +void *totembuf_alloc (struct totembuf_list *free_list);
>
> Not totally convinced separate lists are needed.
>
>> +void *totembuf_retain (void *ptr);
>> +void totembuf_release (void *ptr);
>> +
>> +#endif
>> +
>> diff --git a/exec/totemudp.c b/exec/totemudp.c
>> index 804d00b..dc74d37 100644
>> --- a/exec/totemudp.c
>> +++ b/exec/totemudp.c
>> @@ -65,6 +65,7 @@
>> #include <corosync/totem/coropoll.h>
>> #define LOGSYS_UTILS_ONLY 1
>> #include <corosync/engine/logsys.h>
>> +#include "totembuf.h"
>> #include "totemudp.h"
>> #include "wthread.h"
>>
>> @@ -223,6 +224,8 @@ static int totemudp_build_sockets (
>>
>> static struct totem_ip_address localhost;
>>
>> +static struct totembuf_list *free_list = NULL;
>> +
>> static void totemudp_instance_initialize (struct totemudp_instance *instance)
>> {
>> memset (instance, 0, sizeof (struct totemudp_instance));
>> @@ -1747,6 +1750,10 @@ int totemudp_initialize (
>> {
>> struct totemudp_instance *instance;
>>
>> + if (!free_list) {
>> + free_list = totembuf_list_init (FRAME_SIZE_MAX);
>> + }
>> +
>> instance = malloc (sizeof (struct totemudp_instance));
>> if (instance == NULL) {
>> return (-1);
>> @@ -1826,12 +1833,12 @@ int totemudp_initialize (
>>
>> void *totemudp_buffer_alloc (void)
>> {
>> - return malloc (FRAME_SIZE_MAX);
>> + return totembuf_alloc (free_list);
>> }
>>
>> void totemudp_buffer_release (void *ptr)
>> {
>> - return free (ptr);
>> + totembuf_release (ptr);
>> }
>>
>> int totemudp_processor_count_set (
>>
>> _______________________________________________
>> Openais mailing list
>> [email protected]
>> https://lists.linux-foundation.org/mailman/listinfo/openais
>
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
