• Marco Elver's avatar
    bpf: Allow compiler to inline most of bpf_local_storage_lookup() · 68bc61c2
    Marco Elver authored
    In various performance profiles of kernels with BPF programs attached,
    bpf_local_storage_lookup() appears as a significant portion of CPU
    cycles spent. To enable the compiler generate more optimal code, turn
    bpf_local_storage_lookup() into a static inline function, where only the
    cache insertion code path is outlined
    
    Notably, outlining cache insertion helps avoid bloating callers by
    duplicating setting up calls to raw_spin_{lock,unlock}_irqsave() (on
    architectures which do not inline spin_lock/unlock, such as x86), which
    would cause the compiler produce worse code by deciding to outline
    otherwise inlinable functions. The call overhead is neutral, because we
    make 2 calls either way: either calling raw_spin_lock_irqsave() and
    raw_spin_unlock_irqsave(); or call __bpf_local_storage_insert_cache(),
    which calls raw_spin_lock_irqsave(), followed by a tail-call to
    raw_spin_unlock_irqsave() where the compiler can perform TCO and (in
    optimized uninstrumented builds) turns it into a plain jump. The call to
    __bpf_local_storage_insert_cache() can be elided entirely if
    cacheit_lockit is a false constant expression.
    
    Based on results from './benchs/run_bench_local_storage.sh' (21 trials,
    reboot between each trial; x86 defconfig + BPF, clang 16) this produces
    improvements in throughput and latency in the majority of cases, with an
    average (geomean) improvement of 8%:
    
    +---- Hashmap Control --------------------
    |
    | + num keys: 10
    | :                                         <before>             | <after>
    | +-+ hashmap (control) sequential get    +----------------------+----------------------
    |   +- hits throughput                    | 14.789 M ops/s       | 14.745 M ops/s (  ~  )
    |   +- hits latency                       | 67.679 ns/op         | 67.879 ns/op   (  ~  )
    |   +- important_hits throughput          | 14.789 M ops/s       | 14.745 M ops/s (  ~  )
    |
    | + num keys: 1000
    | :                                         <before>             | <after>
    | +-+ hashmap (control) sequential get    +----------------------+----------------------
    |   +- hits throughput                    | 12.233 M ops/s       | 12.170 M ops/s (  ~  )
    |   +- hits latency                       | 81.754 ns/op         | 82.185 ns/op   (  ~  )
    |   +- important_hits throughput          | 12.233 M ops/s       | 12.170 M ops/s (  ~  )
    |
    | + num keys: 10000
    | :                                         <before>             | <after>
    | +-+ hashmap (control) sequential get    +----------------------+----------------------
    |   +- hits throughput                    | 7.220 M ops/s        | 7.204 M ops/s  (  ~  )
    |   +- hits latency                       | 138.522 ns/op        | 138.842 ns/op  (  ~  )
    |   +- important_hits throughput          | 7.220 M ops/s        | 7.204 M ops/s  (  ~  )
    |
    | + num keys: 100000
    | :                                         <before>             | <after>
    | +-+ hashmap (control) sequential get    +----------------------+----------------------
    |   +- hits throughput                    | 5.061 M ops/s        | 5.165 M ops/s  (+2.1%)
    |   +- hits latency                       | 198.483 ns/op        | 194.270 ns/op  (-2.1%)
    |   +- important_hits throughput          | 5.061 M ops/s        | 5.165 M ops/s  (+2.1%)
    |
    | + num keys: 4194304
    | :                                         <before>             | <after>
    | +-+ hashmap (control) sequential get    +----------------------+----------------------
    |   +- hits throughput                    | 2.864 M ops/s        | 2.882 M ops/s  (  ~  )
    |   +- hits latency                       | 365.220 ns/op        | 361.418 ns/op  (-1.0%)
    |   +- important_hits throughput          | 2.864 M ops/s        | 2.882 M ops/s  (  ~  )
    |
    +---- Local Storage ----------------------
    |
    | + num_maps: 1
    | :                                         <before>             | <after>
    | +-+ local_storage cache sequential get  +----------------------+----------------------
    |   +- hits throughput                    | 33.005 M ops/s       | 39.068 M ops/s (+18.4%)
    |   +- hits latency                       | 30.300 ns/op         | 25.598 ns/op   (-15.5%)
    |   +- important_hits throughput          | 33.005 M ops/s       | 39.068 M ops/s (+18.4%)
    | :
    | :                                         <before>             | <after>
    | +-+ local_storage cache interleaved get +----------------------+----------------------
    |   +- hits throughput                    | 37.151 M ops/s       | 44.926 M ops/s (+20.9%)
    |   +- hits latency                       | 26.919 ns/op         | 22.259 ns/op   (-17.3%)
    |   +- important_hits throughput          | 37.151 M ops/s       | 44.926 M ops/s (+20.9%)
    |
    | + num_maps: 10
    | :                                         <before>             | <after>
    | +-+ local_storage cache sequential get  +----------------------+----------------------
    |   +- hits throughput                    | 32.288 M ops/s       | 38.099 M ops/s (+18.0%)
    |   +- hits latency                       | 30.972 ns/op         | 26.248 ns/op   (-15.3%)
    |   +- important_hits throughput          | 3.229 M ops/s        | 3.810 M ops/s  (+18.0%)
    | :
    | :                                         <before>             | <after>
    | +-+ local_storage cache interleaved get +----------------------+----------------------
    |   +- hits throughput                    | 34.473 M ops/s       | 41.145 M ops/s (+19.4%)
    |   +- hits latency                       | 29.010 ns/op         | 24.307 ns/op   (-16.2%)
    |   +- important_hits throughput          | 12.312 M ops/s       | 14.695 M ops/s (+19.4%)
    |
    | + num_maps: 16
    | :                                         <before>             | <after>
    | +-+ local_storage cache sequential get  +----------------------+----------------------
    |   +- hits throughput                    | 32.524 M ops/s       | 38.341 M ops/s (+17.9%)
    |   +- hits latency                       | 30.748 ns/op         | 26.083 ns/op   (-15.2%)
    |   +- important_hits throughput          | 2.033 M ops/s        | 2.396 M ops/s  (+17.9%)
    | :
    | :                                         <before>             | <after>
    | +-+ local_storage cache interleaved get +----------------------+----------------------
    |   +- hits throughput                    | 34.575 M ops/s       | 41.338 M ops/s (+19.6%)
    |   +- hits latency                       | 28.925 ns/op         | 24.193 ns/op   (-16.4%)
    |   +- important_hits throughput          | 11.001 M ops/s       | 13.153 M ops/s (+19.6%)
    |
    | + num_maps: 17
    | :                                         <before>             | <after>
    | +-+ local_storage cache sequential get  +----------------------+----------------------
    |   +- hits throughput                    | 28.861 M ops/s       | 32.756 M ops/s (+13.5%)
    |   +- hits latency                       | 34.649 ns/op         | 30.530 ns/op   (-11.9%)
    |   +- important_hits throughput          | 1.700 M ops/s        | 1.929 M ops/s  (+13.5%)
    | :
    | :                                         <before>             | <after>
    | +-+ local_storage cache interleaved get +----------------------+----------------------
    |   +- hits throughput                    | 31.529 M ops/s       | 36.110 M ops/s (+14.5%)
    |   +- hits latency                       | 31.719 ns/op         | 27.697 ns/op   (-12.7%)
    |   +- important_hits throughput          | 9.598 M ops/s        | 10.993 M ops/s (+14.5%)
    |
    | + num_maps: 24
    | :                                         <before>             | <after>
    | +-+ local_storage cache sequential get  +----------------------+----------------------
    |   +- hits throughput                    | 18.602 M ops/s       | 19.937 M ops/s (+7.2%)
    |   +- hits latency                       | 53.767 ns/op         | 50.166 ns/op   (-6.7%)
    |   +- important_hits throughput          | 0.776 M ops/s        | 0.831 M ops/s  (+7.2%)
    | :
    | :                                         <before>             | <after>
    | +-+ local_storage cache interleaved get +----------------------+----------------------
    |   +- hits throughput                    | 21.718 M ops/s       | 23.332 M ops/s (+7.4%)
    |   +- hits latency                       | 46.047 ns/op         | 42.865 ns/op   (-6.9%)
    |   +- important_hits throughput          | 6.110 M ops/s        | 6.564 M ops/s  (+7.4%)
    |
    | + num_maps: 32
    | :                                         <before>             | <after>
    | +-+ local_storage cache sequential get  +----------------------+----------------------
    |   +- hits throughput                    | 14.118 M ops/s       | 14.626 M ops/s (+3.6%)
    |   +- hits latency                       | 70.856 ns/op         | 68.381 ns/op   (-3.5%)
    |   +- important_hits throughput          | 0.442 M ops/s        | 0.458 M ops/s  (+3.6%)
    | :
    | :                                         <before>             | <after>
    | +-+ local_storage cache interleaved get +----------------------+----------------------
    |   +- hits throughput                    | 17.111 M ops/s       | 17.906 M ops/s (+4.6%)
    |   +- hits latency                       | 58.451 ns/op         | 55.865 ns/op   (-4.4%)
    |   +- important_hits throughput          | 4.776 M ops/s        | 4.998 M ops/s  (+4.6%)
    |
    | + num_maps: 100
    | :                                         <before>             | <after>
    | +-+ local_storage cache sequential get  +----------------------+----------------------
    |   +- hits throughput                    | 5.281 M ops/s        | 5.528 M ops/s  (+4.7%)
    |   +- hits latency                       | 192.398 ns/op        | 183.059 ns/op  (-4.9%)
    |   +- important_hits throughput          | 0.053 M ops/s        | 0.055 M ops/s  (+4.9%)
    | :
    | :                                         <before>             | <after>
    | +-+ local_storage cache interleaved get +----------------------+----------------------
    |   +- hits throughput                    | 6.265 M ops/s        | 6.498 M ops/s  (+3.7%)
    |   +- hits latency                       | 161.436 ns/op        | 152.877 ns/op  (-5.3%)
    |   +- important_hits throughput          | 1.636 M ops/s        | 1.697 M ops/s  (+3.7%)
    |
    | + num_maps: 1000
    | :                                         <before>             | <after>
    | +-+ local_storage cache sequential get  +----------------------+----------------------
    |   +- hits throughput                    | 0.355 M ops/s        | 0.354 M ops/s  (  ~  )
    |   +- hits latency                       | 2826.538 ns/op       | 2827.139 ns/op (  ~  )
    |   +- important_hits throughput          | 0.000 M ops/s        | 0.000 M ops/s  (  ~  )
    | :
    | :                                         <before>             | <after>
    | +-+ local_storage cache interleaved get +----------------------+----------------------
    |   +- hits throughput                    | 0.404 M ops/s        | 0.403 M ops/s  (  ~  )
    |   +- hits latency                       | 2481.190 ns/op       | 2487.555 ns/op (  ~  )
    |   +- important_hits throughput          | 0.102 M ops/s        | 0.101 M ops/s  (  ~  )
    
    The on_lookup test in {cgrp,task}_ls_recursion.c is removed
    because the bpf_local_storage_lookup is no longer traceable
    and adding tracepoint will make the compiler generate worse
    code: https://lore.kernel.org/bpf/ZcJmok64Xqv6l4ZS@elver.google.com/Signed-off-by: default avatarMarco Elver <elver@google.com>
    Cc: Martin KaFai Lau <martin.lau@linux.dev>
    Acked-by: default avatarYonghong Song <yonghong.song@linux.dev>
    Link: https://lore.kernel.org/r/20240207122626.3508658-1-elver@google.comSigned-off-by: default avatarMartin KaFai Lau <martin.lau@kernel.org>
    68bc61c2
bpf_local_storage.h 6.45 KB