Commit 20a286c1 authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'transfer-rcu-lock-state-across-subprog-calls' 

Kumar Kartikeya Dwivedi says:

====================
Transfer RCU lock state across subprog calls

David suggested during the discussion in [0] that we should handle RCU
locks in a similar fashion to spin locks where the verifier understands
when a lock held in a caller is released in callee, or lock taken in
callee is released in a caller, or the callee is called within a lock
critical section. This set extends the same semantics to RCU read locks
and adds a few selftests to verify correct behavior. This issue has also
come up for sched-ext programs.

This would now allow static subprog calls to be made without errors
within RCU read sections, for subprogs to release RCU locks of callers
and return to them, or for subprogs to take RCU lock which is later
released in the caller.

  [0]: https://lore.kernel.org/bpf/20240204120206.796412-1-memxor@gmail.com

Changelog:
----------
v1 -> v2:
v1: https://lore.kernel.org/bpf/20240204230231.1013964-1-memxor@gmail.com

 * Add tests for global subprog behaviour (Yafang)
 * Add Acks, Tested-by (Yonghong, Yafang)
====================

Link: https://lore.kernel.org/r/20240205055646.1112186-1-memxor@gmail.com


Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 8244ab50 8be6a014

kernel/bpf/verifier.c

+1 −2
Original line number Diff line number Diff line
@@ -17703,8 +17703,7 @@ static int do_check(struct bpf_verifier_env *env) 
					return -EINVAL;

				}









				if (env->cur_state->active_rcu_lock &&









				    !in_rbtree_lock_required_cb(env)) {









				if (env->cur_state->active_rcu_lock && !env->cur_state->curframe) {









					verbose(env, "bpf_rcu_read_unlock is missing\n");









					return -EINVAL;









				}

























tools/testing/selftests/bpf/prog_tests/rcu_read_lock.c



+6
−0






























Original line number
Diff line number
Diff line







@@ -29,6 +29,10 @@ static void test_success(void)







	bpf_program__set_autoload(skel->progs.non_sleepable_1, true);









	bpf_program__set_autoload(skel->progs.non_sleepable_2, true);









	bpf_program__set_autoload(skel->progs.task_trusted_non_rcuptr, true);









	bpf_program__set_autoload(skel->progs.rcu_read_lock_subprog, true);









	bpf_program__set_autoload(skel->progs.rcu_read_lock_global_subprog, true);









	bpf_program__set_autoload(skel->progs.rcu_read_lock_subprog_lock, true);









	bpf_program__set_autoload(skel->progs.rcu_read_lock_subprog_unlock, true);









	err = rcu_read_lock__load(skel);









	if (!ASSERT_OK(err, "skel_load"))









		goto out;








@@ -75,6 +79,8 @@ static const char * const inproper_region_tests[] = {







	"inproper_sleepable_helper",









	"inproper_sleepable_kfunc",









	"nested_rcu_region",









	"rcu_read_lock_global_subprog_lock",









	"rcu_read_lock_global_subprog_unlock",









};



















static void test_inproper_region(void)
































tools/testing/selftests/bpf/progs/rcu_read_lock.c



+120
−0






























Original line number
Diff line number
Diff line







@@ -319,3 +319,123 @@ int cross_rcu_region(void *ctx)







	bpf_rcu_read_unlock();









	return 0;









}



















__noinline









static int static_subprog(void *ctx)









{









	volatile int ret = 0;



















	if (bpf_get_prandom_u32())









		return ret + 42;









	return ret + bpf_get_prandom_u32();









}



















__noinline









int global_subprog(u64 a)









{









	volatile int ret = a;



















	return ret + static_subprog(NULL);









}



















__noinline









static int static_subprog_lock(void *ctx)









{









	volatile int ret = 0;



















	bpf_rcu_read_lock();









	if (bpf_get_prandom_u32())









		return ret + 42;









	return ret + bpf_get_prandom_u32();









}



















__noinline









int global_subprog_lock(u64 a)









{









	volatile int ret = a;



















	return ret + static_subprog_lock(NULL);









}



















__noinline









static int static_subprog_unlock(void *ctx)









{









	volatile int ret = 0;



















	bpf_rcu_read_unlock();









	if (bpf_get_prandom_u32())









		return ret + 42;









	return ret + bpf_get_prandom_u32();









}



















__noinline









int global_subprog_unlock(u64 a)









{









	volatile int ret = a;



















	return ret + static_subprog_unlock(NULL);









}



















SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")









int rcu_read_lock_subprog(void *ctx)









{









	volatile int ret = 0;



















	bpf_rcu_read_lock();









	if (bpf_get_prandom_u32())









		ret += static_subprog(ctx);









	bpf_rcu_read_unlock();









	return 0;









}



















SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")









int rcu_read_lock_global_subprog(void *ctx)









{









	volatile int ret = 0;



















	bpf_rcu_read_lock();









	if (bpf_get_prandom_u32())









		ret += global_subprog(ret);









	bpf_rcu_read_unlock();









	return 0;









}



















SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")









int rcu_read_lock_subprog_lock(void *ctx)









{









	volatile int ret = 0;



















	ret += static_subprog_lock(ctx);









	bpf_rcu_read_unlock();









	return 0;









}



















SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")









int rcu_read_lock_global_subprog_lock(void *ctx)









{









	volatile int ret = 0;



















	ret += global_subprog_lock(ret);









	bpf_rcu_read_unlock();









	return 0;









}



















SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")









int rcu_read_lock_subprog_unlock(void *ctx)









{









	volatile int ret = 0;



















	bpf_rcu_read_lock();









	ret += static_subprog_unlock(ctx);









	return 0;









}



















SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")









int rcu_read_lock_global_subprog_unlock(void *ctx)









{









	volatile int ret = 0;



















	bpf_rcu_read_lock();









	ret += global_subprog_unlock(ret);









	return 0;









}