mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 00/12] poller: run pollers as proper coroutines (green threads)
@ 2021-02-15 10:36 Ahmad Fatoum
  2021-02-15 10:36 ` [PATCH 01/12] common: add coroutine support Ahmad Fatoum
                   ` (11 more replies)
  0 siblings, 12 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2021-02-15 10:36 UTC (permalink / raw)
  To: barebox

barebox pollers were so far very limited coroutines that only yield at
the end of the function. This series leverages setjmp/longjmp to allow
pollers to yield at any point of their execution. The motivation behind
this is that it finally becomes feasible to port threaded code to barebox with
minimal modification: If your thread has a delay, just stick in a
poller_yield() and give other threads as well as the main thread a chance
to run before resuming execution again.

How that can look like is showing in "usbgadget: ums: run gadget loop in a
background coroutine if possible", which turns a blocking loop very
naturally into a thread.

The implementation is basically: setjmp() to store the original place of
execution, longjmp() to jump back on a context switch, initjmp() to start
a new poller the very first time. The reason why we need an extra initjmp()
beyond standard C setjmp and longjmp is because those two only save a
stack pointer, so coroutines can clobber each other's stack.
With initjmp, the new coroutine starts directly executing on a disjoint stack.
initjmp, like setjmp and longjmp should be implemented in assembly, but it
should be fairly straight forward.  e.g. the ARM implementation for initjmp is
a mere 4 instructions long.

For now, I only implemented this for ARM and sandbox. If this is accepted and
becomes available for other architectures as well, it'll likely result in
changes to other parts of barebox:

  - Workqueues will become obsolete. This series already has pollers
    doing file system access yielding until they can be run in command context

  - Slices will become obsolete. They are equivalent to a mutex_try_lock
    and we can implement blocking mutexes now

  - Functions calling poller_call can be changed to call poller_reschedule()
    to become available from calling from any context

  - Backtraces should indicate which coroutine is currently running

The execution tax on context switching is that for an idle i.MX8MM system
30% less pollers can be executed in a given time frame. The extra accounting
overhead for a yielding poller is in the range of a 100 byte or so, which
is negligible compared to the stack newly allocated on poller creation, which
is CONFIG_STACK_SIZE bytes long, usually 32K. Pollers probably don't need that
much stack space, but for now, we just use the same stack size everywhere.

This is series is based on:
 - <20210215102442.28731-1-a.fatoum@pengutronix.de> to avoid a conflict
 - <20210215102740.30418-1-a.fatoum@pengutronix.de> which adds blocking
   usb mass storage, which is made non-blocing here

Note: Despite the name green threads (or fibers), this is wholly cooperative.
There is no preemption and thus no new extra precautions that need to be taken
when writing pollers. On the contrary, if we make this available everywhere,
you can basically do anything in a poller.

Looking forward to feedback,
Ahmad Fatoum (12):
  common: add coroutine support
  poller: run pollers as proper coroutines if architecture supports it
  ARM: asm: setjmp: annotate setjmp/longjmp for GCC
  ARM: asm: setjmp: implement coroutine dependency initjmp()
  sandbox: asm: implement setjmp/longjmp/initjmp
  poller: command: add new coroutine check
  slice: have assert_command_context() yield until true if possible
  poller: implement basic Linux-like completion API
  include: add kthread wrappers for pollers
  usbgadget: ums: run gadget loop in a background coroutine if possible
  usbgadget: refactor usbgadget_register to accept array
  usbgadget: multi: wire mass storage gadget into composite gadget

 Documentation/user/usb.rst          |   2 +
 arch/arm/Kconfig                    |   1 +
 arch/arm/include/asm/setjmp.h       |   6 +-
 arch/arm/lib32/setjmp.S             |   8 ++
 arch/arm/lib64/setjmp.S             |   9 ++
 arch/sandbox/Kconfig                |   1 +
 arch/sandbox/Makefile               |   5 +-
 arch/sandbox/include/asm/setjmp.h   |  17 +++
 arch/sandbox/os/Makefile            |   5 +-
 arch/sandbox/os/setjmp.c            | 180 ++++++++++++++++++++++++++++
 commands/Kconfig                    |  10 ++
 commands/Makefile                   |   2 +-
 commands/usbgadget.c                |  29 +++--
 common/Kconfig                      |  17 +++
 common/Makefile                     |   1 +
 common/coroutine.c                  | 178 +++++++++++++++++++++++++++
 common/poller.c                     | 111 ++++++++++++++---
 common/usbgadget.c                  |  55 +++++++--
 drivers/usb/gadget/Kconfig          |  12 +-
 drivers/usb/gadget/f_mass_storage.c |  14 +++
 drivers/usb/gadget/multi.c          |  36 ++++++
 include/coroutine.h                 |  43 +++++++
 include/linux/completion.h          |  61 ++++++++++
 include/linux/kthread.h             |  72 +++++++++++
 include/poller.h                    |  16 ++-
 include/slice.h                     |  14 ++-
 include/usb/gadget-multi.h          |  19 ++-
 27 files changed, 869 insertions(+), 55 deletions(-)
 create mode 100644 arch/sandbox/include/asm/setjmp.h
 create mode 100644 arch/sandbox/os/setjmp.c
 create mode 100644 common/coroutine.c
 create mode 100644 include/coroutine.h
 create mode 100644 include/linux/completion.h
 create mode 100644 include/linux/kthread.h

-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 01/12] common: add coroutine support
  2021-02-15 10:36 [PATCH 00/12] poller: run pollers as proper coroutines (green threads) Ahmad Fatoum
@ 2021-02-15 10:36 ` Ahmad Fatoum
  2021-02-15 10:36 ` [PATCH 02/12] poller: run pollers as proper coroutines if architecture supports it Ahmad Fatoum
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2021-02-15 10:36 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Coroutines generalize subroutines for non-preemptive multitasking,
by allowing execution to be suspended and resumed. We already have very
limited coroutines in the form of pollers. A poller is a function that
is cooperatively scheduled and yields after it has run to completion.
In the next poller_call(), this function is resumed from start.

Proper coroutines allow for the this yielding to happen at any point of
time. The coroutine's state is then saved and execution continues else
where. Later on, execution is resumed by restoring the saved context.

standard C setjmp/longjmp can be used to implement stackless coroutines.
setjmp stores the registers comprising the execution context into a
jmp_buf and longjmp switches to that context and continues execution just
after the setjmp that allocated that jmp_buf.

These coroutines are stackless, because jumping to a setjmp down the
call stack means that the code there will clobber the stack
below it. On resuming the coroutine, it will run with a stack changed
in the interim leading to undefined behavior.

There are ways around that without resorting to Assembly:

  - Allocate a buffer on the scheduler's stack, so coroutine can
    grow into them
     -> Problem: exploits Undefined behavior
  - Yield first time on scheduler stack, then patch jmp_buf to point at
    another stack
     -> Problem: Code switching stacks should not itself use the stack

It thus seems there is no way around adding a new function to initialize
a setjmp with a freshly cloned stack.

This commit adds the C boilerplate. Architectures wishing to use it need
to provide setjmp/longjmp/initjmp and in their arch Kconfig should
select CONFIG_HAS_ARCH_SJLJ. Code wishing to make use of it will need a
depends on CONFIG_HAS_ARCH_SJLJ. For now this will just be the new
poller_yield() facility introduced in a later commit.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/Kconfig      |   6 ++
 common/Makefile     |   1 +
 common/coroutine.c  | 178 ++++++++++++++++++++++++++++++++++++++++++++
 include/coroutine.h |  43 +++++++++++
 4 files changed, 228 insertions(+)
 create mode 100644 common/coroutine.c
 create mode 100644 include/coroutine.h

diff --git a/common/Kconfig b/common/Kconfig
index edadcc9f4979..d78aad1deb6b 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -20,6 +20,12 @@ config HAS_CACHE
 	  Drivers that depend on a cache implementation can depend on this
 	  config, so that you don't get a compilation error.
 
+config HAS_ARCH_SJLJ
+	bool
+	help
+	  Architecture has support implemented for
+	  setjmp()/longjmp()/initjmp()
+
 config HAS_DMA
 	bool
 	help
diff --git a/common/Makefile b/common/Makefile
index 0e0ba384c9b5..e85a27713177 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_OFTREE)		+= oftree.o
 obj-$(CONFIG_PARTITION_DISK)	+= partitions.o partitions/
 obj-$(CONFIG_PASSWORD)		+= password.o
 obj-$(CONFIG_POLLER)		+= poller.o
+obj-$(CONFIG_HAS_ARCH_SJLJ)	+= coroutine.o
 obj-$(CONFIG_RESET_SOURCE)	+= reset_source.o
 obj-$(CONFIG_SHELL_HUSH)	+= hush.o
 obj-$(CONFIG_SHELL_SIMPLE)	+= parser.o
