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

[Chris Metcalf <cmetcalf@mellanox.com>]

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