Re: [PATCH] bpf: fix rq lock recursion issue

[Yonghong Song <yhs@fb.com>]

On 6/13/22 6:10 PM, Satya Durga Srinivasu Prabhala wrote:
> 
> On 6/13/22 2:49 PM, Alexei Starovoitov wrote:
>> On Mon, Jun 13, 2022 at 2:35 PM Satya Durga Srinivasu Prabhala
>> <quic_satyap@quicinc.com> wrote:
>>>
>>> On 6/13/22 2:01 PM, Alexei Starovoitov wrote:
>>>> is doesn't solve anything.
>>>> Please provide a reproducer.
>>> I'm trying to find an easy way to repro the issue, so far, unsuccessful.
>>>
>>>> iirc the task's affinity change can race even with preemption disabled
>>>> on this cpu. Why would s/migrate/preemption/ address the deadlock ?
>>> I don't think task's affinity change races with preemption 
>>> disabled/enabled.
>>>
>>> Switching to preemption disable/enable calls helps as it's just simple
>>> counter increment and decrement with a barrier, but with migrate
>>> disable/enable when task's affinity changes, we run into recursive bug
>>> due to rq lock.
>> As Yonghong already explained, replacing migrate_disable
>> with preempt_disable around bpf prog invocation is not an option.
> 
> If I understand correctly, Yonghong mentioned that replacing migrate_
> with preempt_ won't work for RT Kernels and migrate_ APIs were introduced
> for RT Kernels is what he was pointing to. I went back and cross checked
> on 5.10 LTS Kernel, I see that the migrate_ calls end up just calling into
> preemt_ calls [1]. So far non-RT kernels, sticking to preemt_ calls should
> still work. Please let me know if I missed anything.

Yes, old kernel migrate_disable/enable() implementation with
simply preempt_disable/enable() are transitional. You can check
5.12 kernel migrate_disable/enable() implementation. Note that
your patch, if accepted, will apply to the latest kernel. So we
cannot simply replace migrate_disable() with prempt_disable(),
which won't work for RT kernel.

> 
> [1]
> https://android.googlesource.com/kernel/common/+/refs/heads/android12-5.10/include/linux/preempt.h#335 
>