diff --git a/common/coroutine.c b/common/coroutine.c
new file mode 100644
index 000000000000..d21cfc45067f
--- /dev/null
+++ b/common/coroutine.c
@@ -0,0 +1,178 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 Ahmad Fatoum, Pengutronix
+ *
+ * ASAN bookkeeping based on Qemu coroutine-ucontext.c
+ */
+
+/* To avoid future issues; fortify doesn't like longjmp up the call stack */
+#ifndef __NO_FORTIFY
+#define __NO_FORTIFY
+#endif
+
+#include <common.h>
+#include <coroutine.h>
+#include <asm/setjmp.h>
+#include <linux/overflow.h>
+
+/** struct coroutine
+ *
+ * @entry	coroutine entry point
+ * @arg		coroutine user-supplied argument
+ * @jmp_buf	coroutine context when yielded
+ * @stack	pointer to stack bottom (for stacks growing down)
+ * @stack_size	size of stack in bytes
+ * @stack_space actual stack for coroutines; empty for main thread
+ */
+struct coroutine {
+	__coroutine (*entry)(void *);
+	void *arg;
+	jmp_buf jmp_buf;
+	void *stack;
+	size_t stack_size;
+	u8 stack_space[] __aligned(16);
+};
+
+/** enum coroutine_action
+ *
+ * @COROUTINE_CHECKPOINT	placeholder, 0 reserved for regular setjmp return
+ * @COROUTINE_BOOTSTRAP		initial yield in trampline
+ * @COROUTINE_ENTER		switching from scheduler to coroutine
+ * @COROUTINE_YIELD		switching from coroutine to scheduler
+ * @COROUTINE_TERMINATE		final yield in trampoline
+ */
+enum coroutine_action {
+	COROUTINE_CHECKPOINT = 0,
+	COROUTINE_BOOTSTRAP,
+	COROUTINE_ENTER,
+	COROUTINE_YIELD,
+	COROUTINE_TERMINATE
+};
+
+/** The main "thread". Execution returns here when a coroutine yields */
+static struct coroutine scheduler;
+/** Argument for trampoline as initjmp doesn't pass an argument */
+static struct coroutine *new_coro;
+
+static enum coroutine_action coroutine_switch(struct coroutine *from,
+					      struct coroutine *to,
+					      enum coroutine_action action);
+
+static void __noreturn coroutine_trampoline(void)
+{
+	struct coroutine *coro = new_coro;
+
+	coroutine_switch(coro, &scheduler, COROUTINE_BOOTSTRAP);
+
+	coro->entry(coro->arg);
+
+	longjmp(scheduler.jmp_buf, COROUTINE_TERMINATE);
+}
+
+struct coroutine *coroutine_alloc(__coroutine (*entry)(void *), void *arg)
+{
+	struct coroutine *coro;
+	int ret;
+
+	coro = malloc(struct_size(coro, stack_space, CONFIG_STACK_SIZE));
+	if (!coro)
+		return NULL;
+
+	coro->stack = coro->stack_space;
+	coro->stack_size = CONFIG_STACK_SIZE;
+	coro->entry = entry;
+	coro->arg = arg;
+
+	/* set up coroutine context with the new stack */
+	ret = initjmp(coro->jmp_buf, coroutine_trampoline,
+		      coro->stack + CONFIG_STACK_SIZE);
+	if (ret) {
+		free(coro);
+		return NULL;
+	}
+
+	/* jump to new context to finish initialization */
+	new_coro = coro;
+	coroutine_schedule(coro);
+	new_coro = NULL;
+
+	return coro;
+}
+
+void coroutine_schedule(struct coroutine *coro)
+{
+	coroutine_switch(&scheduler, coro, COROUTINE_ENTER);
+}
+
+void coroutine_yield(struct coroutine *coro)
+{
+	coroutine_switch(coro, &scheduler, COROUTINE_YIELD);
+}
+
+void coroutine_free(struct coroutine *coro)
+{
+	free(coro);
+}
+
+/*
+ * When using ASAN, it needs to be told when we switch stacks.
+ */
+static void start_switch_fiber(void **fake_stack, struct coroutine *to);
+static void finish_switch_fiber(void *fake_stack_save);
+
+static enum coroutine_action coroutine_switch(struct coroutine *from, struct coroutine *to,
+					      enum coroutine_action action)
+{
+	void *fake_stack_save = NULL;
+	int ret;
+
+	if (action == COROUTINE_BOOTSTRAP)
+		finish_switch_fiber(NULL);
+
+	ret = setjmp(from->jmp_buf);
+	if (ret == 0) {
+		start_switch_fiber(action == COROUTINE_TERMINATE ? NULL : &fake_stack_save, to);
+		longjmp(to->jmp_buf, COROUTINE_YIELD);
+	}
+
+	finish_switch_fiber(fake_stack_save);
+
+	return ret;
+}
+
+#ifdef CONFIG_ASAN
+
+void __sanitizer_start_switch_fiber(void **fake_stack_save, const void *bottom, size_t size);
+void __sanitizer_finish_switch_fiber(void *fake_stack_save, const void **bottom_old, size_t *size_old);
+
+static void finish_switch_fiber(void *fake_stack_save)
+{
+    const void *bottom_old;
+    size_t size_old;
+
+    __sanitizer_finish_switch_fiber(fake_stack_save, &bottom_old, &size_old);
+
+    if (!scheduler.stack) {
+        scheduler.stack = (void *)bottom_old;
+        scheduler.stack_size = size_old;
+    }
+}
+
+static void start_switch_fiber(void **fake_stack_save,
+                               struct coroutine *to)
+{
+	__sanitizer_start_switch_fiber(fake_stack_save, to->stack, to->stack_size);
+}
+
+#else
+
+static void finish_switch_fiber(void *fake_stack_save)
+{
+}
+
+static void start_switch_fiber(void **fake_stack_save,
+                               struct coroutine *to)
+{
+}
+
+#endif
diff --git a/include/coroutine.h b/include/coroutine.h
new file mode 100644
index 000000000000..f6484e930166
--- /dev/null
+++ b/include/coroutine.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 Ahmad Fatoum, Pengutronix
+ */
+
+#ifndef __COROUTINE_H_
+#define __COROUTINE_H_
+
+#include <linux/stddef.h>
+
+struct coroutine;
+
+#define __coroutine void
+
+#ifdef CONFIG_HAS_ARCH_SJLJ
+
+struct coroutine *coroutine_alloc(__coroutine (*entry)(void *), void *arg);
+void coroutine_schedule(struct coroutine *coro);
+void coroutine_yield(struct coroutine *coro);
+void coroutine_free(struct coroutine *coro);
+
+#else
+
+static inline struct coroutine *coroutine_alloc(__coroutine (*entry)(void *), void *arg)
+{
+	return NULL;
+}
+
+static inline void coroutine_schedule(struct coroutine *coro)
+{
+}
+
+static inline void coroutine_yield(struct coroutine *coro)
+{
+}
+
+static inline void coroutine_free(struct coroutine *coro)
+{
+}
+
+#endif
+
+#endif
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 02/12] poller: run pollers as proper coroutines if architecture supports it
  2021-02-15 10:36 [PATCH 00/12] poller: run pollers as proper coroutines (green threads) Ahmad Fatoum
  2021-02-15 10:36 ` [PATCH 01/12] common: add coroutine support Ahmad Fatoum
@ 2021-02-15 10:36 ` Ahmad Fatoum
  2021-02-15 10:36 ` [PATCH 03/12] ARM: asm: setjmp: annotate setjmp/longjmp for GCC Ahmad Fatoum
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2021-02-15 10:36 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

barebox pollers were so far very limited coroutines that only yield at
the end of the function. Architectures which select CONFIG_HAS_ARCH_SJLJ
can leverage the new coroutine support to allow pollers to yield at any
point of their execution. These pollers are suspended and will be
resumed the next time poller_call() runs. This considerably eases
porting of threaded code, as each poller becomes a thread and
poller_call() becomes the scheduler.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/Kconfig   | 11 ++++++++
 common/poller.c  | 70 ++++++++++++++++++++++++++++++++++++++++++------
 include/poller.h | 16 ++++++++++-
 include/slice.h  |  6 ++---
 4 files changed, 91 insertions(+), 12 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index d78aad1deb6b..f3158d914800 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -960,6 +960,17 @@ config BAREBOXCRC32_TARGET
 
 config POLLER
 	bool "generic polling infrastructure"
+	help
+	 Pollers are routines that are called within delay loops and the console
+	 idle to asynchronously execute actions, like checking for link up or
+	 feeding a watchdog.
+
+config POLLER_YIELD
+	bool "Yielding poller support (coroutines)"
+	depends on HAS_ARCH_SJLJ
+	help
+	  Pollers may call poller_yield(), which saves context of the current
+	  poller for later resumption. This is often called green threads.
 
 config STATE
 	bool "generic state infrastructure"
diff --git a/common/poller.c b/common/poller.c
index 61da5698d225..592dc0a11a39 100644
--- a/common/poller.c
+++ b/common/poller.c
@@ -12,9 +12,12 @@
 #include <clock.h>
 #include <work.h>
 #include <slice.h>
+#include <coroutine.h>
 
 static LIST_HEAD(poller_list);
-int poller_active;
+struct poller_struct *active_poller;
+
+static __coroutine poller_thread(void *data);
 
 int poller_register(struct poller_struct *poller, const char *name)
 {
@@ -22,6 +25,10 @@ int poller_register(struct poller_struct *poller, const char *name)
 		return -EBUSY;
 
 	poller->name = xstrdup(name);
+
+	if (IS_ENABLED(CONFIG_POLLER_YIELD))
+		poller->coroutine = coroutine_alloc(poller_thread, poller);
+
 	list_add_tail(&poller->list, &poller_list);
 	poller->registered = 1;
 
@@ -36,6 +43,7 @@ int poller_unregister(struct poller_struct *poller)
 
 	list_del(&poller->list);
 	poller->registered = 0;
+	coroutine_free(poller->coroutine);
 	free(poller->name);
 
 	return 0;
@@ -78,7 +86,6 @@ int poller_async_cancel(struct poller_async *pa)
  * @pa		the poller to be used
  * @delay	The delay in nanoseconds
  * @fn		The function to call
- * @ctx		context pointer passed to the function
  *
  * This calls the passed function after a delay of delay_ns. Returns
  * a pointer which can be used as a cookie to cancel a scheduled call.
@@ -107,12 +114,59 @@ int poller_async_unregister(struct poller_async *pa)
 	return poller_unregister(&pa->poller);
 }
 
+static void __poller_yield(struct poller_struct *poller)
+{
+	if (WARN_ON(!poller))
+		return;
+
+	coroutine_yield(poller->coroutine);
+}
+
+#ifdef CONFIG_POLLER_YIELD
+/* No stub for this function. That way we catch wrong Kconfig dependencies
+ * that enable code that uses poller_yield() unconditionally
+ */
+void poller_yield(void)
+{
+	return __poller_yield(active_poller);
+}
+#endif
+
+int poller_reschedule(void)
+{
+	if (!in_poller())
+		return ctrlc() ? -ERESTARTSYS : 0;
+
+	__poller_yield(active_poller);
+	return 0;
+}
+
+static __coroutine poller_thread(void *data)
+{
+	struct poller_struct *poller = data;
+
+	for (;;) {
+		poller->func(poller);
+		__poller_yield(poller);
+	}
+}
+
+static void poller_schedule(struct poller_struct *poller)
+{
+	if (!IS_ENABLED(CONFIG_POLLER_YIELD)) {
+		poller->func(poller);
+		return;
+	}
+
+	coroutine_schedule(poller->coroutine);
+}
+
 void poller_call(void)
 {
 	struct poller_struct *poller, *tmp;
 	bool run_workqueues = !slice_acquired(&command_slice);
 
-	if (poller_active)
+	if (active_poller)
 		return;
 
 	command_slice_acquire();
@@ -120,13 +174,13 @@ void poller_call(void)
 	if (run_workqueues)
 		wq_do_all_works();
 
-	poller_active = 1;
-
-	list_for_each_entry_safe(poller, tmp, &poller_list, list)
-		poller->func(poller);
+	list_for_each_entry_safe(poller, tmp, &poller_list, list) {
+		active_poller = poller;
+		poller_schedule(poller);
+	}
 
+	active_poller = NULL;
 	command_slice_release();
-	poller_active = 0;
 }
 
 #if defined CONFIG_CMD_POLLER
diff --git a/include/poller.h b/include/poller.h
index db773265b2f6..ac3c5865ba2d 100644
--- a/include/poller.h
+++ b/include/poller.h
@@ -8,13 +8,18 @@
 
 #include <linux/list.h>
 
+struct coroutine;
+
 struct poller_struct {
 	void (*func)(struct poller_struct *poller);
 	int registered;
 	struct list_head list;
 	char *name;
+	struct coroutine *coroutine;
 };
 
+extern struct poller_struct *active_poller;
+
 int poller_register(struct poller_struct *poller, const char *name);
 int poller_unregister(struct poller_struct *poller);
 
@@ -39,12 +44,21 @@ static inline bool poller_async_active(struct poller_async *pa)
 	return pa->active;
 }
 
+static inline bool in_poller(void)
+{
+	return active_poller != NULL;
+}
+
 #ifdef CONFIG_POLLER
 void poller_call(void);
 #else
 static inline void poller_call(void)
 {
 }
-#endif	/* CONFIG_POLLER */
+#endif /* CONFIG_POLLER */
+
+/* Only for use when CONFIG_POLLER_YIELD=y */
+void poller_yield(void);
+int poller_reschedule(void);
 
 #endif	/* !POLLER_H */
diff --git a/include/slice.h b/include/slice.h
index b2d65b80cd69..d09d17924fb4 100644
--- a/include/slice.h
+++ b/include/slice.h
@@ -1,6 +1,8 @@
 #ifndef __SLICE_H
 #define __SLICE_H
 
+#include <poller.h>
+
 enum slice_action {
 	SLICE_ACQUIRE = 1,
 	SLICE_RELEASE = -1,
@@ -33,11 +35,9 @@ extern struct slice command_slice;
 void command_slice_acquire(void);
 void command_slice_release(void);
 
-extern int poller_active;
-
 #ifdef CONFIG_POLLER
 #define assert_command_context() ({    \
-	WARN_ONCE(poller_active, "%s called in poller\n", __func__); \
+	WARN_ONCE(in_poller(), "%s called in poller\n", __func__); \
 })
 #else
 #define assert_command_context() do { } while (0)
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 03/12] ARM: asm: setjmp: annotate setjmp/longjmp for GCC
  2021-02-15 10:36 [PATCH 00/12] poller: run pollers as proper coroutines (green threads) Ahmad Fatoum
  2021-02-15 10:36 ` [PATCH 01/12] common: add coroutine support Ahmad Fatoum
  2021-02-15 10:36 ` [PATCH 02/12] poller: run pollers as proper coroutines if architecture supports it Ahmad Fatoum
@ 2021-02-15 10:36 ` Ahmad Fatoum
  2021-02-15 10:36 ` [PATCH 04/12] ARM: asm: setjmp: implement coroutine dependency initjmp() Ahmad Fatoum
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2021-02-15 10:36 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

To avoid invalid optimizations and to enable warnings, GCC must be told
that setjmp and longjmp are to be handled specially. Add the missing
attributes.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/include/asm/setjmp.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/setjmp.h b/arch/arm/include/asm/setjmp.h
index 62bac613d6e1..538d9cd50651 100644
--- a/arch/arm/include/asm/setjmp.h
+++ b/arch/arm/include/asm/setjmp.h
@@ -23,7 +23,7 @@ struct jmp_buf_data {
 
 typedef struct jmp_buf_data jmp_buf[1];
 
-int setjmp(jmp_buf jmp);
-void longjmp(jmp_buf jmp, int ret);
+int setjmp(jmp_buf jmp) __attribute__((returns_twice));
+void longjmp(jmp_buf jmp, int ret) __attribute__((noreturn));
 
 #endif /* _SETJMP_H_ */
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 04/12] ARM: asm: setjmp: implement coroutine dependency initjmp()
  2021-02-15 10:36 [PATCH 00/12] poller: run pollers as proper coroutines (green threads) Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2021-02-15 10:36 ` [PATCH 03/12] ARM: asm: setjmp: annotate setjmp/longjmp for GCC Ahmad Fatoum
