Re: [RFC patch 14/19] bpf: Use migrate_disable() in hashtab code

From: Alexei Starovoitov
Date: Tue Feb 18 2020 - 18:36:52 EST


On Fri, Feb 14, 2020 at 08:56:12PM +0100, Thomas Gleixner wrote:
>
> > Also, I don't think using __this_cpu_inc() without preempt-disable or
> > irq off is safe. You'll probably want to move to this_cpu_inc/dec
> > instead, which can be heavier on some architectures.
>
> Good catch.

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 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?

Regarding
union {
raw_spinlock_t raw_lock;
spinlock_t lock;
};
yeah. it's not pretty, but I also don't have better ideas.

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.