Commit 387b5321 authored by Martin KaFai Lau's avatar Martin KaFai Lau Committed by Alexei Starovoitov
Browse files

selftests/bpf: Tracing prog can still do lookup under busy lock 



This patch modifies the task_ls_recursion test to check that
the first bpf_task_storage_get(&map_a, ...) in BPF_PROG(on_update)
can still do the lockless lookup even it cannot acquire the percpu
busy lock.  If the lookup succeeds, it will increment the value
by 1 and the value in the task storage map_a will become 200+1=201.
After that, BPF_PROG(on_update) tries to delete from map_a and
should get -EBUSY because it cannot acquire the percpu busy lock
after finding the data.

Signed-off-by: default avatarMartin KaFai Lau <martin.lau@kernel.org>
Link: https://lore.kernel.org/r/20221025184524.3526117-10-martin.lau@linux.dev


Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent 0334b4d8

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

+46 −2
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@ 
#include <sys/syscall.h>   /* For SYS_xxx definitions */

#include <sys/types.h>

#include <test_progs.h>

#include "task_local_storage_helpers.h"

#include "task_local_storage.skel.h"

#include "task_local_storage_exit_creds.skel.h"

#include "task_ls_recursion.skel.h"
@@ -78,21 +79,64 @@ static void test_exit_creds(void) 


static void test_recursion(void)

{

	int err, map_fd, prog_fd, task_fd;

	struct task_ls_recursion *skel;

	int err;

	struct bpf_prog_info info;

	__u32 info_len = sizeof(info);

	long value;



	task_fd = sys_pidfd_open(getpid(), 0);

	if (!ASSERT_NEQ(task_fd, -1, "sys_pidfd_open"))

		return;



	skel = task_ls_recursion__open_and_load();

	if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))

		return;

		goto out;



	err = task_ls_recursion__attach(skel);

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

		goto out;



	/* trigger sys_enter, make sure it does not cause deadlock */

	skel->bss->test_pid = getpid();

	syscall(SYS_gettid);

	skel->bss->test_pid = 0;

	task_ls_recursion__detach(skel);



	/* Refer to the comment in BPF_PROG(on_update) for

	 * the explanation on the value 201 and 100.

	 */

	map_fd = bpf_map__fd(skel->maps.map_a);

	err = bpf_map_lookup_elem(map_fd, &task_fd, &value);

	ASSERT_OK(err, "lookup map_a");

	ASSERT_EQ(value, 201, "map_a value");

	ASSERT_EQ(skel->bss->nr_del_errs, 1, "bpf_task_storage_delete busy");



	map_fd = bpf_map__fd(skel->maps.map_b);

	err = bpf_map_lookup_elem(map_fd, &task_fd, &value);

	ASSERT_OK(err, "lookup map_b");

	ASSERT_EQ(value, 100, "map_b value");



	prog_fd = bpf_program__fd(skel->progs.on_lookup);

	memset(&info, 0, sizeof(info));

	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);

	ASSERT_OK(err, "get prog info");

	ASSERT_GT(info.recursion_misses, 0, "on_lookup prog recursion");



	prog_fd = bpf_program__fd(skel->progs.on_update);

	memset(&info, 0, sizeof(info));

	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);

	ASSERT_OK(err, "get prog info");

	ASSERT_EQ(info.recursion_misses, 0, "on_update prog recursion");



	prog_fd = bpf_program__fd(skel->progs.on_enter);

	memset(&info, 0, sizeof(info));

	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);

	ASSERT_OK(err, "get prog info");

	ASSERT_EQ(info.recursion_misses, 0, "on_enter prog recursion");



out:

	close(task_fd);

	task_ls_recursion__destroy(skel);

}

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

+40 −3
Original line number Diff line number Diff line
@@ -5,7 +5,13 @@ 
#include <bpf/bpf_helpers.h>

#include <bpf/bpf_tracing.h>



#ifndef EBUSY

#define EBUSY 16

#endif



char _license[] SEC("license") = "GPL";

int nr_del_errs = 0;

int test_pid = 0;



struct {

	__uint(type, BPF_MAP_TYPE_TASK_STORAGE);
@@ -26,6 +32,13 @@ int BPF_PROG(on_lookup) 
{

	struct task_struct *task = bpf_get_current_task_btf();



	if (!test_pid || task->pid != test_pid)

		return 0;



	/* The bpf_task_storage_delete will call

	 * bpf_local_storage_lookup.  The prog->active will

	 * stop the recursion.

	 */

	bpf_task_storage_delete(&map_a, task);

	bpf_task_storage_delete(&map_b, task);

	return 0;
@@ -37,11 +50,32 @@ int BPF_PROG(on_update) 
	struct task_struct *task = bpf_get_current_task_btf();

	long *ptr;



	if (!test_pid || task->pid != test_pid)

		return 0;



	ptr = bpf_task_storage_get(&map_a, task, 0,

				   BPF_LOCAL_STORAGE_GET_F_CREATE);

	if (ptr)

	/* ptr will not be NULL when it is called from

	 * the bpf_task_storage_get(&map_b,...F_CREATE) in

	 * the BPF_PROG(on_enter) below.  It is because

	 * the value can be found in map_a and the kernel

	 * does not need to acquire any spin_lock.

	 */

	if (ptr) {

		int err;



		*ptr += 1;

		err = bpf_task_storage_delete(&map_a, task);

		if (err == -EBUSY)

			nr_del_errs++;

	}



	/* This will still fail because map_b is empty and

	 * this BPF_PROG(on_update) has failed to acquire

	 * the percpu busy lock => meaning potential

	 * deadlock is detected and it will fail to create

	 * new storage.

	 */

	ptr = bpf_task_storage_get(&map_b, task, 0,

				   BPF_LOCAL_STORAGE_GET_F_CREATE);

	if (ptr)
@@ -57,14 +91,17 @@ int BPF_PROG(on_enter, struct pt_regs *regs, long id) 
	long *ptr;



	task = bpf_get_current_task_btf();

	if (!test_pid || task->pid != test_pid)

		return 0;



	ptr = bpf_task_storage_get(&map_a, task, 0,

				   BPF_LOCAL_STORAGE_GET_F_CREATE);

	if (ptr)

	if (ptr && !*ptr)

		*ptr = 200;



	ptr = bpf_task_storage_get(&map_b, task, 0,

				   BPF_LOCAL_STORAGE_GET_F_CREATE);

	if (ptr)

	if (ptr && !*ptr)

		*ptr = 100;

	return 0;

}