@ 2021-02-15 10:36 ` Ahmad Fatoum
  2021-02-15 10:36 ` [PATCH 05/12] sandbox: asm: implement setjmp/longjmp/initjmp Ahmad Fatoum
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2021-02-15 10:36 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Implement initjmp() for use with the coroutine implementation.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/Kconfig              | 1 +
 arch/arm/include/asm/setjmp.h | 2 ++
 arch/arm/lib32/setjmp.S       | 8 ++++++++
 arch/arm/lib64/setjmp.S       | 9 +++++++++
 4 files changed, 20 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index ab0bf030131c..cdb934136e34 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -5,6 +5,7 @@ config ARM
 	select HAVE_CONFIGURABLE_TEXT_BASE if !RELOCATABLE
 	select HAVE_IMAGE_COMPRESSION
 	select HAVE_ARCH_KASAN
+	select HAS_ARCH_SJLJ
 	select ARM_OPTIMZED_STRING_FUNCTIONS if KASAN
 	default y
 
diff --git a/arch/arm/include/asm/setjmp.h b/arch/arm/include/asm/setjmp.h
index 538d9cd50651..4877e4312411 100644
--- a/arch/arm/include/asm/setjmp.h
+++ b/arch/arm/include/asm/setjmp.h
@@ -26,4 +26,6 @@ typedef struct jmp_buf_data jmp_buf[1];
 int setjmp(jmp_buf jmp) __attribute__((returns_twice));
 void longjmp(jmp_buf jmp, int ret) __attribute__((noreturn));
 
+int initjmp(jmp_buf jmp, void __noreturn (*func)(void), void *stack_top);
+
 #endif /* _SETJMP_H_ */
diff --git a/arch/arm/lib32/setjmp.S b/arch/arm/lib32/setjmp.S
index f0606a7f6659..626d915da183 100644
--- a/arch/arm/lib32/setjmp.S
+++ b/arch/arm/lib32/setjmp.S
@@ -33,4 +33,12 @@ ENTRY(longjmp)
 1:
 	bx   lr
 ENDPROC(longjmp)
