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

[Thomas Gleixner]

Alexei,

Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> 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