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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <69cbe2bd-3d39-b3ae-2ebc-6399125fc782@mellanox.com>
Date:   Tue, 30 Aug 2016 11:41:36 -0400
From:   Chris Metcalf <cmetcalf@...lanox.com>
To:     Frederic Weisbecker <fweisbec@...il.com>
CC:     Christoph Lameter <cl@...ux.com>,
        Gilad Ben Yossef <giladb@...lanox.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...nel.org>,
        "Peter Zijlstra" <peterz@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Rik van Riel <riel@...hat.com>, Tejun Heo <tj@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Andy Lutomirski <luto@...capital.net>,
        Michal Hocko <mhocko@...e.com>, <linux-mm@...ck.org>,
        <linux-doc@...r.kernel.org>, <linux-api@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v14 04/14] task_isolation: add initial support

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