<div dir="ltr"><p dir="ltr">Hi, Sebastian, thanks for your review!</p><p dir="ltr">
在 2013-7-7 下午9:49，"Sebastian Huber" <<a href="mailto:sebastian.huber@embedded-brains.de" target="_blank">sebastian.huber@embedded-brains.de</a>>写道：<br>
><br>
> Hello Ashi,<br>
><br>
><br>
> On 06/07/13 09:17, Ashi wrote:<br>
>><br>
>> Hi all,<br>
>><br>
>> this patch adds a data structure called freelist to score, there are no<br>
>> test cases yet and should be added later.<br>
><br>
><br>
> I would appreciate to have the test for this new stuff included in the patch.</p><p dir="ltr"><br>
sure, I will update the patch with test cases.<br>
><br>
><br>
>><br>
>> Freelist is a structure, which contains a chain of nodes. It supports 2<br>
>> operation:<br>
>> - get node from freelist<br>
>> - put node to freelist.<br>
>> And when there is no enough node in freelist, it will automatically<br>
>> increase itself's size by allocate more nodes from heap or workspace(which<br>
>> is specified by user).<br>
><br>
><br>
> What can I do if I like to get the nodes from a magic space?</p><p dir="ltr"><br>
sorry for the unclear, you can get nodes from freelist by 'get'
operation. And if you mean get nodes from heap or workspace, it's done
by _Freelist_Get_node(), which calls _Freelist_Bump() when there is no
free node left.<br>
><br>
>> And the size of new allocated nodes is specified by<br>
>> user when structure initialization. When initializing, freelist would<br>
>> pre-allocate a bunch of nodes, which size is also can be specified by user.<br>
>><br>
>> By the way, the part of motivation of adding this data structure is to make<br>
>> RTEMS POSIX Key(this patch will be posted later) unlimited mode more<br>
>> efficient.<br>
>><br>
>> Thanks,<br>
>> Zhongwei<br>
>><br>
>><br>
>> add_freelist.patch<br>
>><br>
>><br>
>> diff --git a/cpukit/score/Makefile.am b/cpukit/score/Makefile.am<br>
>> index 3f6e686..092d983 100644<br>
>> --- a/cpukit/score/Makefile.am<br>
>> +++ b/cpukit/score/Makefile.am<br>
>> @@ -30,6 +30,7 @@ include_rtems_score_HEADERS += include/rtems/score/protectedheap.h<br>
>> include_rtems_score_HEADERS += include/rtems/score/interr.h<br>
>> include_rtems_score_HEADERS += include/rtems/score/isr.h<br>
>> include_rtems_score_HEADERS += include/rtems/score/isrlevel.h<br>
>> +include_rtems_score_HEADERS += include/rtems/score/freelist.h<br>
>> include_rtems_score_HEADERS += include/rtems/score/object.h<br>
>> include_rtems_score_HEADERS += include/rtems/score/percpu.h<br>
>> include_rtems_score_HEADERS += include/rtems/score/priority.h<br>
>> @@ -265,6 +266,9 @@ libscore_a_SOURCES += src/pheapallocate.c \<br>
>> src/pheapgetblocksize.c src/pheapgetfreeinfo.c src/pheapgetinfo.c \<br>
>> src/pheapinit.c src/pheapresizeblock.c src/pheapwalk.c src/pheapiterate.c<br>
>> +## FREELIST_C_FILES<br>
>> +libscore_a_SOURCES += src/freelist.c<br>
>> +<br>
>> ## RBTREE_C_FILES<br>
>> libscore_a_SOURCES += src/rbtree.c \<br>
>> src/rbtreeextract.c src/rbtreefind.c src/rbtreefindheader.c \<br>
>> diff --git a/cpukit/score/include/rtems/score/freelist.h b/cpukit/score/include/rtems/score/freelist.h<br>
>> new file mode 100644<br>
>> index 0000000..c21a872<br>
>> --- /dev/null<br>
>> +++ b/cpukit/score/include/rtems/score/freelist.h<br>
>> @@ -0,0 +1,138 @@<br>
>> +/**<br>
>> + * @file<br>
>> + *<br>
>> + * @ingroup ScoreFreelist<br>
>> + *<br>
>> + * @brief Freelist Handler API<br>
>> + */<br>
>> +/*<br>
>> + * Copyright (c) 2013 Gedare Bloom.<br>
>> + *<br>
>> + * The license and distribution terms for this file may be<br>
>> + * found in the file LICENSE in this distribution or at<br>
>> + *<a href="http://www.rtems.com/license/LICENSE" target="_blank">http://www.rtems.com/license/LICENSE</a>.<br>
>> + */<br>
>> +<br>
>> +#ifndef _FREELIST_H<br>
>> +#define _FREELIST_H<br>
>> +<br>
>> +#include <rtems/score/chain.h><br>
>> +#include <rtems/rtems/types.h><br>
>> +#include <rtems/score/wkspace.h><br>
>> +<br>
>> +/**<br>
>> + * @defgroup ScoreFreelist Freelist Handler API<br>
>> + *<br>
>> + * @ingroup Score<br>
>> + *<br>
>> + * The Freelist Handler is used to manage a chain of nodes, of which size can<br>
>> + * automatically increase when there is no free node left. This handler provides one data structure:<br>
>> + * Freelist_Control.<br>
>> + */<br>
>> +/**@{*/<br>
>> +<br>
>> +#ifdef __cplusplus<br>
>> +extern "C" {<br>
>> +#endif<br>
>> +<br>
>> +/**<br>
>> + * @typedef Freelist_Control<br>
>> + */<br>
>> +typedef struct Freelist_Control_struct Freelist_Control;<br>
><br>
><br>
> You can also use<br>
><br>
> typedef struct Freelist_Control Freelist_Control;</p><p dir="ltr">OK<br>
><br>
>> +<br>
>> +/**<br>
>> + * @typedef freelist_callout<br>
>> + */<br>
>> +typedef void (*freelist_callout)(<br>
>> + Freelist_Control *fc,<br>
>> + void *nodes<br>
>> +);<br>
>> +<br>
>> +/**<br>
>> + * @typedef Freelist_Control_struct<br>
>> + *<br>
>> + * This is used to manage each element.<br>
>> + */<br>
>> +struct Freelist_Control_struct {<br>
>> + Chain_Control Freelist;<br>
>> + size_t free_count;<br>
><br>
><br>
> Why do we need the free_count?</p><p dir="ltr">free_count is used
to keep track how many nodes there is in freelist. And if free_count is 0
when you try to get node from freelist by call _Freelist_Get_node(),
_Freelist_Get_node() will call _Freelist_Bump() to allocate more node
from heap or workspace.<br>
><br>
>> + size_t bump_count;<br>
>> + size_t node_size;<br>
><br>
><br>
>> + freelist_callout callout;<br>
>> + bool use_workspace;<br>
>> +};<br>
><br>
><br>
> I would replace this with an extend handler.<br>
><br>
> /**<br>
> * @brief Extends the freelist.<br>
> *<br>
> * @param[in] freelist The freelist control.<br>
> *<br>
> * @return The count of new nodes.<br>
> */<br>
> typedef size_t ( *Freelist_Extend )( Freelist_Control *freelist );<br>
><br>
> This is much more flexible since you only specify the interface and don't limit this to heap/workspace.<br>
><br>
> You can provide a _Freelist_Extend_with_nothing() which simply returns 0.</p><p dir="ltr">Yeah, this Freelist_Extend is very flexible, but I feel the Freelist_Extend is a little complex. As it shows in _Freelist_Bump(), if users provides their own extension function, they have to append there new nodes to freelist's internal chain and call their callout function on new nodes. And since _Freelist_Initialize() also would call Freelist_Extend(), if we provided _Freelist_Extend_with_nothing(), the initialization may fail.</p>
<p> Anyway, I'm not sure which kind approach RTEMS prefer.<br>
</p><p dir="ltr">
><br>
>> +<br>
>> +/**<br>
>> + * @brief Initialize a freelist<br>
>> + *<br>
>> + * This routine initializes @a the Freelist_Control structure to manage a<br>
>> + * chain of nodes, each node's size is @a node_size and the size of chain is<br>
>> + * initialized to @a bump_count and it also will increase with size of<br>
>> + * @a bump_count. @a callout is called on all the nodes after allocated from<br>
>> + * workspace.<br>
>> + *<br>
>> + * @param[in] fc is the freelist too initialize.<br>
>> + * @param[in] node_size is size of the elemment in this freelist.<br>
>> + * @param[in] bump_count is the size of chain increased when no free node left.<br>
>> + * @param[in] callout is the function called on all nodes in freelist_bump,<br>
>> + * if it's null, a default function is set.<br>
>> + * @param[in] use_workspace is used to determine whether heap or workspace is<br>
>> + * in for Freelist node.<br>
>> + */<br>
>> +void _Freelist_Initialize(<br>
>> + Freelist_Control *fc,<br>
><br>
><br>
> I would use "freelist" instead of "fc". Maybe we should use "chain" instead of "list.</p><p dir="ltr">Yeah, it's actually a chain of nodes. I don't know whether Gedare have other concerns about the name.<br>
</p><p dir="ltr">
><br>
> [...]<br>
><br>
> -- <br>
> Sebastian Huber, embedded brains GmbH<br>
><br>
> Address : Dornierstr. 4, D-82178 Puchheim, Germany<br>
> Phone : +49 89 189 47 41-16<br>
> Fax : +49 89 189 47 41-09<br>
> E-Mail : <a href="mailto:sebastian.huber@embedded-brains.de" target="_blank">sebastian.huber@embedded-brains.de</a><br>
> PGP : Public key available on request.<br>
><br>
> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.<br>
><br>
</p>
</div>