mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: barebox@lists.infradead.org
Cc: Jan Luebbe <j.luebbe@pengutronix.de>
Subject: Re: [PATCH v4 4/8] common: bthread: schedule only in command context
Date: Tue, 22 Jun 2021 11:08:28 +0200	[thread overview]
Message-ID: <7d45e6d7-0649-7b4b-3744-ee01e3f34bea@pengutronix.de> (raw)
In-Reply-To: <20210622082617.18011-5-a.fatoum@pengutronix.de>

On 22.06.21 10:26, Ahmad Fatoum wrote:
> Originally, I envisioned bthreads as replacement for pollers and
> workqueues. But even without preemption, having functions you call
> possibly accessing structures you are iterating over can corrupt
> memory. This problem exists with pollers as well, but because of their
> limited scope, it's harder to shoot your foot with them, as you
> don't keep implicit state between poller activations unlike bthreads,
> which maintain their stack across context switches.
> 
> Limit bthread scope instead to be a replacement for workqueues. This
> still allows us to port some classes of state-machine-in-kthread kernel
> code, while avoding the aforementioned pitfalls.
> 
> Cc: Jan Luebbe <j.luebbe@pengutronix.de>
> Cc: Sascha Hauer <a.hauer@pengutronix.de>

Mailer daemon just told me the first character of Sascha is not an a.

Please fixup on apply (or I do with v5)

> Cc: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  commands/bthread.c | 5 +++++
>  common/sched.c     | 6 +++---
>  include/slice.h    | 2 --
>  3 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/commands/bthread.c b/commands/bthread.c
> index 964a1044c58b..0c9c221884e8 100644
> --- a/commands/bthread.c
> +++ b/commands/bthread.c
> @@ -10,12 +10,15 @@
>  #include <command.h>
>  #include <getopt.h>
>  #include <clock.h>
> +#include <slice.h>
>  
>  static int bthread_time(void)
>  {
>  	uint64_t start = get_time_ns();
>  	int i = 0;
>  
> +	slice_release(&command_slice);
> +
>  	/*
>  	 * How many background tasks can we have in one second?
>  	 *
> @@ -25,6 +28,8 @@ static int bthread_time(void)
>  	while (!is_timeout(start, SECOND))
>  		i++;
>  
> +	slice_acquire(&command_slice);
> +
>  	return i;
>  }
>  
> diff --git a/common/sched.c b/common/sched.c
> index dcf6522ac058..02582b2c5e4d 100644
> --- a/common/sched.c
> +++ b/common/sched.c
> @@ -15,12 +15,12 @@ void resched(void)
>  
>  	command_slice_acquire();
>  
> -	if (run_workqueues)
> +	if (run_workqueues) {
>  		wq_do_all_works();
> +		bthread_reschedule();
> +	}
>  
>  	poller_call();
>  
>  	command_slice_release();
> -
> -	bthread_reschedule();
>  }
> diff --git a/include/slice.h b/include/slice.h
> index 67d47b9082e6..6c4688e308fa 100644
> --- a/include/slice.h
> +++ b/include/slice.h
> @@ -38,8 +38,6 @@ void command_slice_release(void);
>  
>  #define assert_command_context() do { \
>  	WARN_ONCE(poller_active(), "%s called in poller\n", __func__); \
> -	WARN_ONCE(IS_ENABLED(CONFIG_BTHREAD) && !bthread_is_main(current), \
> -		  "%s called in secondary bthread\n", __func__); \
>  } while (0)
>  
>  #endif
> 

-- 
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


  reply	other threads:[~2021-06-22  9:09 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-22  8:26 [PATCH v4 0/8] usbgadget: add support for USB mass storage gadget Ahmad Fatoum
2021-06-22  8:26 ` [PATCH v4 1/8] input: virtio: poll from poller, not bthread Ahmad Fatoum
2021-06-22  8:26 ` [PATCH v4 2/8] bthread: add debug print for scheduler context switches Ahmad Fatoum
2021-06-22  8:26 ` [PATCH v4 3/8] common: move workqueue handling from poller_call() to sched() Ahmad Fatoum
2021-06-22  8:26 ` [PATCH v4 4/8] common: bthread: schedule only in command context Ahmad Fatoum
2021-06-22  9:08   ` Ahmad Fatoum [this message]
2021-06-22  8:26 ` [PATCH v4 5/8] bthread: implement basic Linux-like completion API Ahmad Fatoum
2021-06-22  8:26 ` [PATCH v4 6/8] Documentation: devel: background-execution: update bthread docs Ahmad Fatoum
2021-06-22  8:26 ` [PATCH v4 7/8] usbgadget: refactor usbgadget_register to accept array Ahmad Fatoum
2021-06-22  8:26 ` [PATCH v4 8/8] usbgadget: add support for USB mass storage gadget Ahmad Fatoum
2021-06-25  7:34 ` [PATCH v4 0/8] " Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7d45e6d7-0649-7b4b-3744-ee01e3f34bea@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=j.luebbe@pengutronix.de \
    --subject='Re: [PATCH v4 4/8] common: bthread: schedule only in command context' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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