+
+.pushsection .text.initjmp, "ax"
+ENTRY(initjmp)
+	str  a3, [a1, #32] /* stack pointer */
+	str  a2, [a1, #36] /* return address */
+	mov  a1, #0
+	bx   lr
+ENDPROC(initjmp)
 .popsection
diff --git a/arch/arm/lib64/setjmp.S b/arch/arm/lib64/setjmp.S
index 0910e2f5a6c3..80be8cb0f201 100644
--- a/arch/arm/lib64/setjmp.S
+++ b/arch/arm/lib64/setjmp.S
@@ -36,3 +36,12 @@ ENTRY(longjmp)
 	ret
 ENDPROC(longjmp)
 .popsection
+
+.pushsection .text.initjmp, "ax"
+ENTRY(initjmp)
+	str  x2, [x0, #96] /* stack pointer */
+	str  x1, [x0, #88] /* return address */
+	mov  x0, #0
+	ret
+ENDPROC(initjmp)
+.popsection
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 05/12] sandbox: asm: implement setjmp/longjmp/initjmp
  2021-02-15 10:36 [PATCH 00/12] poller: run pollers as proper coroutines (green threads) Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2021-02-15 10:36 ` [PATCH 04/12] ARM: asm: setjmp: implement coroutine dependency initjmp() Ahmad Fatoum
@ 2021-02-15 10:36 ` Ahmad Fatoum
  2021-02-15 10:36 ` [PATCH 06/12] poller: command: add new coroutine check Ahmad Fatoum
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2021-02-15 10:36 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

To extend yielding poller support to sandbox, implement setjmp, longjmp
and initjmp. Unlike bare metal platforms, setjmp() and longjmp() are
readily provided on standard-conforming hosted platforms.  initjmp() on
the other hand requires us to be able to invoke a function with a
user-supplied stack pointer, which isn't possible in standard C.

For POSIX systems, there are two methods to portably achieve this
though:

  - Use makecontext(2) to set up a new context. makecontext(2) was however
    removed in POSIX.1-2008 and at least GCC 10.2.1 ASan complains that it
    "doesn't fully support makecontext/swapcontext functions and may
    produce false positives in some cases!"

  - Use sigaltstack to set a new signal stack, call setjmp there to
    store the stack pointer, return regularly from signal handler and then
    longjmp back and invoke the coroutine.

Both methods are implemented in QEMU. While QEMU uses the makecontext
method by default, for the reasons described, import the second implementation
and use it implement initjmp.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/sandbox/Kconfig              |   1 +
 arch/sandbox/Makefile             |   5 +-
 arch/sandbox/include/asm/setjmp.h |  17 +++
 arch/sandbox/os/Makefile          |   5 +-
 arch/sandbox/os/setjmp.c          | 180 ++++++++++++++++++++++++++++++
 5 files changed, 204 insertions(+), 4 deletions(-)
 create mode 100644 arch/sandbox/include/asm/setjmp.h
 create mode 100644 arch/sandbox/os/setjmp.c

diff --git a/arch/sandbox/Kconfig b/arch/sandbox/Kconfig
index 1a4e3bacf66d..cef8e9fb7ab4 100644
--- a/arch/sandbox/Kconfig
+++ b/arch/sandbox/Kconfig
@@ -13,6 +13,7 @@ config SANDBOX
 	select PARTITION_DISK
 	select ARCH_HAS_STACK_DUMP if ASAN
 	select GENERIC_FIND_NEXT_BIT
+	select HAS_ARCH_SJLJ
 	default y
 
 config ARCH_TEXT_BASE
diff --git a/arch/sandbox/Makefile b/arch/sandbox/Makefile
index ea594944e4eb..5fc7e227be67 100644
--- a/arch/sandbox/Makefile
+++ b/arch/sandbox/Makefile
@@ -27,7 +27,8 @@ KBUILD_CFLAGS += -Dmalloc=barebox_malloc -Dcalloc=barebox_calloc \
 		-Dftruncate=barebox_ftruncate -Dasprintf=barebox_asprintf \
 		-Dopendir=barebox_opendir -Dreaddir=barebox_readdir \
 		-Dclosedir=barebox_closedir -Dreadlink=barebox_readlink \
-		-Doptarg=barebox_optarg -Doptind=barebox_optind
+		-Doptarg=barebox_optarg -Doptind=barebox_optind \
+		-Dsetjmp=barebox_setjmp -Dlongjmp=barebox_longjmp
 
 machdirs := $(patsubst %,arch/sandbox/mach-%/,$(machine-y))
 
@@ -64,7 +65,7 @@ endif
 BAREBOX_LDFLAGS += \
 	-Wl,-T,$(BAREBOX_LDS) \
 	-Wl,--whole-archive $(BAREBOX_OBJS) -Wl,--no-whole-archive \
-	-lrt $(SDL_LIBS) $(FTDI1_LIBS) \
+	-lrt -pthread $(SDL_LIBS) $(FTDI1_LIBS) \
 	$(SANITIZER_LIBS)
 
 cmd_barebox__ = $(CC) -o $@ $(BAREBOX_LDFLAGS)
diff --git a/arch/sandbox/include/asm/setjmp.h b/arch/sandbox/include/asm/setjmp.h
new file mode 100644
index 000000000000..f085a9079dd7
--- /dev/null
+++ b/arch/sandbox/include/asm/setjmp.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __SETJMP_H_
+#define __SETJMP_H_
+
+struct jmp_buf_data {
+	unsigned char opaque[512] __aligned(16);
+};
+
+typedef struct jmp_buf_data jmp_buf[1];
+
+int setjmp(jmp_buf jmp) __attribute__((returns_twice));
+void longjmp(jmp_buf jmp, int ret) __attribute__((noreturn));
+
+int initjmp(jmp_buf jmp, void __noreturn (*func)(void), void *stack_top);
+
+#endif
diff --git a/arch/sandbox/os/Makefile b/arch/sandbox/os/Makefile
index fb2c3cfd8632..5d0c938ce68c 100644
--- a/arch/sandbox/os/Makefile
+++ b/arch/sandbox/os/Makefile
@@ -4,7 +4,8 @@ machdirs := $(patsubst %,arch/sandbox/mach-%/,$(machine-y))
 
 KBUILD_CPPFLAGS = $(patsubst %,-I$(srctree)/%include,$(machdirs))
 
-KBUILD_CPPFLAGS += -DCONFIG_MALLOC_SIZE=$(CONFIG_MALLOC_SIZE) -D_FILE_OFFSET_BITS=64
+KBUILD_CPPFLAGS += -DCONFIG_MALLOC_SIZE=$(CONFIG_MALLOC_SIZE) -D_FILE_OFFSET_BITS=64 \
+		   -DCONFIG_STACK_SIZE=$(CONFIG_STACK_SIZE)
 
 KBUILD_CFLAGS := -Wall
 
@@ -14,7 +15,7 @@ ifeq ($(CONFIG_SANDBOX_LINUX_I386),y)
 KBUILD_CFLAGS += -m32
 endif
 
-obj-y = common.o tap.o
+obj-y = common.o tap.o setjmp.o
 obj-$(CONFIG_MALLOC_LIBC) += libc_malloc.o
 
 CFLAGS_sdl.o = $(shell pkg-config sdl2 --cflags)
diff --git a/arch/sandbox/os/setjmp.c b/arch/sandbox/os/setjmp.c
new file mode 100644
index 000000000000..7f686b0fc6e9
--- /dev/null
+++ b/arch/sandbox/os/setjmp.c
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+/*
+ * sigaltstack coroutine initialization code
+ *
+ * Copyright (C) 2006  Anthony Liguori <anthony@codemonkey.ws>
+ * Copyright (C) 2011  Kevin Wolf <kwolf@redhat.com>
+ * Copyright (C) 2012  Alex Barcelo <abarcelo@ac.upc.edu>
+ * Copyright (C) 2021  Ahmad Fatoum, Pengutronix
+ * This file is partly based on pth_mctx.c, from the GNU Portable Threads
+ *  Copyright (c) 1999-2006 Ralf S. Engelschall <rse@engelschall.com>
+ */
+
+/* XXX Is there a nicer way to disable glibc's stack check for longjmp? */
+#ifdef _FORTIFY_SOURCE
+#undef _FORTIFY_SOURCE
+#endif
+
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <setjmp.h>
+#include <signal.h>
+
+typedef sigjmp_buf _jmp_buf __attribute__((aligned((16))));
+_Static_assert(sizeof(_jmp_buf) <= 512, "sigjmp_buf size exceeds expectation");
+
+/*
+ * Information for the signal handler (trampoline)
+ */
+static struct {
+	_jmp_buf *reenter;
+	void (*entry)(void);
+	volatile sig_atomic_t called;
+} tr_state;
+
+/*
+ * "boot" function
+ * This is what starts the coroutine, is called from the trampoline
+ * (from the signal handler when it is not signal handling, read ahead
+ * for more information).
+ */
+static void __attribute__((noinline, noreturn))
+coroutine_bootstrap(void (*entry)(void))
+{
+	for (;;)
+		entry();
+}
+
+/*
+ * This is used as the signal handler. This is called with the brand new stack
+ * (thanks to sigaltstack). We have to return, given that this is a signal
+ * handler and the sigmask and some other things are changed.
+ */
+static void coroutine_trampoline(int signal)
+{
+	/* Get the thread specific information */
+	tr_state.called = 1;
+
+	/*
+	 * Here we have to do a bit of a ping pong between the caller, given that
+	 * this is a signal handler and we have to do a return "soon". Then the
+	 * caller can reestablish everything and do a siglongjmp here again.
+	 */
+	if (!sigsetjmp(*tr_state.reenter, 0)) {
+		return;
+	}
+
+	/*
+	 * Ok, the caller has siglongjmp'ed back to us, so now prepare
+	 * us for the real machine state switching. We have to jump
+	 * into another function here to get a new stack context for
+	 * the auto variables (which have to be auto-variables
+	 * because the start of the thread happens later). Else with
+	 * PIC (i.e. Position Independent Code which is used when PTH
+	 * is built as a shared library) most platforms would
+	 * horrible core dump as experience showed.
+	 */
+	coroutine_bootstrap(tr_state.entry);
+}
+
+int initjmp(_jmp_buf jmp, void (*func)(void), void *stack_top)
+{
+	struct sigaction sa;
+	struct sigaction osa;
+	stack_t ss;
+	stack_t oss;
+	sigset_t sigs;
+	sigset_t osigs;
+
+	/* The way to manipulate stack is with the sigaltstack function. We
+	 * prepare a stack, with it delivering a signal to ourselves and then
+	 * put sigsetjmp/siglongjmp where needed.
+	 * This has been done keeping coroutine-ucontext (from the QEMU project)
+	 * as a model and with the pth ideas (GNU Portable Threads).
+	 * See coroutine-ucontext for the basics of the coroutines and see
+	 * pth_mctx.c (from the pth project) for the
+	 * sigaltstack way of manipulating stacks.
+	 */
+
+	tr_state.entry = func;
+	tr_state.reenter = (void *)jmp;
+
+	/*
+	 * Preserve the SIGUSR2 signal state, block SIGUSR2,
+	 * and establish our signal handler. The signal will
+	 * later transfer control onto the signal stack.
+	 */
+	sigemptyset(&sigs);
+	sigaddset(&sigs, SIGUSR2);
+	pthread_sigmask(SIG_BLOCK, &sigs, &osigs);
+	sa.sa_handler = coroutine_trampoline;
+	sigfillset(&sa.sa_mask);
+	sa.sa_flags = SA_ONSTACK;
+	if (sigaction(SIGUSR2, &sa, &osa) != 0) {
+		return -1;
+	}
+
+	/*
+	 * Set the new stack.
+	 */
+	ss.ss_sp = stack_top - CONFIG_STACK_SIZE;
+	ss.ss_size = CONFIG_STACK_SIZE;
+	ss.ss_flags = 0;
+	if (sigaltstack(&ss, &oss) < 0) {
+		return -1;
+	}
+
+	/*
+	 * Now transfer control onto the signal stack and set it up.
+	 * It will return immediately via "return" after the sigsetjmp()
+	 * was performed. Be careful here with race conditions.  The
+	 * signal can be delivered the first time sigsuspend() is
+	 * called.
+	 */
+	tr_state.called = 0;
+	pthread_kill(pthread_self(), SIGUSR2);
+	sigfillset(&sigs);
+	sigdelset(&sigs, SIGUSR2);
+	while (!tr_state.called) {
+		sigsuspend(&sigs);
+	}
+
+	/*
+	 * Inform the system that we are back off the signal stack by
+	 * removing the alternative signal stack. Be careful here: It
+	 * first has to be disabled, before it can be removed.
+	 */
+	sigaltstack(NULL, &ss);
+	ss.ss_flags = SS_DISABLE;
+	if (sigaltstack(&ss, NULL) < 0) {
+		return -1;
+	}
+	sigaltstack(NULL, &ss);
+	if (!(oss.ss_flags & SS_DISABLE)) {
+		sigaltstack(&oss, NULL);
+	}
+
+	/*
+	 * Restore the old SIGUSR2 signal handler and mask
+	 */
+	sigaction(SIGUSR2, &osa, NULL);
+	pthread_sigmask(SIG_SETMASK, &osigs, NULL);
+
+	/*
+	 * jmp can now be used to enter the trampoline again, but not as a
+	 * signal handler. Instead it's longjmp'd to directly.
+	 */
+
+	return 0;
+}
+
+int  __attribute__((returns_twice)) barebox_setjmp(_jmp_buf jmp)
+{
+	return sigsetjmp(jmp, 0);
+}
+
+void __attribute((noreturn)) barebox_longjmp(_jmp_buf jmp, int ret)
+{
+	siglongjmp(jmp, ret);
+}
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 06/12] poller: command: add new coroutine check
  2021-02-15 10:36 [PATCH 00/12] poller: run pollers as proper coroutines (green threads) Ahmad Fatoum
                   ` (4 preceding siblings ...)
  2021-02-15 10:36 ` [PATCH 05/12] sandbox: asm: implement setjmp/longjmp/initjmp Ahmad Fatoum
@ 2021-02-15 10:36 ` Ahmad Fatoum
  2021-02-15 10:37 ` [PATCH 07/12] slice: have assert_command_context() yield until true if possible Ahmad Fatoum
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2021-02-15 10:36 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The poller command can already counts how many pollers can execute
serially in a second. Add a new -c options that additionally
registers a poller that does non-blockingly sleep via poller_yield.

A system with working coroutines and enabled CONFIG_POLLER_YIELD should
show that the poller yielded exactly 4 times during considerably more
poller_call()s. For example:

    barebox@Embest MarS Board i.MX6Dual:/ poller -c
    Poller yield #1
    Poller yield #2
    Poller yield #3
    Poller yield #4
    295066 poller calls in 1s

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/poller.c | 41 +++++++++++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/common/poller.c b/common/poller.c
index 592dc0a11a39..a63af0e792a6 100644
--- a/common/poller.c
+++ b/common/poller.c
@@ -187,8 +187,9 @@ void poller_call(void)
 
 #include <command.h>
 #include <getopt.h>
+#include <clock.h>
 
-static void poller_time(void)
+static int poller_time(void)
 {
 	uint64_t start = get_time_ns();
 	int i = 0;
@@ -202,7 +203,7 @@ static void poller_time(void)
 	while (!is_timeout(start, SECOND))
 		i++;
 
-	printf("%d poller calls in 1s\n", i);
+	return i;
 }
 
 static void poller_info(void)
@@ -226,20 +227,48 @@ BAREBOX_CMD_HELP_TEXT("")
 BAREBOX_CMD_HELP_TEXT("Options:")
 BAREBOX_CMD_HELP_OPT ("-i", "Print information about registered pollers")
 BAREBOX_CMD_HELP_OPT ("-t", "measure how many pollers we run in 1s")
+BAREBOX_CMD_HELP_OPT ("-c", "run coroutine test")
 BAREBOX_CMD_HELP_END
 
+static void poller_coroutine(struct poller_struct *poller)
+{
+	volatile u64 start;
+	volatile int i = 0;
+
+	for (;;) {
+		start = get_time_ns();
+		while (!is_timeout_non_interruptible(start, 225 * MSECOND))
+			__poller_yield(active_poller);
+
+		printf("Poller yield #%d\n", ++i);
+	}
+}
+
 static int do_poller(int argc, char *argv[])
 {
-	int opt;
+	struct poller_struct poller = {};
+	int ret, opt;
 
-	while ((opt = getopt(argc, argv, "it")) > 0) {
+	while ((opt = getopt(argc, argv, "itc")) > 0) {
 		switch (opt) {
 		case 'i':
 			poller_info();
 			return 0;
+		case 'c':
+			if (!IS_ENABLED(CONFIG_POLLER_YIELD)) {
+				printf("CONFIG_POLLER_YIELD support not compiled in\n");
+				return -ENOSYS;
+			}
+
+			poller.func = poller_coroutine;
+			ret = poller_register(&poller, "poller-coroutine-test");
+			if (ret)
+				return ret;
+
+			/* fallthrough */
 		case 't':
-			poller_time();
-			return 0;
+			printf("%d poller calls in 1s\n", poller_time());
+			return poller.func ? poller_unregister(&poller) : 0;
 		}
 	}
 
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 07/12] slice: have assert_command_context() yield until true if possible
  2021-02-15 10:36 [PATCH 00/12] poller: run pollers as proper coroutines (green threads) Ahmad Fatoum
                   ` (5 preceding siblings ...)
  2021-02-15 10:36 ` [PATCH 06/12] poller: command: add new coroutine check Ahmad Fatoum
@ 2021-02-15 10:37 ` Ahmad Fatoum
  2021-02-15 10:37 ` [PATCH 08/12] poller: implement basic Linux-like completion API Ahmad Fatoum
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2021-02-15 10:37 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

assert_command_context() is a safety net to warn about pollers doing
file system operations. It so far prints a splat when this happens.

With pollers that may yield, this can be implemented differently by
just yielding until the poller is run in the command context. So so.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/slice.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/slice.h b/include/slice.h
index d09d17924fb4..eae74b9fc199 100644
--- a/include/slice.h
+++ b/include/slice.h
@@ -36,8 +36,14 @@ void command_slice_acquire(void);
 void command_slice_release(void);
 
 #ifdef CONFIG_POLLER
-#define assert_command_context() ({    \
-	WARN_ONCE(in_poller(), "%s called in poller\n", __func__); \
+#define assert_command_context() ({                                       \
+	while (in_poller() && !slice_acquired(&command_slice)) {          \
+		if (!IS_ENABLED(CONFIG_POLLER_YIELD)) {                   \
+			WARN_ONCE(1, "%s called in poller\n", __func__);  \
+			break;                                            \
+		}                                                         \
+		poller_yield();                                           \
+	}                                                                 \
 })
 #else
 #define assert_command_context() do { } while (0)
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 08/12] poller: implement basic Linux-like completion API
  2021-02-15 10:36 [PATCH 00/12] poller: run pollers as proper coroutines (green threads) Ahmad Fatoum
                   ` (6 preceding siblings ...)
  2021-02-15 10:37 ` [PATCH 07/12] slice: have assert_command_context() yield until true if possible Ahmad Fatoum
@ 2021-02-15 10:37 ` Ahmad Fatoum
  2021-02-15 10:37 ` [PATCH 09/12] include: add kthread wrappers for pollers Ahmad Fatoum
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2021-02-15 10:37 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

So far, completions made sense only in one direction: The main thread
can wait for pollers, but the other way around didn't work. With the new
yielding poller support, pollers can wait for completions as well. Wrap
this up using the Linux poller API to make porting threaded code easier.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/linux/completion.h | 61 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 include/linux/completion.h

diff --git a/include/linux/completion.h b/include/linux/completion.h
new file mode 100644
index 000000000000..f2db80716c7f
--- /dev/null
+++ b/include/linux/completion.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_COMPLETION_H
+#define __LINUX_COMPLETION_H
+
+/*
+ * (C) Copyright 2021 Ahmad Fatoum
+ *
+ * Async wait-for-completion handler data structures.
+ * This allows pollers to wait for main thread and vice versa
+ * Requires poller_yield() support
+ */
+
+#include <poller.h>
+#include <linux/bug.h>
+
+struct completion {
+	unsigned int done;
+};
+
+static inline void init_completion(struct completion *x)
+{
+	x->done = 0;
+}
+
+static inline void reinit_completion(struct completion *x)
+{
+	x->done = 0;
+}
+
+static inline int wait_for_completion_interruptible(struct completion *x)
+{
+	int ret;
+
+	while (!x->done) {
+		ret = poller_reschedule();
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static inline bool completion_done(struct completion *x)
+{
+	return x->done;
+}
+
+static inline void complete(struct completion *x)
+{
+	x->done = 1;
+}
+
+static inline void __noreturn complete_and_exit(struct completion *x, int ret)
+{
+	BUG_ON(!in_poller());
+	complete(x);
+	for (;;)
+		poller_yield();
+}
+
+#endif
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 09/12] include: add kthread wrappers for pollers
  2021-02-15 10:36 [PATCH 00/12] poller: run pollers as proper coroutines (green threads) Ahmad Fatoum
                   ` (7 preceding siblings ...)
  2021-02-15 10:37 ` [PATCH 08/12] poller: implement basic Linux-like completion API Ahmad Fatoum
@ 2021-02-15 10:37 ` Ahmad Fatoum
  2021-02-15 12:31   ` Sascha Hauer
  2021-02-15 10:37 ` [PATCH 10/12] usbgadget: ums: run gadget loop in a background coroutine if possible Ahmad Fatoum
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 14+ messages in thread
From: Ahmad Fatoum @ 2021-02-15 10:37 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

With the new fancy yielding poller support, we can properly represent
kernel threads in barebox. Do so.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/linux/kthread.h | 72 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 include/linux/kthread.h

diff --git a/include/linux/kthread.h b/include/linux/kthread.h
new file mode 100644
index 000000000000..17b6de9cf168
--- /dev/null
+++ b/include/linux/kthread.h
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_KTHREAD_H
+#define _LINUX_KTHREAD_H
+/* Wrapper around pollers to ease porting from Linux */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/string.h>
+#include <poller.h>
+
+struct task_struct {
+	struct poller_struct poller;
+	int (*threadfn)(void *data);
+	void *data;
+};
+
+static inline void kthread_poller(struct poller_struct *poller)
+{
+	struct task_struct *task = container_of(poller, struct task_struct, poller);
+	task->threadfn(task->data);
+}
+
+static inline struct task_struct *kthread_create(int (*threadfn)(void *data),
+						 void *data, const char *name)
+{
+	struct task_struct *task;
+
+	task = calloc(sizeof(*task), 1);
+	if (!task)
+		return ERR_PTR(-ENOMEM);
+
+	task->threadfn = threadfn;
+	task->data = data;
+	task->poller.func = kthread_poller;
+	task->poller.name = strdup(name);
+
+	return task;
+}
+
+static inline int wake_up_process(struct task_struct *task)
+{
+	return poller_register(&task->poller, task->poller.name);
+}
+
+/**
+ * kthread_run - create and wake a thread.
+ * @threadfn: the function to run until signal_pending(current).
+ * @data: data ptr for @threadfn.
+ * @name: name for the thread.
+ *
+ * Description: Convenient wrapper for kthread_create() followed by
+ * wake_up_process().  Returns the kthread or ERR_PTR(-ENOMEM).
+ */
+#define kthread_run(threadfn, data, name)				   \
+({									   \
+	struct task_struct *__k						   \
+		= kthread_create(threadfn, data, name);			   \
+	if (!IS_ERR(__k))						   \
+		wake_up_process(__k);					   \
+	__k;								   \
+})
+
+static inline void free_kthread_struct(struct task_struct *k)
+{
+	if (!k)
+		return;
+
+	poller_unregister(&k->poller);
+	free(k);
+}
+
+#endif /* _LINUX_KTHREAD_H */
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 10/12] usbgadget: ums: run gadget loop in a background coroutine if possible
  2021-02-15 10:36 [PATCH 00/12] poller: run pollers as proper coroutines (green threads) Ahmad Fatoum
                   ` (8 preceding siblings ...)
  2021-02-15 10:37 ` [PATCH 09/12] include: add kthread wrappers for pollers Ahmad Fatoum
@ 2021-02-15 10:37 ` Ahmad Fatoum
  2021-02-15 10:37 ` [PATCH 11/12] usbgadget: refactor usbgadget_register to accept array Ahmad Fatoum
  2021-02-15 10:37 ` [PATCH 12/12] usbgadget: multi: wire mass storage gadget into composite gadget Ahmad Fatoum
  11 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2021-02-15 10:37 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

With the new fancy poller yield support in place, we can let let the
loop yield in sleep_thread and naturally run in the background. Add this
and wrap it up inside an #ifdef, so the driver can still be used in
blocking mode for the architectures still missing out on poller yield
support.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/usb/gadget/f_mass_storage.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index 9c0076be26d1..c3e6eb933ce7 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -287,6 +287,18 @@ static struct usb_gadget_strings	fsg_stringtab = {
 
 /*-------------------------------------------------------------------------*/
 
+#ifdef CONFIG_POLLER_YIELD
+
+#include <linux/completion.h>
+#include <linux/kthread.h>
+
+static inline bool poll(void)
+{
+	return !ctrlc();
+}
+
+#else
+
 struct completion { int done; };
 
 #define init_completion(x) do { (x)->done = 0; } while (0)
@@ -331,6 +343,8 @@ static struct task_struct *kthread_run(int (*threadfn)(void *), void *arg,
 
 #define poll() thread_task->threadfn(thread_task->arg)
 
+#endif
+
 #define wait_event(queue, cond)	do { poll(); } while (!(cond))
 #define wake_up(...)		do {} while (0)
 
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 11/12] usbgadget: refactor usbgadget_register to accept array
  2021-02-15 10:36 [PATCH 00/12] poller: run pollers as proper coroutines (green threads) Ahmad Fatoum
                   ` (9 preceding siblings ...)
  2021-02-15 10:37 ` [PATCH 10/12] usbgadget: ums: run gadget loop in a background coroutine if possible Ahmad Fatoum
@ 2021-02-15 10:37 ` Ahmad Fatoum
  2021-02-15 10:37 ` [PATCH 12/12] usbgadget: multi: wire mass storage gadget into composite gadget Ahmad Fatoum
  11 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2021-02-15 10:37 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

usbgadget_register currently takes 6 arguments. Instead of increasing
them to 8 to support the new usb mass storage gadget, rewrite it to
accept a pointer to a struct with all the options instead.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/usbgadget.c       | 19 +++++++++----------
 common/usbgadget.c         | 31 +++++++++++++++++++++----------
 include/usb/gadget-multi.h | 15 ++++++++++++---
 3 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/commands/usbgadget.c b/commands/usbgadget.c
index 3b115f147d80..07094026db71 100644
--- a/commands/usbgadget.c
+++ b/commands/usbgadget.c
@@ -18,26 +18,25 @@
 
 static int do_usbgadget(int argc, char *argv[])
 {
+	struct usbgadget_funcs funcs = {};
 	int opt;
-	bool acm = false, dfu = false, fastboot = false, export_bbu = false;
-	const char *fastboot_opts = NULL, *dfu_opts = NULL;
 
 	while ((opt = getopt(argc, argv, "asdA::D::b")) > 0) {
 		switch (opt) {
 		case 'a':
 		case 's':
-			acm = true;
+			funcs.flags |= USBGADGET_ACM;
 			break;
 		case 'D':
-			dfu = true;
-			dfu_opts = optarg;
+			funcs.flags |= USBGADGET_DFU;
+			funcs.dfu_opts = optarg;
 			break;
 		case 'A':
-			fastboot = true;
-			fastboot_opts = optarg;
+			funcs.flags |= USBGADGET_FASTBOOT;
+			funcs.fastboot_opts = optarg;
 			break;
 		case 'b':
-			export_bbu = true;
+			funcs.flags |= USBGADGET_EXPORT_BBU;
 			break;
 		case 'd':
 			usb_multi_unregister();
@@ -47,8 +46,8 @@ static int do_usbgadget(int argc, char *argv[])
 		}
 	}
 
-	return usbgadget_register(dfu, dfu_opts, fastboot, fastboot_opts, acm,
-				  export_bbu);
+
+	return usbgadget_register(&funcs);
 }
 
 BAREBOX_CMD_HELP_START(usbgadget)
diff --git a/common/usbgadget.c b/common/usbgadget.c
index feec0b6634be..48e2ea9a349c 100644
--- a/common/usbgadget.c
+++ b/common/usbgadget.c
@@ -32,14 +32,20 @@ static struct file_list *parse(const char *files)
 	return list;
 }
 
-int usbgadget_register(bool dfu, const char *dfu_opts,
-		       bool fastboot, const char *fastboot_opts,
-		       bool acm, bool export_bbu)
+int usbgadget_register(const struct usbgadget_funcs *funcs)
 {
 	int ret;
+	int flags = funcs->flags;
 	struct device_d *dev;
 	struct f_multi_opts *opts;
 	const char *fastboot_partitions = get_fastboot_partitions();
+	const char *dfu_opts = funcs->dfu_opts;
+	const char *fastboot_opts = funcs->fastboot_opts;
+	bool dfu, fastboot, acm;
+
+	dfu = flags & USBGADGET_DFU;
+	fastboot = flags & USBGADGET_FASTBOOT;
+	acm = flags & USBGADGET_ACM;
 
 	if (dfu && !dfu_opts && dfu_function && *dfu_function)
 		dfu_opts = dfu_function;
@@ -66,7 +72,7 @@ int usbgadget_register(bool dfu, const char *dfu_opts,
 
 	if (fastboot_opts) {
 		opts->fastboot_opts.files = parse(fastboot_opts);
-		opts->fastboot_opts.export_bbu = export_bbu;
+		opts->fastboot_opts.export_bbu = flags & USBGADGET_EXPORT_BBU;
 	}
 
 	if (dfu_opts)
@@ -93,18 +99,23 @@ int usbgadget_register(bool dfu, const char *dfu_opts,
 
 static int usbgadget_autostart_set(struct param_d *param, void *ctx)
 {
+	struct usbgadget_funcs funcs = {};
 	static bool started;
-	bool fastboot_bbu = get_fastboot_bbu();
-	int err;
 
 	if (!IS_ENABLED(CONFIG_USB_GADGET_AUTOSTART) || !autostart || started)
 		return 0;
 
-	err = usbgadget_register(true, NULL, true, NULL, acm, fastboot_bbu);
-	if (!err)
-		started = true;
+	if (get_fastboot_bbu())
+		funcs.flags |= USBGADGET_EXPORT_BBU;
+
+	if (acm)
+		funcs.flags |= USBGADGET_ACM;
+
+	funcs.flags |= USBGADGET_DFU | USBGADGET_FASTBOOT;
+
+	started = 1;
 
-	return err;
+	return usbgadget_register(&funcs);
 }
 
 static int usbgadget_globalvars_init(void)
diff --git a/include/usb/gadget-multi.h b/include/usb/gadget-multi.h
index 9bb6c889f3e9..244bdd946f91 100644
--- a/include/usb/gadget-multi.h
+++ b/include/usb/gadget-multi.h
@@ -16,8 +16,17 @@ int usb_multi_register(struct f_multi_opts *opts);
 void usb_multi_unregister(void);
 void usb_multi_opts_release(struct f_multi_opts *opts);
 
-int usbgadget_register(bool dfu, const char *dfu_opts,
-		       bool fastboot, const char *fastboot_opts,
-		       bool acm, bool export_bbu);
+#define USBGADGET_EXPORT_BBU	(1 << 0)
+#define USBGADGET_ACM		(1 << 1)
+#define USBGADGET_DFU		(1 << 2)
+#define USBGADGET_FASTBOOT	(1 << 3)
+
+struct usbgadget_funcs {
+	int flags;
+	const char *fastboot_opts;
+	const char *dfu_opts;
+};
+
+int usbgadget_register(const struct usbgadget_funcs *funcs);
 
 #endif /* __USB_GADGET_MULTI_H */
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 12/12] usbgadget: multi: wire mass storage gadget into composite gadget
  2021-02-15 10:36 [PATCH 00/12] poller: run pollers as proper coroutines (green threads) Ahmad Fatoum
                   ` (10 preceding siblings ...)
  2021-02-15 10:37 ` [PATCH 11/12] usbgadget: refactor usbgadget_register to accept array Ahmad Fatoum
@ 2021-02-15 10:37 ` Ahmad Fatoum
  11 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2021-02-15 10:37 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

For configuration with CONFIG_POLLER_YIELD=y, we can call the blocking
loop in a poller and do other stuff in the "foreground". Add a new
usbgadget -S option for doing that. The old ums command still remains,
but is off by default for all platforms that can use usbgadget -S.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 Documentation/user/usb.rst          |  2 ++
 commands/Kconfig                    | 10 ++++++++
 commands/Makefile                   |  2 +-
 commands/usbgadget.c                | 10 ++++++--
 common/usbgadget.c                  | 28 ++++++++++++++++++----
 drivers/usb/gadget/Kconfig          | 12 +++++++++-
 drivers/usb/gadget/f_mass_storage.c |  2 +-
 drivers/usb/gadget/multi.c          | 36 +++++++++++++++++++++++++++++
 include/usb/gadget-multi.h          |  4 ++++
 9 files changed, 96 insertions(+), 10 deletions(-)

diff --git a/Documentation/user/usb.rst b/Documentation/user/usb.rst
index ca5f8138deda..55b2d7eaf13b 100644
--- a/Documentation/user/usb.rst
+++ b/Documentation/user/usb.rst
@@ -266,6 +266,8 @@ USB Gadget autostart Options
   See :ref:`command_usbgadget` -a. (Default 0).
 ``global.usbgadget.dfu_function``
   Function description for DFU. See :ref:`command_usbgadget` -D [desc].
+``global.usbgadget.ums_function``
+  Function description for USB mass storage. See :ref:`command_usbgadget` -S [desc].
 ``global.fastboot.partitions``
   Function description for fastboot. See :ref:`command_usbgadget` -A [desc].
 ``global.fastboot.bbu``
diff --git a/commands/Kconfig b/commands/Kconfig
index b672f0c16a85..281a6c30b57f 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -1981,6 +1981,16 @@ config CMD_USBGADGET
 	depends on USB_GADGET
 	prompt "usbgadget"
 
+config CMD_UMS
+	bool "blocking ums (usb mass storage) command" if USB_GADGET_MASS_STORAGE_MULTI
+	default y if !USB_GADGET_MASS_STORAGE_MULTI
+	depends on USB_GADGET
+	help
+	  The USB mass storage driver can't run in the background on all
+	  supported platforms. If you are on such a platform, say y here.
+	  Otherwise, use the command usbgadget to set it up as part of a
+	  composite gadget.
+
 config CMD_WD
 	bool
 	depends on WATCHDOG
diff --git a/commands/Makefile b/commands/Makefile
index a31d5c877703..7d0f69f834ba 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -130,6 +130,6 @@ obj-$(CONFIG_CMD_NAND_BITFLIP)	+= nand-bitflip.o
 obj-$(CONFIG_CMD_SEED)		+= seed.o
 obj-$(CONFIG_CMD_IP_ROUTE_GET)  += ip-route-get.o
 obj-$(CONFIG_CMD_UBSAN)		+= ubsan.o
-obj-$(CONFIG_USB_GADGET_MASS_STORAGE) += ums.o
+obj-$(CONFIG_CMD_UMS)		+= ums.o
 
 UBSAN_SANITIZE_ubsan.o := y
diff --git a/commands/usbgadget.c b/commands/usbgadget.c
index 07094026db71..03df3ecd717d 100644
--- a/commands/usbgadget.c
+++ b/commands/usbgadget.c
@@ -21,7 +21,7 @@ static int do_usbgadget(int argc, char *argv[])
 	struct usbgadget_funcs funcs = {};
 	int opt;
 
-	while ((opt = getopt(argc, argv, "asdA::D::b")) > 0) {
+	while ((opt = getopt(argc, argv, "asdA::D::S::b")) > 0) {
 		switch (opt) {
 		case 'a':
 		case 's':
@@ -35,6 +35,10 @@ static int do_usbgadget(int argc, char *argv[])
 			funcs.flags |= USBGADGET_FASTBOOT;
 			funcs.fastboot_opts = optarg;
 			break;
+		case 'S':
+			funcs.flags |= USBGADGET_MASS_STORAGE;
+			funcs.ums_opts = optarg;
+			break;
 		case 'b':
 			funcs.flags |= USBGADGET_EXPORT_BBU;
 			break;
@@ -60,13 +64,15 @@ BAREBOX_CMD_HELP_OPT ("-A <desc>", "Create Android Fastboot function. If 'desc'
 BAREBOX_CMD_HELP_OPT ("-b\t", "include registered barebox update handlers (fastboot specific)")
 BAREBOX_CMD_HELP_OPT ("-D <desc>", "Create DFU function. If 'desc' is not provided, "
 				   "try to use 'global.usbgadget.dfu_function' variable.")
+BAREBOX_CMD_HELP_OPT ("-S <desc>", "Create USB Mass Storage function. If 'desc' is not provided, "
+				   "try to use 'global.usbgadget.ums_function' variable.")
 BAREBOX_CMD_HELP_OPT ("-d\t", "Disable the currently running gadget")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(usbgadget)
 	.cmd		= do_usbgadget,
 	BAREBOX_CMD_DESC("Create USB Gadget multifunction device")
-	BAREBOX_CMD_OPTS("[-adAD]")
+	BAREBOX_CMD_OPTS("[-adADS]")
 	BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP)
 	BAREBOX_CMD_HELP(cmd_usbgadget_help)
 BAREBOX_CMD_END
diff --git a/common/usbgadget.c b/common/usbgadget.c
index 48e2ea9a349c..14e1f392300e 100644
--- a/common/usbgadget.c
+++ b/common/usbgadget.c
@@ -20,7 +20,7 @@
 
 static int autostart;
 static int acm;
-static char *dfu_function;
+static char *dfu_function, *ums_function;
 
 static struct file_list *parse(const char *files)
 {
@@ -41,11 +41,13 @@ int usbgadget_register(const struct usbgadget_funcs *funcs)
 	const char *fastboot_partitions = get_fastboot_partitions();
 	const char *dfu_opts = funcs->dfu_opts;
 	const char *fastboot_opts = funcs->fastboot_opts;
-	bool dfu, fastboot, acm;
+	const char *ums_opts = funcs->ums_opts;
+	bool dfu, fastboot, acm, ums;
 
 	dfu = flags & USBGADGET_DFU;
 	fastboot = flags & USBGADGET_FASTBOOT;
 	acm = flags & USBGADGET_ACM;
+	ums = flags & USBGADGET_MASS_STORAGE;
 
 	if (dfu && !dfu_opts && dfu_function && *dfu_function)
 		dfu_opts = dfu_function;
@@ -54,7 +56,10 @@ int usbgadget_register(const struct usbgadget_funcs *funcs)
 	    fastboot_partitions && *fastboot_partitions)
 		fastboot_opts = fastboot_partitions;
 
-	if (!dfu_opts && !fastboot_opts && !acm)
+	if (ums && !dfu_opts && ums_function && *ums_function)
+		ums_opts = ums_function;
+
+	if (!dfu_opts && !fastboot_opts && !ums_opts && !acm)
 		return COMMAND_ERROR_USAGE;
 
 	/*
@@ -67,6 +72,12 @@ int usbgadget_register(const struct usbgadget_funcs *funcs)
 		return -EINVAL;
 	}
 
+	if (ums_opts && !IS_ENABLED(CONFIG_USB_GADGET_MASS_STORAGE_MULTI)) {
+		pr_err("%s only supports blocking 'ums' command\n",
+		       IS_ENABLED(CONFIG_HAS_ARCH_SJLJ) ? "Configuration" : "Architecture");
+		return -ENOSYS;
+	}
+
 	opts = xzalloc(sizeof(*opts));
 	opts->release = usb_multi_opts_release;
 
@@ -78,7 +89,11 @@ int usbgadget_register(const struct usbgadget_funcs *funcs)
 	if (dfu_opts)
 		opts->dfu_opts.files = parse(dfu_opts);
 
-	if (!opts->dfu_opts.files && !opts->fastboot_opts.files && !acm) {
+	if (ums_opts)
+		opts->ums_opts.files = parse(ums_opts);
+
+	if (!opts->dfu_opts.files && !opts->fastboot_opts.files &&
+	    !opts->ums_opts.files && !acm) {
 		pr_warn("No functions to register\n");
 		free(opts);
 		return 0;
@@ -111,7 +126,7 @@ static int usbgadget_autostart_set(struct param_d *param, void *ctx)
 	if (acm)
 		funcs.flags |= USBGADGET_ACM;
 
-	funcs.flags |= USBGADGET_DFU | USBGADGET_FASTBOOT;
+	funcs.flags |= USBGADGET_DFU | USBGADGET_FASTBOOT | USBGADGET_MASS_STORAGE;
 
 	started = 1;
 
@@ -126,6 +141,7 @@ static int usbgadget_globalvars_init(void)
 		globalvar_add_simple_bool("usbgadget.acm", &acm);
 	}
 	globalvar_add_simple_string("usbgadget.dfu_function", &dfu_function);
+	globalvar_add_simple_string("usbgadget.ums_function", &ums_function);
 
 	return 0;
 }
@@ -137,3 +153,5 @@ BAREBOX_MAGICVAR(global.usbgadget.acm,
 		 "usbgadget: Create CDC ACM function");
 BAREBOX_MAGICVAR(global.usbgadget.dfu_function,
 		 "usbgadget: Create DFU function");
+BAREBOX_MAGICVAR(global.usbgadget.ums_function,
+		 "usbgadget: Create USB Mass Storage function");
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 90d2378b5b72..f0756667f110 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -39,7 +39,8 @@ config USB_GADGET_AUTOSTART
 	help
 	  Enabling this option allows to automatically start a dfu or
 	  fastboot gadget during boot. This behaviour is controlled with
-	  the global.usbgadget.dfu_function and global.fastboot.* variables.
+	  the global.usbgadget.dfu_function, global.usbgadget.ums_function
+	  and global.fastboot.* variables.
 
 comment "USB Gadget drivers"
 
@@ -70,4 +71,13 @@ config USB_GADGET_MASS_STORAGE
 	  device. Multiple storages can be specified at once on
 	  instantiation time.
 
+config USB_GADGET_MASS_STORAGE_MULTI
+	def_bool y
+	depends on USB_GADGET_MASS_STORAGE
+	depends on POLLER_YIELD
+	help
+	  This enables the USB Mass Storage gadget to run in the
+	  background, either on its own or as part of a multifunction
+	  composite gadget.
+
 endif
diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index c3e6eb933ce7..472f9cf3bea0 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -287,7 +287,7 @@ static struct usb_gadget_strings	fsg_stringtab = {
 
 /*-------------------------------------------------------------------------*/
 
-#ifdef CONFIG_POLLER_YIELD
+#ifdef CONFIG_USB_GADGET_MASS_STORAGE_MULTI
 
 #include <linux/completion.h>
 #include <linux/kthread.h>
diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
index 95f5b90c88b5..9189caf20f98 100644
--- a/drivers/usb/gadget/multi.c
+++ b/drivers/usb/gadget/multi.c
@@ -60,6 +60,8 @@ static struct usb_function_instance *fi_dfu;
 static struct usb_function *f_dfu;
 static struct usb_function_instance *fi_fastboot;
 static struct usb_function *f_fastboot;
+static struct usb_function_instance *fi_ums;
+static struct usb_function *f_ums;
 
 static struct usb_configuration config = {
 	.bConfigurationValue	= 1,
@@ -139,6 +141,31 @@ static int multi_bind_fastboot(struct usb_composite_dev *cdev)
 	return usb_add_function(&config, f_fastboot);
 }
 
+static int multi_bind_ums(struct usb_composite_dev *cdev)
+{
+	int ret;
+	struct f_ums_opts *opts;
+
+	fi_ums = usb_get_function_instance("ums");
+	if (IS_ERR(fi_ums)) {
+		ret = PTR_ERR(fi_ums);
+		fi_ums = NULL;
+		return ret;
+	}
+
+	opts = container_of(fi_ums, struct f_ums_opts, func_inst);
+	opts->files = gadget_multi_opts->ums_opts.files;
+
+	f_ums = usb_get_function(fi_ums);
+	if (IS_ERR(f_ums)) {
+		ret = PTR_ERR(f_ums);
+		f_ums = NULL;
+		return ret;
+	}
+
+	return usb_add_function(&config, f_ums);
+}
+
 static int multi_unbind(struct usb_composite_dev *cdev)
 {
 	if (gadget_multi_opts->create_acm) {
@@ -205,6 +232,13 @@ static int multi_bind(struct usb_composite_dev *cdev)
 			goto out;
 	}
 
+	if (gadget_multi_opts->ums_opts.files) {
+		printf("%s: creating USB Mass Storage function\n", __func__);
+		ret = multi_bind_ums(cdev);
+		if (ret)
+			goto out;
+	}
+
 	if (gadget_multi_opts->create_acm) {
 		printf("%s: creating ACM function\n", __func__);
 		ret = multi_bind_acm(cdev);
@@ -272,6 +306,8 @@ void usb_multi_opts_release(struct f_multi_opts *opts)
 		file_list_free(opts->fastboot_opts.files);
 	if (opts->dfu_opts.files)
 		file_list_free(opts->dfu_opts.files);
+	if (opts->ums_opts.files)
+		file_list_free(opts->ums_opts.files);
 
 	free(opts);
 }
diff --git a/include/usb/gadget-multi.h b/include/usb/gadget-multi.h
index 244bdd946f91..e733bbbbc366 100644
--- a/include/usb/gadget-multi.h
+++ b/include/usb/gadget-multi.h
@@ -4,10 +4,12 @@
 #include <usb/fastboot.h>
 #include <usb/dfu.h>
 #include <usb/usbserial.h>
+#include <usb/mass_storage.h>
 
 struct f_multi_opts {
 	struct fastboot_opts fastboot_opts;
 	struct f_dfu_opts dfu_opts;
+	struct f_ums_opts ums_opts;
 	int create_acm;
 	void (*release)(struct f_multi_opts *opts);
 };
@@ -20,11 +22,13 @@ void usb_multi_opts_release(struct f_multi_opts *opts);
 #define USBGADGET_ACM		(1 << 1)
 #define USBGADGET_DFU		(1 << 2)
 #define USBGADGET_FASTBOOT	(1 << 3)
+#define USBGADGET_MASS_STORAGE	(1 << 4)
 
 struct usbgadget_funcs {
 	int flags;
 	const char *fastboot_opts;
 	const char *dfu_opts;
+	const char *ums_opts;
 };
 
 int usbgadget_register(const struct usbgadget_funcs *funcs);
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 09/12] include: add kthread wrappers for pollers
  2021-02-15 10:37 ` [PATCH 09/12] include: add kthread wrappers for pollers Ahmad Fatoum
@ 2021-02-15 12:31   ` Sascha Hauer
  0 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2021-02-15 12:31 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Mon, Feb 15, 2021 at 11:37:02AM +0100, Ahmad Fatoum wrote:
> With the new fancy yielding poller support, we can properly represent
> kernel threads in barebox. Do so.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  include/linux/kthread.h | 72 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
>  create mode 100644 include/linux/kthread.h
> 
> diff --git a/include/linux/kthread.h b/include/linux/kthread.h
> new file mode 100644
> index 000000000000..17b6de9cf168
> --- /dev/null
> +++ b/include/linux/kthread.h
> @@ -0,0 +1,72 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_KTHREAD_H
> +#define _LINUX_KTHREAD_H
> +/* Wrapper around pollers to ease porting from Linux */
> +
> +#include <linux/kernel.h>
> +#include <linux/err.h>
> +#include <linux/string.h>
> +#include <poller.h>
> +
> +struct task_struct {
> +	struct poller_struct poller;
> +	int (*threadfn)(void *data);
> +	void *data;
> +};
> +
> +static inline void kthread_poller(struct poller_struct *poller)
> +{
> +	struct task_struct *task = container_of(poller, struct task_struct, poller);
> +	task->threadfn(task->data);
> +}

kthreads behave differently. When you return from a kthread then the
thread is stopped. We should either behave the same way or use a
different name.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-02-15 12:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-15 10:36 [PATCH 00/12] poller: run pollers as proper coroutines (green threads) Ahmad Fatoum
2021-02-15 10:36 ` [PATCH 01/12] common: add coroutine support Ahmad Fatoum
2021-02-15 10:36 ` [PATCH 02/12] poller: run pollers as proper coroutines if architecture supports it Ahmad Fatoum
2021-02-15 10:36 ` [PATCH 03/12] ARM: asm: setjmp: annotate setjmp/longjmp for GCC Ahmad Fatoum
2021-02-15 10:36 ` [PATCH 04/12] ARM: asm: setjmp: implement coroutine dependency initjmp() Ahmad Fatoum
2021-02-15 10:36 ` [PATCH 05/12] sandbox: asm: implement setjmp/longjmp/initjmp Ahmad Fatoum
2021-02-15 10:36 ` [PATCH 06/12] poller: command: add new coroutine check Ahmad Fatoum
2021-02-15 10:37 ` [PATCH 07/12] slice: have assert_command_context() yield until true if possible Ahmad Fatoum
2021-02-15 10:37 ` [PATCH 08/12] poller: implement basic Linux-like completion API Ahmad Fatoum
2021-02-15 10:37 ` [PATCH 09/12] include: add kthread wrappers for pollers Ahmad Fatoum
2021-02-15 12:31   ` Sascha Hauer
2021-02-15 10:37 ` [PATCH 10/12] usbgadget: ums: run gadget loop in a background coroutine if possible Ahmad Fatoum
2021-02-15 10:37 ` [PATCH 11/12] usbgadget: refactor usbgadget_register to accept array Ahmad Fatoum
2021-02-15 10:37 ` [PATCH 12/12] usbgadget: multi: wire mass storage gadget into composite gadget Ahmad Fatoum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox