Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Andy Lutomirski <luto@amacapital.net>,
        Peter Zijlstra <peterz@infradead.org>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        Masami Hiramatsu <mhiramat@kernel.org>
Subject: Re: [patch 4/5] sched: Delay task stack freeing on RT
In-Reply-To: <CALCETrVP3asoqWyNqEe+rDs+YECd9gnp9eFb1==X0140oMZ41g@mail.gmail.com>
References: <20210928122339.502270600@linutronix.de>
 <20210928122411.593486363@linutronix.de>
 <YVRT6QbX5zwiIJkI@hirez.programming.kicks-ass.net>
 <CALCETrVP3asoqWyNqEe+rDs+YECd9gnp9eFb1==X0140oMZ41g@mail.gmail.com>
Date:   Fri, 01 Oct 2021 19:24:45 +0200
Message-ID: <87o8884q02.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Fri, Oct 01 2021 at 09:12, Andy Lutomirski wrote:
> On Wed, Sep 29, 2021 at 4:54 AM Peter Zijlstra <peterz@infradead.org> wrote:
>> Having this logic split across two files seems unfortunate and prone to
>> 'accidents'. Is there a real down-side to unconditionally doing it in
>> delayed_put_task_struct() ?
>>
>> /me goes out for lunch... meanwhile tglx points at: 68f24b08ee89.
>>
>> Bah.. Andy?
>
> Could we make whatever we do here unconditional?

Sure. I just was unsure about your reasoning in 68f24b08ee89.

> And what actually causes the latency?  If it's vfree, shouldn't the
> existing use of vfree_atomic() in free_thread_stack() handle it?  Or
> is it the accounting?

The accounting muck because it can go into the allocator and sleep in
the worst case, which is nasty even on !RT kernels.

But thinking some more, there is actually a way nastier issue on RT in
the following case:

CPU 0                           CPU 1
  T1                            
  spin_lock(L1)
  rt_mutex_lock()
      schedule()

  T2
     do_exit()
     do_task_dead()             spin_unlock(L1)
                                   wake(T1)
     __schedule()                           
       switch_to(T1)
       finish_task_switch()
         put_task_stack()
           account()
             ....
             spin_lock(L2)

So if L1 == L2 or L1 and L2 have a reverse dependency then this can just
deadlock.

We've never observed that, but the above case is obviously hard to
hit. Nevertheless it's there.

Thanks,

        tglx