Great design
Curious why not just have the free list as a static global in totembuf.c
(ie only have one list)?
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
