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;
Date:   Thu, 06 May 2021 00:11:30 -0700 (PDT)
Subject:     Re: [PATCH] RISC-V: insn: Use a raw spinlock to protect TEXT_POKE*
In-Reply-To: <mhng-dbd1fd8c-37f7-4b60-a61e-3f8d22e5baf0@palmerdabbelt-glaptop>
CC:     Anup Patel <Anup.Patel@wdc.com>, changbin.du@gmail.com,
        linux-riscv@lists.infradead.org,
        Paul Walmsley <paul.walmsley@sifive.com>,
        aou@eecs.berkeley.edu, peterz@infradead.org, jpoimboe@redhat.com,
        jbaron@akamai.com, ardb@kernel.org,
        Atish Patra <Atish.Patra@wdc.com>, akpm@linux-foundation.org,
        rppt@kernel.org, mhiramat@kernel.org, zong.li@sifive.com,
        guoren@linux.alibaba.com, wangkefeng.wang@huawei.com,
        0x7f454c46@gmail.com, chenhuang5@huawei.com,
        linux-kernel@vger.kernel.org, kernel-team@android.com
From:   Palmer Dabbelt <palmerdabbelt@google.com>
To:     rostedt@goodmis.org
Message-ID: <mhng-08581a54-f9c8-4119-97bb-a9be86f19e41@palmerdabbelt-glaptop>
Mime-Version: 1.0 (MHng)
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Fri, 30 Apr 2021 12:44:37 PDT (-0700), Palmer Dabbelt wrote:
> On Fri, 30 Apr 2021 04:34:31 PDT (-0700), rostedt@goodmis.org wrote:
>> On Fri, 30 Apr 2021 04:06:35 +0000
>> Anup Patel <Anup.Patel@wdc.com> wrote:
>>
>>> This patch only takes care of ftrace path.
>>>
>>> The RISC-V instruction patching is used by RISC-V jump label implementation
>>> as well and will called from various critical parts of core kernel.
>>>
>>> The RAW spinlock approach allows same instruction patching to be used
>>> for kprobes, ftrace, and jump label.
>>
>> So what path hits this outside of stop machine?
>
> I didn't actually dig through all the usages of jump_label, I just saw a
> handful in places where it's generally not sane to assume that sleeping
> is safe -- for example, thoughout kernel/sched.  If you think it's OK to
> rely on users of the static branch stuff (IIUC the only jump_label user
> in the kernel?) to know that it can sleep then I'm fine keeping the
> text_mutex call in jump_label and adding one to ftrace (I'm fine with
> something generic, but it's simple to do in arch/riscv).
>
> IMO if the static branch stuff can be expected to sleep it'd be good to
> call that out in the documentation, and I'd like to audit the uses
> before committing to that.  I'm happy to do that, we can just take the
> lock in arch/riscv's frace code for now to get around the lockdep
> assertion failure -- IIUC that's indicating a real bug, as nothing in
> ftrace avoids concurrency with jump_label and kprobes.

Turns out I'd actually opened the wrong thread and was looking at a 
stack trace from a bur reported a year ago, which is why nothing I was 
saying here makes any sense.  That bug has already been fixed, I have a 
proper fix for this one.  It turns out to be almost exactly the same as 
something Steven suggested in this thread.