Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [RFC patch 14/19] bpf: Use migrate_disable() in hashtab code | Date | Wed, 19 Feb 2020 01:49:57 +0100 |
| |
Alexei,
Alexei Starovoitov <alexei.starovoitov@gmail.com> writes: > Overall looks great. > Thank you for taking time to write commit logs and detailed cover letter. > I think s/__this_cpu_inc/this_cpu_inc/ is the only bit that needs to be > addressed for it to be merged. > There were few other suggestions from Mathieu and Jakub. > Could you address them and resend?
I have them fixed up already, but I was waiting for further comments. I'll send it out tomorrow morning as I'm dead tired by now.
> I saw patch 1 landing in tip tree, but it needs to be in bpf-next as well > along with the rest of the series. Does it really need to be in the tip? > I would prefer to take the whole thing and avoid conflicts around > migrate_disable() especially if nothing in tip is going to use it in this > development cycle. So just drop patch 1 from the tip?
I'll add patch 2 to a tip branch as well and I'll give you a tag to pull into BPF (which has only those two commits). That allows us to further tweak the relevant files without creating conflicts in next.
> Regarding > union { > raw_spinlock_t raw_lock; > spinlock_t lock; > }; > yeah. it's not pretty, but I also don't have better ideas.
Yeah. I really tried hard to avoid it, but the alternative solution was code duplication which was even more horrible.
> Regarding migrate_disable()... can you enable it without the rest of RT? > I haven't seen its implementation. I suspect it's scheduler only change? > If I can use migrate_disable() without RT it will help my work on sleepable > BPF programs. I would only have to worry about rcu_read_lock() since > preempt_disable() is nicely addressed.
You have to talk to Peter Zijlstra about this as this is really scheduler relevant stuff. FYI, he undamentaly hates migrate_disable() from a schedulabilty POV, but as with the above lock construct the amount of better solutions is also close to zero.
Thanks,
tglx
| |