Re: [PATCH v14 04/14] task_isolation: add initial support

[Chris Metcalf]

On 8/29/2016 8:55 PM, Frederic Weisbecker wrote:
> On Mon, Aug 15, 2016 at 10:59:55AM -0400, Chris Metcalf wrote:
> > On 8/11/2016 2:50 PM, Christoph Lameter wrote:
> > > On Thu, 11 Aug 2016, Frederic Weisbecker wrote:
> > >
> > > > Do we need to quiesce vmstat everytime before entering userspace?
> > > > I thought that vmstat only need to be offlined once and for all?
> > > Once is sufficient after disabling the tick.
> > It's true that task_isolation_enter() is called every time before
> > returning to user space while task isolation is enabled.
> >
> > But once we enter the kernel again after returning from the initial
> > prctl() -- assuming we are in NOSIG mode so doing so is legal in the
> > first place -- almost anything can happen, certainly including
> > restarting the tick.  Thus, we have to make sure that normal quiescing
> > happens again before we return to userspace.
> Yes but we need to sort out what needs to be called only once on prctl().
>
> Once vmstat is quiesced, it's not going to need quiescing again even if we
> restart the tick.

That's true, but I really do like the idea of having a clean structure
where we verify all our prerequisites in task_isolation_ready(), and
have code to try to get things fixed up in task_isolation_enter().
If we start moving some things here and some things there, it gets
harder to manage.  I think by testing "!vmstat_idle()" in
task_isolation_enter() we are avoiding any substantial overhead.

I think it would be clearer to rename task_isolation_enter()
to task_isolation_prepare(); it might be less confusing.

Remember too that in general, we really don't need to think about
return-to-userspace as a hot path for task isolation, unlike how we
think about it all the rest of the time.  So it makes sense to
prioritize keeping things clean from a software development
perspective first, and high-performance only secondarily.

> > The thing to remember is that this is only relevant if the user has
> > explicitly requested the NOSIG behavior from task isolation, which we
> > don't really expect to be the default - we are implicitly encouraging
> > use of the default semantics of "you can't enter the kernel again
> > until you turn off isolation".
> That's right. Although NOSIG is the only thing we can afford as long as
> we drag around the 1Hz.

True enough.  Hopefully we'll finish sorting that out soon enough.

> > > > +	if (!tick_nohz_tick_stopped())
> > > > +		set_tsk_need_resched(current);
> > > > Again, that won't help
> > It won't be better than spinning in a loop if there aren't any other
> > schedulable processes, but it won't be worse either.  If there is
> > another schedulable process, we at least will schedule it sooner than
> > if we just sat in a busy loop and waited for the scheduler to kick
> > us. But there's nothing else we can do anyway if we want to maintain
> > the guarantee that the dyn tick is stopped before return to userspace.
> I don't think it helps either way. If reschedule is pending, the current
> task already has TIF_RESCHED set.

See the other thread with Peter Z for the longer discussion of this.
At this point I'm leaning towards replacing the set_tsk_need_resched() with

    set_current_state(TASK_INTERRUPTIBLE);
    schedule();
    __set_current_state(TASK_RUNNING);

--
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com