Re: [PATCH v2 1/4] nanokernel: Add fiber_config structure and wrapper


Luiz Augusto von Dentz
 

Hi Vlad,

On Tue, May 10, 2016 at 1:46 PM, Vlad Dogaru <vlad.dogaru(a)intel.com> wrote:
On Mon, May 09, 2016 at 04:30:16PM +0300, Luiz Augusto von Dentz wrote:
Hi Vlad,

On Mon, May 9, 2016 at 1:28 PM, Vlad Dogaru <vlad.dogaru(a)intel.com> wrote:
On Mon, May 09, 2016 at 01:16:21PM +0300, Luiz Augusto von Dentz wrote:
Hi Vlad,

On Mon, May 9, 2016 at 11:29 AM, Vlad Dogaru <vlad.dogaru(a)intel.com> wrote:
Many drivers need to start fibers whose parameters are based on user
configuration. Make it easier to specify such parameters by creating a
fiber_config structure and a wrapper to use it.

Change-Id: I26917ada71acbc676c0f32b6ee081aff7bb1d6d8
Signed-off-by: Vlad Dogaru <vlad.dogaru(a)intel.com>
---
include/nanokernel.h | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)

diff --git a/include/nanokernel.h b/include/nanokernel.h
index 8d7a864..d79bde6 100644
--- a/include/nanokernel.h
+++ b/include/nanokernel.h
@@ -230,6 +230,45 @@ extern void fiber_yield(void);
*/
extern void fiber_abort(void);

+/**
+ * @brief Fiber configuration structure.
+ *
+ * Parameters such as stack size and fiber priority are often
+ * user configurable. This structure makes it simple to specify such a
+ * configuration.
+ */
+struct fiber_config {
+ char *stack;
+ unsigned stack_size;
+ unsigned prio;
+};
You can embedded the stack on the struct making it the last field with size 0:

char stack[0];
How would one allocate a stack in this case? Normally it might be
something like:

struct fiber_config *conf = malloc(sizeof(*conf) + STACK_SIZE);

But I fail to see how it could be done easily without dynamic
allocation.
Just have it in a super struct:

struct config {
struct fiber_config fg;
char __stack[STACK_SIZE];
}
I'm not sure that helps in the bigger picture (although I admit I had
overlooked this possibility when I first asked you the question :]).

Consider the case where we have multiple instances of the same device.
Each instance has its own config structure. If we use the method you
describe above, all config structures would grow by one stack,
regardless if they need their own workqueue.

The following is, I believe, better:

/* Sensor 1 */
static char __stack dev_1_own_stack[1024];

static const struct my_dev_config dev_1_conf = {
/* ... */
.fiber_config = {
.stack = dev_1_own_stack,
.size = 1024,
},
/*... */
};

/* Sensor 2 -- same type, different chip */
static const struct my_dev_config dev_2_conf = {
/* ... */
.fiber_config = {
.stack = NULL, /* Use the global workqueue */
},
/* ... */
};
Not sure in what context you are taking this, the workqueue and work
should be 2 different objects, the workqueue object may or may not
carry its fiber_config depending if you want to share perhaps it needs
to be a separate variable, but for sure it wouldn't be part of the
work object for the reason you just pointed above so this design is
perhaps sub-optimal and should not be used in practice.

--
Luiz Augusto von Dentz

Join devel@lists.zephyrproject.org to automatically receive all group messages.