This avoids processing of TIF_TASK_ISOL, when returning to userspace,
for tasks which do not have task isolation configured.
Signed-off-by: Marcelo Tosatti <[email protected]>
Index: linux-2.6/include/linux/task_isolation.h
===================================================================
--- linux-2.6.orig/include/linux/task_isolation.h
+++ linux-2.6/include/linux/task_isolation.h
@@ -5,6 +5,8 @@
#ifdef CONFIG_TASK_ISOLATION
+#include <uapi/linux/prctl.h>
+
struct task_isol_info {
/* Which features have been configured */
u64 conf_mask;
@@ -51,6 +53,24 @@ int __copy_task_isol(struct task_struct
void task_isol_exit_to_user_mode(void);
+static inline bool task_isol_quiesce_activated(struct task_struct *tsk,
+ u64 quiesce_mask)
+{
+ struct task_isol_info *i;
+
+ i = tsk->task_isol_info;
+ if (!i)
+ return false;
+
+ if (i->active_mask != ISOL_F_QUIESCE)
+ return false;
+
+ if ((i->quiesce_mask & quiesce_mask) == quiesce_mask)
+ return true;
+
+ return false;
+}
+
#else
static inline void task_isol_exit_to_user_mode(void)
@@ -105,6 +125,12 @@ static inline int prctl_task_isol_activa
return -EOPNOTSUPP;
}
+static inline bool task_isol_quiesce_activated(struct task_struct *tsk,
+ u64 quiesce_mask)
+{
+ return false;
+}
+
#endif /* CONFIG_TASK_ISOLATION */
#endif /* __LINUX_TASK_ISOL_H */
Index: linux-2.6/mm/vmstat.c
===================================================================
--- linux-2.6.orig/mm/vmstat.c
+++ linux-2.6/mm/vmstat.c
@@ -28,6 +28,7 @@
#include <linux/mm_inline.h>
#include <linux/page_ext.h>
#include <linux/page_owner.h>
+#include <linux/task_isolation.h>
#include "internal.h"
@@ -344,7 +345,8 @@ static inline void mark_vmstat_dirty(voi
return;
raw_cpu_write(vmstat_dirty, true);
- set_thread_flag(TIF_TASK_ISOL);
+ if (task_isol_quiesce_activated(current, ISOL_F_QUIESCE_VMSTATS))
+ set_thread_flag(TIF_TASK_ISOL);
}
void init_sync_vmstat(void)
On Tue, Mar 15 2022 at 12:31, Marcelo Tosatti wrote:
$Subject does not qualify as a parseable sentence.
> This avoids processing of TIF_TASK_ISOL, when returning to userspace,
> for tasks which do not have task isolation configured.
That's how kernel development works, right:
1) Add half baken stuff
2) Apply duct tape on top
You know exactly, that this is _not_ the way it works.
This whole thing is half thought out tinkerware with [ill|un]defined
semantics.
Thanks,
tglx
On Wed, Apr 27, 2022 at 09:45:54AM +0200, Thomas Gleixner wrote:
> On Tue, Mar 15 2022 at 12:31, Marcelo Tosatti wrote:
>
> $Subject does not qualify as a parseable sentence.
>
> > This avoids processing of TIF_TASK_ISOL, when returning to userspace,
> > for tasks which do not have task isolation configured.
>
> That's how kernel development works, right:
>
> 1) Add half baken stuff
> 2) Apply duct tape on top
>
> You know exactly, that this is _not_ the way it works.
>
> This whole thing is half thought out tinkerware with [ill|un]defined
> semantics.
It seems to be inline with the remaining TIF_ bits:
if (ti_work & _TIF_NOTIFY_RESUME)
tracehook_notify_resume(regs);
+ if (ti_work & _TIF_TASK_ISOL)
+ task_isol_exit_to_user_mode();
+
And there is even:
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -83,6 +83,7 @@ struct thread_info {
#define TIF_NEED_RESCHED 3 /* rescheduling necessary */
#define TIF_SINGLESTEP 4 /* reenable singlestep on user return*/
#define TIF_SSBD 5 /* Speculative store bypass disable */
+#define TIF_TASK_ISOL 6 /* task isolation work pending */
#define TIF_SPEC_IB 9 /* Indirect branch speculation mitigation */
#define TIF_SPEC_L1D_FLUSH 10 /* Flush L1D on mm switches (processes) */
#define TIF_USER_RETURN_NOTIFY 11 /* notify kernel of userspace return */
So the purpose of TIF_TASK_ISOL is to condense in a single bit the
question: "is there task isolation work pending?"
By looking at the code, we see the sites where this bit is set are:
1) Task isolation configuration.
@@ -251,6 +257,11 @@ static int cfg_feat_quiesce_set(unsigned
info->quiesce_mask = i_qctrl->quiesce_mask;
info->oneshot_mask = i_qctrl->quiesce_oneshot_mask;
info->conf_mask |= ISOL_F_QUIESCE;
+
+ if ((info->active_mask & ISOL_F_QUIESCE) &&
+ (info->quiesce_mask & ISOL_F_QUIESCE_VMSTATS))
+ set_thread_flag(TIF_TASK_ISOL);
+
ret = 0;
2) copy_process (clone / fork):
@@ -303,6 +314,7 @@ int __copy_task_isol(struct task_struct
new_info->active_mask = info->active_mask;
tsk->task_isol_info = new_info;
+ set_ti_thread_flag(task_thread_info(tsk), TIF_TASK_ISOL);
return 0;
}
3) task isolation activation:
@@ -330,6 +342,10 @@ int prctl_task_isol_activate_set(unsigne
info->active_mask = active_mask;
ret = 0;
+ if ((info->active_mask & ISOL_F_QUIESCE) &&
+ (info->quiesce_mask & ISOL_F_QUIESCE_VMSTATS))
+ set_thread_flag(TIF_TASK_ISOL);
+
out:
return ret;
Would you prefer an explanation, in words, when these bits are set, when
they are cleared?
So the meaning is:
+#define TIF_TASK_ISOL 6 /* task isolation work pending */
And the definition is "task isolation work" depends on what task
isolation features are implemented.
(honestly, seem pretty clear to me, but willing to improve...).
On Tue, May 03 2022 at 16:12, Marcelo Tosatti wrote:
> On Wed, Apr 27, 2022 at 09:45:54AM +0200, Thomas Gleixner wrote:
> It seems to be inline with the remaining TIF_ bits:
>
> if (ti_work & _TIF_NOTIFY_RESUME)
> tracehook_notify_resume(regs);
>
> + if (ti_work & _TIF_TASK_ISOL)
> + task_isol_exit_to_user_mode();
> +
>
>
> And there is even:
I know that the bit is defined, but that does still not make an argument.
> By looking at the code, we see the sites where this bit is set are:
Again. I'm able to read the patches myself.
> Would you prefer an explanation, in words, when these bits are set, when
> they are cleared?
No. The point is that contrary to TIF_NOTIFY_RESUME and other TIF bits,
this is going to end up being sprinkled all over the place.
With the current vmstat quiesce that's limited, but it's bound to
increase and spread simply because the whole thing has no semantics and
it's all headed to be adhoc cure for the isolation itch of the day.
Thanks,
tglx