On Thu, May 14, 2020 at 08:21:52PM +0200, Daniel Glöckner wrote:
> Some code can't foresee which resources will be used by its poller. This
> is the case especially in pollers that will execute arbitrary commands
> input by the user. With this commit a special slice is introduced that is
> released only when Barebox is waiting for console input and only when it
> is doing that outside of any command. Code that wants to depend on the
> idle slice must select CONFIG_IDLE_SLICE to make it available.
>
> Signed-off-by: Daniel Glöckner <dg@xxxxxxxxx>
> ---
> common/Kconfig | 4 ++++
> common/binfmt.c | 3 +++
> common/command.c | 3 +++
> common/console_countdown.c | 3 +++
> common/poller.c | 2 ++
> common/slice.c | 24 ++++++++++++++++++++++++
> include/slice.h | 10 ++++++++++
> lib/readline.c | 8 +++++++-
> 8 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/common/Kconfig b/common/Kconfig
> index 400c0553c..2fa9140de 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -913,6 +913,10 @@ config BAREBOXCRC32_TARGET
> config POLLER
> bool "generic polling infrastructure"
>
> +config IDLE_SLICE
> + bool
> + default n
We discussed config SLICE away, is there a reason to introduce config
IDLE_SLICE?
> +
> config STATE
> bool "generic state infrastructure"
> select CRC32
> diff --git a/common/binfmt.c b/common/binfmt.c
> index f2ff62458..899f270d4 100644
> --- a/common/binfmt.c
> +++ b/common/binfmt.c
> @@ -10,6 +10,7 @@
> #include <malloc.h>
> #include <command.h>
> #include <errno.h>
> +#include <slice.h>
>
> static LIST_HEAD(binfmt_hooks);
>
> @@ -23,7 +24,9 @@ static int binfmt_run(char *file, int argc, char **argv)
> if (b->type != type)
> continue;
>
> + idle_slice_acquire();
> ret = b->hook(b, file, argc, argv);
> + idle_slice_release();
> if (ret != -ERESTARTNOHAND)
> return ret;
> }
> diff --git a/common/command.c b/common/command.c
> index 49845938a..2b10248f5 100644
> --- a/common/command.c
> +++ b/common/command.c
> @@ -30,6 +30,7 @@
> #include <init.h>
> #include <complete.h>
> #include <getopt.h>
> +#include <slice.h>
>
> LIST_HEAD(command_list);
> EXPORT_SYMBOL(command_list);
> @@ -72,7 +73,9 @@ int execute_command(int argc, char **argv)
> /* Look up command in command table */
> if ((cmdtp = find_cmd(argv[0]))) {
> /* OK - call function to do the command */
> + idle_slice_acquire();
> ret = cmdtp->cmd(argc, argv);
> + idle_slice_release();
The command could be a shell in which case we would never be able to
release the idle slice as long this shell is running.
> @@ -200,11 +201,16 @@ int readline(const char *prompt, char *buf, int len)
> puts (prompt);
>
> while (1) {
> + idle_slice_release();
> while (!tstc()) {
> poller_call();
> - if (IS_ENABLED(CONFIG_CONSOLE_RATP))
> + if (IS_ENABLED(CONFIG_CONSOLE_RATP)) {
> + idle_slice_acquire();
> barebox_ratp_command_run();
> + idle_slice_release();
> + }
> }
> + idle_slice_acquire();
I am not sure this is the right place to release the idle slice, I would
rather do it in the caller. readline() may be called from different
places, not all of them seem to be appropriate for releasing the idle
slice.
I think the idle slice should be released right before the shell calls
into readline and acquired again right afterwards.
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@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
