[bpf-next,v2,0/4] Use this_cpu_xxx for preemption-safety

Message

Hou Tao Sept. 1, 2022, 6:19 a.m. UTC

From: Hou Tao <[e-mail]>

Hi,

The patchset aims to make the update of per-cpu prog->active and per-cpu
bpf_task_storage_busy being preemption-safe. The problem is on same
architectures (e.g. arm64), __this_cpu_{inc|dec|inc_return} are neither
preemption-safe nor IRQ-safe, so under fully preemptible kernel the
concurrent updates on these per-cpu variables may be interleaved and the
final values of these variables may be not zero.

Patch 1 & 2 use the preemption-safe per-cpu helpers to manipulate
prog->active and bpf_task_storage_busy. Patch 3 & 4 add a test case in
map_tests to show the concurrent updates on the per-cpu
bpf_task_storage_busy by using __this_cpu_{inc|dec} are not atomic.

Comments are always welcome.

Regards,
Tao

Change Log:
v2:
* Patch 1: update commit message to indicate the problem is only
  possible for fully preemptible kernel
* Patch 2: a new patch which fixes the problem for prog->active
* Patch 3 & 4: move it to test_maps and make it depend on CONFIG_PREEMPT
 
v1: https://lore.kernel.org/bpf/[e-mail]/

Hou Tao (4):
  bpf: Use this_cpu_{inc|dec|inc_return} for bpf_task_storage_busy
  bpf: Use this_cpu_{inc_return|dec} for prog->active
  selftests/bpf: Move sys_pidfd_open() into task_local_storage_helpers.h
  selftests/bpf: Test concurrent updates on bpf_task_storage_busy

 kernel/bpf/bpf_local_storage.c                |   4 +-
 kernel/bpf/bpf_task_storage.c                 |   8 +-
 kernel/bpf/trampoline.c                       |   8 +-
 .../bpf/map_tests/task_storage_map.c          | 122 ++++++++++++++++++
 .../selftests/bpf/prog_tests/test_bprm_opts.c |  10 +-
 .../bpf/prog_tests/test_local_storage.c       |  10 +-
 .../bpf/progs/read_bpf_task_storage_busy.c    |  39 ++++++
 .../bpf/task_local_storage_helpers.h          |  18 +++
 8 files changed, 191 insertions(+), 28 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/map_tests/task_storage_map.c
 create mode 100644 tools/testing/selftests/bpf/progs/read_bpf_task_storage_busy.c
 create mode 100644 tools/testing/selftests/bpf/task_local_storage_helpers.h

Comments

[e-mail] Sept. 1, 2022, 7:30 p.m. UTC | #1

Hello:

This series was applied to bpf/bpf-next.git (master)
by Martin KaFai Lau <[e-mail]>:

On Thu,  1 Sep 2022 14:19:34 +0800 you wrote:
> From: Hou Tao <[e-mail]>
> 
> Hi,
> 
> The patchset aims to make the update of per-cpu prog->active and per-cpu
> bpf_task_storage_busy being preemption-safe. The problem is on same
> architectures (e.g. arm64), __this_cpu_{inc|dec|inc_return} are neither
> preemption-safe nor IRQ-safe, so under fully preemptible kernel the
> concurrent updates on these per-cpu variables may be interleaved and the
> final values of these variables may be not zero.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2,1/4] bpf: Use this_cpu_{inc|dec|inc_return} for bpf_task_storage_busy
    https://git.kernel.org/bpf/bpf-next/c/197827a05e13
  - [bpf-next,v2,2/4] bpf: Use this_cpu_{inc_return|dec} for prog->active
    https://git.kernel.org/bpf/bpf-next/c/c89e843a11f1
  - [bpf-next,v2,3/4] selftests/bpf: Move sys_pidfd_open() into task_local_storage_helpers.h
    https://git.kernel.org/bpf/bpf-next/c/c710136e8774
  - [bpf-next,v2,4/4] selftests/bpf: Test concurrent updates on bpf_task_storage_busy
    https://git.kernel.org/bpf/bpf-next/c/73b97bc78b32

You are awesome, thank you!