Re: [PATCH 07/19] Introduce idle slice

From: "Daniel Glöckner" <dg@emlix.com>
To: Sascha Hauer <s.hauer@pengutronix.de>,
	Barebox List <barebox@lists.infradead.org>
Cc: Edmund Henniges <eh@emlix.com>
Subject: Re: [PATCH 07/19] Introduce idle slice
Date: Fri, 22 May 2020 13:56:33 +0200	[thread overview]
Message-ID: <901a205b-1e1e-bd23-0e35-88d9b9d88351@emlix.com> (raw)
In-Reply-To: <20200520094403.12651-8-s.hauer@pengutronix.de>

Hello Sascha,

Am 20.05.20 um 11:43 schrieb Sascha Hauer:
> diff --git a/common/console_countdown.c b/common/console_countdown.c
> index 74dc382795..c69029dc0e 100644
> --- a/common/console_countdown.c
> +++ b/common/console_countdown.c
> @@ -19,6 +19,7 @@
>  #include <errno.h>
>  #include <console_countdown.h>
>  #include <stdio.h>
> +#include <slice.h>
>  
>  static bool console_countdown_timeout_abort;
>  

with your changes we no longer need that include in here.

> diff --git a/common/hush.c b/common/hush.c
> index c24b2c7cd2..61424d98e7 100644
> --- a/common/hush.c
> +++ b/common/hush.c
> @@ -460,7 +460,12 @@ static void get_user_input(struct in_str *i)
>  	else
>  		prompt = CONFIG_PROMPT_HUSH_PS2;
>  
> +	idle_slice_release();
> +
>  	n = readline(prompt, console_buffer, CONFIG_CBSIZE);
> +
> +	idle_slice_acquire();
> +
>  	if (n == -1 ) {
>  		i->interrupt = 1;
>  		n = 0;

Ok, you moved the release/acquire from readline and console_countdown to
get_user_input and do_autoboot_countdown. That is actually more strict
than what I had in mind and avoids having to acquire/release in binfmt_run
and execute_command.

But I'm not so happy about the removal of the acquire/release in poller.c.
Now the fastboot poller might be run from within another poller and neither
do we know which resources that other poller uses nor do we know which
resources the commands need that we are told over to execute in our poller.

> @@ -369,6 +372,8 @@ static void ratp_poller(struct poller_struct *poller)
>  
>  	free(buf);
>  
> +	barebox_ratp_command_run();
> +
>  	return;
>  
>  out:

Ok, you reused the existing RATP poller instead of adding a second one.
Should work.

> diff --git a/lib/Kconfig b/lib/Kconfig
> index b4a8079700..053ac15cfc 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -111,6 +111,7 @@ config RATP
>  	select COMPILE_MEMORY
>  	select COMMAND_SUPPORT
>  	select POLLER
> +	select IDLE_SLICE
>  	depends on CONSOLE_FULL
>  	depends on !SHELL_NONE
>  	bool "RATP protocol support"

This has no effect. You removed CONFIG_IDLE_SLICE.

> diff --git a/lib/readline.c b/lib/readline.c
> index 3d16c1838c..cafdc58b1b 100644
> --- a/lib/readline.c
> +++ b/lib/readline.c
> @@ -3,7 +3,7 @@
>  #include <init.h>
>  #include <libbb.h>
>  #include <poller.h>
> -#include <ratp_bb.h>
> +#include <slice.h>
>  #include <xfuncs.h>
>  #include <complete.h>
>  #include <linux/ctype.h>

Again, with your changes no include for slice.h needed.

Best regards,

  Daniel

-- 
Dipl.-Math. Daniel Glöckner, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11,
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke
Ust-IdNr.: DE 205 198 055

emlix - your embedded linux partner

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