From: Thomas Gleixner <tglx@linutronix.de>
To: Alex Belits <abelits@marvell.com>,
"nitesh\@redhat.com" <nitesh@redhat.com>,
"frederic\@kernel.org" <frederic@kernel.org>
Cc: Prasun Kapoor <pkapoor@marvell.com>,
"linux-api\@vger.kernel.org" <linux-api@vger.kernel.org>,
"davem\@davemloft.net" <davem@davemloft.net>,
"trix\@redhat.com" <trix@redhat.com>,
"mingo\@kernel.org" <mingo@kernel.org>,
"catalin.marinas\@arm.com" <catalin.marinas@arm.com>,
"rostedt\@goodmis.org" <rostedt@goodmis.org>,
"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"peterx\@redhat.com" <peterx@redhat.com>,
"linux-arch\@vger.kernel.org" <linux-arch@vger.kernel.org>,
"mtosatti\@redhat.com" <mtosatti@redhat.com>,
"will\@kernel.org" <will@kernel.org>,
"peterz\@infradead.org" <peterz@infradead.org>,
"leon\@sidebranch.com" <leon@sidebranch.com>,
"linux-arm-kernel\@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"pauld\@redhat.com" <pauld@redhat.com>,
"netdev\@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH v5 4/9] task_isolation: Add task isolation hooks to arch-independent code
Date: Mon, 23 Nov 2020 23:31:18 +0100 [thread overview]
Message-ID: <875z5vn1s9.fsf@nanos.tec.linutronix.de> (raw)
In-Reply-To: <ec4bacce635fed4e77ab46752d41432f270edf83.camel@marvell.com>
Alex,
On Mon, Nov 23 2020 at 17:57, Alex Belits wrote:
> Kernel entry and exit functions for task isolation are added to context
> tracking and common entry points. Common handling of pending work on exit
> to userspace now processes isolation breaking, cleanup and start.
Again: You fail to explain the rationale and just explain what the patch
is doing. I can see what the patch is doing from the patch itself.
> ---
> include/linux/hardirq.h | 2 ++
> include/linux/sched.h | 2 ++
> kernel/context_tracking.c | 5 +++++
> kernel/entry/common.c | 10 +++++++++-
> kernel/irq/irqdesc.c | 5 +++++
At least 3 different subsystems, which means this again failed to be
split into seperate patches.
> extern void synchronize_irq(unsigned int irq);
> @@ -115,6 +116,7 @@ extern void rcu_nmi_exit(void);
> do { \
> lockdep_off(); \
> arch_nmi_enter(); \
> + task_isolation_kernel_enter(); \
Where is the explanation why this is safe and correct vs. this fragile
code path?
> @@ -1762,6 +1763,7 @@ extern char *__get_task_comm(char *to, size_t len, struct task_struct *tsk);
> #ifdef CONFIG_SMP
> static __always_inline void scheduler_ipi(void)
> {
> + task_isolation_kernel_enter();
Why is the scheduler_ipi() special? Just because everything else cannot
happen at all? Oh well...
> #define CREATE_TRACE_POINTS
> #include <trace/events/context_tracking.h>
> @@ -100,6 +101,8 @@ void noinstr __context_tracking_enter(enum ctx_state state)
> __this_cpu_write(context_tracking.state, state);
> }
> context_tracking_recursion_exit();
> +
> + task_isolation_exit_to_user_mode();
Why is this here at all and why is it outside of the recursion
protection
> }
> EXPORT_SYMBOL_GPL(__context_tracking_enter);
>
> @@ -148,6 +151,8 @@ void noinstr __context_tracking_exit(enum ctx_state state)
> if (!context_tracking_recursion_enter())
> return;
>
> + task_isolation_kernel_enter();
while this is inside?
And why has the scheduler_ipi() on x86 call this twice? Just because?
> if (__this_cpu_read(context_tracking.state) == state) {
> if (__this_cpu_read(context_tracking.active)) {
> /*
> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
> static void exit_to_user_mode_prepare(struct pt_regs *regs)
> {
> - unsigned long ti_work = READ_ONCE(current_thread_info()->flags);
> + unsigned long ti_work;
>
> lockdep_assert_irqs_disabled();
>
> + task_isolation_before_pending_work_check();
> +
> + ti_work = READ_ONCE(current_thread_info()->flags);
> +
> if (unlikely(ti_work & EXIT_TO_USER_MODE_WORK))
> ti_work = exit_to_user_mode_loop(regs, ti_work);
>
> + if (unlikely(ti_work & _TIF_TASK_ISOLATION))
> + task_isolation_start();
Where is the explaination of this change?
Aside of that how does anything of this compile on x86 at all?
Answer: It does not ...
Stop this frenzy right now. It's going nowhere and all you achieve is to
make people more grumpy than they are already.
Thanks,
tglx
next prev parent reply other threads:[~2020-11-23 22:31 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-23 17:42 [PATCH v5 0/9] "Task_isolation" mode Alex Belits
2020-11-23 17:56 ` [PATCH v5 1/9] task_isolation: vmstat: add quiet_vmstat_sync function Alex Belits
2020-11-23 21:48 ` Thomas Gleixner
2020-11-23 17:56 ` [PATCH v5 2/9] task_isolation: vmstat: add vmstat_idle function Alex Belits
2020-11-23 21:49 ` Thomas Gleixner
2020-11-23 17:56 ` [PATCH v5 3/9] task_isolation: userspace hard isolation from kernel Alex Belits
2020-11-23 22:01 ` Thomas Gleixner
2020-11-23 17:57 ` [PATCH v5 4/9] task_isolation: Add task isolation hooks to arch-independent code Alex Belits
2020-11-23 22:31 ` Thomas Gleixner [this message]
2020-11-23 17:57 ` [PATCH v5 5/9] task_isolation: Add driver-specific hooks Alex Belits
2020-12-02 14:18 ` Mark Rutland
2020-12-04 0:43 ` [EXT] " Alex Belits
2020-11-23 17:58 ` [PATCH v5 6/9] task_isolation: arch/arm64: enable task isolation functionality Alex Belits
2020-12-02 13:59 ` Mark Rutland
2020-12-04 0:37 ` [EXT] " Alex Belits
2020-12-07 11:57 ` Mark Rutland
2020-11-23 17:58 ` [PATCH v5 7/9] task_isolation: don't interrupt CPUs with tick_nohz_full_kick_cpu() Alex Belits
2020-11-23 22:13 ` Frederic Weisbecker
2020-11-23 22:35 ` Alex Belits
2020-11-23 22:36 ` Thomas Gleixner
2020-12-02 14:20 ` Mark Rutland
2020-12-04 0:54 ` [EXT] " Alex Belits
2020-12-07 11:58 ` Mark Rutland
2020-11-23 17:58 ` [PATCH v5 8/9] task_isolation: ringbuffer: don't interrupt CPUs running isolated tasks on buffer resize Alex Belits
2020-11-23 17:58 ` [PATCH v5 9/9] task_isolation: kick_all_cpus_sync: don't kick isolated cpus Alex Belits
2020-11-23 22:29 ` Frederic Weisbecker
2020-11-23 22:39 ` [EXT] " Alex Belits
2020-11-23 23:21 ` Frederic Weisbecker
2020-11-25 3:20 ` Alex Belits
2021-01-22 15:00 ` Marcelo Tosatti
2020-11-24 16:36 ` [PATCH v5 0/9] "Task_isolation" mode Tom Rix
2020-11-24 17:40 ` [EXT] " Alex Belits
2020-12-02 14:02 ` Mark Rutland
2020-12-04 0:39 ` Alex Belits
2020-12-05 20:40 ` Pavel Machek
2020-12-05 23:25 ` Thomas Gleixner
2020-12-11 18:08 ` Yury Norov
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=875z5vn1s9.fsf@nanos.tec.linutronix.de \
--to=tglx@linutronix.de \
--cc=abelits@marvell.com \
--cc=catalin.marinas@arm.com \
--cc=davem@davemloft.net \
--cc=frederic@kernel.org \
--cc=leon@sidebranch.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=mtosatti@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=nitesh@redhat.com \
--cc=pauld@redhat.com \
--cc=peterx@redhat.com \
--cc=peterz@infradead.org \
--cc=pkapoor@marvell.com \
--cc=rostedt@goodmis.org \
--cc=trix@redhat.com \
--cc=will@kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).