On Sun, Oct 20, 2019 at 11:06:13AM +0200, Thomas Gleixner wrote: > On Fri, 18 Oct 2019, Alexei Starovoitov wrote: > > On Fri, Oct 18, 2019 at 01:28:21PM +0200, Thomas Gleixner wrote: > > The concept on local_lock() makes sense to me. > > The magic macro you're proposing that will convert it to old school > > preempt_disable() on !RT should hopefully make the changes across > > net and bpf land mostly mechanical. > > One thing to clarify: > > when networking core interacts with bpf we know that bh doesn't migrate, > > so per-cpu datastructres that bpf side populates are accessed later > > by networking core when program finishes. > > Similar thing happens between tracing bits and bpf progs. > > It feels to me that local_lock() approach should work here as well. > > napi processing bits will grab it. Later bpf will grab potentially > > the same lock again. > > The weird bit that such lock will have numbe_of_lockers >= 1 > > for long periods of time. At least until napi runners won't see > > any more incoming packets. I'm not sure yet where such local_lock > > will be placed in the napi code (may be in drivers too for xdp). > > Does this make sense from RT perspective? > > I don't see why the lock would have more than one locker. The code in BPF > does > > preempt_disable(); > some operation > preempt_enable(); > > So how should that gain more than one context per CPU locking it? napi is doing preempt_disable() then calls into driver's poll function which further calls into bpf and when its over it looks at per-cpu data structures that bpf side shares with drivers. Even without bpf the networking processing takes long time. What is the plan for RT there? Are you planning to replace napi's preempt_disable with local_lock() too? > Also you might have to accept that there is a world outside of BPF and that > the 'some out of tree code' which we are talking about is the last part of > a 15+ years effort which significantly helped to bring the Linux kernel > into the shape it is today. No doubt about good stuff that came out of that effort and will continue to come. I'm arguing that 'pragmatic approach' to disable existing kernel features to get the rest of RT upstream will backfire on RT in the first place. ftrace disables preemption too. Lots of things do. imo it's better to get key RT bits (like local_locks) in without sending contentious patches like the one that sparked this thread. When everyone can see what this local_lock is we can figure out how and when to use it.
