All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <frederic@kernel.org>
To: Marcelo Tosatti <mtosatti@redhat.com>
Cc: linux-kernel@vger.kernel.org, Nitesh Lal <nilal@redhat.com>,
	Nicolas Saenz Julienne <nsaenzju@redhat.com>,
	Christoph Lameter <cl@linux.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Alex Belits <abelits@belits.com>, Peter Xu <peterx@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Daniel Bristot de Oliveira <bristot@redhat.com>
Subject: Re: [patch v5 2/8] add prctl task isolation prctl docs and samples
Date: Thu, 28 Oct 2021 16:53:43 +0200	[thread overview]
Message-ID: <20211028145343.GB77014@lothringen> (raw)
In-Reply-To: <20211027175247.GA296917@fuller.cnet>

On Wed, Oct 27, 2021 at 02:52:47PM -0300, Marcelo Tosatti wrote:
> On Wed, Oct 27, 2021 at 02:38:06PM +0200, Frederic Weisbecker wrote:
> > > +        The 'pmask' argument specifies the location of an 8 byte mask
> > > +        containing which features should be activated. Features whose
> > > +        bits are cleared will be deactivated. The possible
> > > +        bits for this mask are:
> > > +
> > > +                - ``ISOL_F_QUIESCE``:
> > > +
> > > +                Activate quiescing of background kernel activities.
> > > +                Quiescing happens on return to userspace from this
> > > +                system call, and on return from subsequent
> > > +                system calls (unless quiesce_oneshot_mask is configured,
> > > +                see below).
> > > +
> > > +        If the arg3 argument is non-zero, it specifies a pointer to::
> > > +
> > > +         struct task_isol_activate_control {
> > > +                 __u64 flags;
> > > +                 __u64 quiesce_oneshot_mask;
> > 
> > So you are using an entire argument here to set a single feature (ISOL_F_QUIESCE).
> 
> Yes, but there is room at "struct task_isol_activate_control" for other features 
> to use (and additional space in the remaining prctl arguments, if necessary).

Ok but we have a configuration syscall and an activation syscall. Why bothering
with config parts on activation syscall?


> 
> > It looks like the oneshot VS every syscall behaviour should be defined at
> > configuration time for individual ISOL_F_QUIESCE features.
> 
> It seems one-shot selection is dependent on the 
> application logic:
> 
> 	configure task isolation
> 	enable oneshot quiescing of kernel activities
> 	do {
> 		process data (no system calls)
> 		if (event) {
> 			process event with syscalls
> 			enable oneshot quiescing of kernel activities
> 		}
>        } while (!exit_condition);
> 
> Considering configuration performed outside the application (by chisol),
> is the administrator supposed to know the internals of the application
> at this level ?

If the launcher doesn't know about details, just leave them to the isolated
app. I mean we have a syscall to get the configured features, it's easy to
modify their configuration and set the oneshot mode on the place wanted
by the isolated app.

> 
> What if the application desires to use one-shot in a section
> (of code) and "all syscalls" for another section.

Doesn't sound like a problem.

> 
> > Also do we want that to always apply to all syscalls? Should we expect corner
> > cases with some of them? 
> 
> What type of corner cases do you think of?

I don't trust my imagination enough to display all possible user workloads.

> 
> > What about exceptions and interrupts?
> 
> Should move the isolation_exit_to_user_mode_prepare call from
> __syscall_exit_to_user_mode_work to exit_to_user_mode_prepare.
> Good point.
> 
> About your question. Think so, because otherwise: 
> 
>      enable oneshot quiescing of kernel activities
>      do {
>              process data (no system calls)	    <--- 1. IRQ/exception
>              if (event) {
>                      process event with syscalls
>                      enable oneshot quiescing of kernel activities
>              }
>      } while (exit_condition == false);
> 
> 
> If either an interrupt or exception occurs at point 1 above, userspace
> might not be notified, and the interrupt/exception handler might 
> change state in the kernel which makes the current CPU a target
> for IPIs, for example changing per-CPU vm statistics.

Ok but please leave configuration space to modify that in the future just in case.

Thanks.

  reply	other threads:[~2021-10-28 14:53 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-19 15:24 [patch v5 0/8] extensible prctl task isolation interface and vmstat sync Marcelo Tosatti
2021-10-19 15:24 ` [patch v5 1/8] add basic task isolation prctl interface Marcelo Tosatti
2021-10-19 15:24 ` [patch v5 2/8] add prctl task isolation prctl docs and samples Marcelo Tosatti
2021-10-27 12:38   ` Frederic Weisbecker
2021-10-27 17:52     ` Marcelo Tosatti
2021-10-28 14:53       ` Frederic Weisbecker [this message]
2021-10-19 15:24 ` [patch v5 3/8] task isolation: sync vmstats on return to userspace Marcelo Tosatti
2021-10-19 15:24 ` [patch v5 4/8] procfs: add per-pid task isolation state Marcelo Tosatti
2021-10-19 15:24 ` [patch v5 5/8] task isolation: sync vmstats conditional on changes Marcelo Tosatti
2021-10-19 15:24 ` [patch v5 6/8] KVM: x86: call isolation prepare from VM-entry code path Marcelo Tosatti
2021-10-19 15:24 ` [patch v5 7/8] mm: vmstat: move need_update Marcelo Tosatti
2021-10-19 15:24 ` [patch v5 8/8] mm: vmstat_refresh: avoid queueing work item if cpu stats are clean Marcelo Tosatti

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=20211028145343.GB77014@lothringen \
    --to=frederic@kernel.org \
    --cc=abelits@belits.com \
    --cc=bristot@redhat.com \
    --cc=cl@linux.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    --cc=nilal@redhat.com \
    --cc=nsaenzju@redhat.com \
    --cc=peterx@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.