Now that BPF supports adding new kernel functions with kfuncs, and storing
kernel objects in maps with kptrs, we can add a set of kfuncs which allow
struct task_struct objects to be stored in maps as referenced kptrs.
The possible use-cases for doing this are plentiful. During tracing,
for example, it would be useful to be able to collect some tasks that
performed a certain operation, and then periodically summarize who they
are, which cgroup they're in, how much CPU time they've spent, etc.
Doing this now would require storing the task's pids along with some
relevant data to be exported to user space, and later associating the
pids to tasks in other event handlers where the data is recorded.
Another useful by-product of this is that it allows a program to pin a
task, and by proxy therefore also pin its task local storage.
This patch set adds this aforementioned set of kfuncs, along with a new
selftest suite for validation.
Signed-off-by: David Vernet <[email protected]>
---
v1 -> v2:
- Rename tracing_btf_ids to generic_kfunc_btf_ids, and add the new
kfuncs to that list instead of making a separate btf id list (Alexei).
- Don't run the new selftest suite on s390x, which doesn't appear to
support invoking kfuncs.
- Add a missing __diag_ignore block for -Wmissing-prototypes
([email protected]).
- Fix formatting on some of the SPDX-License-Identifier tags.
- Clarified the function header comment a bit on bpf_task_kptr_get().
David Vernet (2):
bpf: Add kfuncs for storing struct task_struct * as a kptr
bpf/selftests: Add selftests for new task kfuncs
kernel/bpf/helpers.c | 83 ++++++-
tools/testing/selftests/bpf/DENYLIST.s390x | 1 +
.../selftests/bpf/prog_tests/task_kfunc.c | 155 ++++++++++++
.../selftests/bpf/progs/task_kfunc_common.h | 83 +++++++
.../selftests/bpf/progs/task_kfunc_failure.c | 225 ++++++++++++++++++
.../selftests/bpf/progs/task_kfunc_success.c | 113 +++++++++
6 files changed, 655 insertions(+), 5 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/task_kfunc.c
create mode 100644 tools/testing/selftests/bpf/progs/task_kfunc_common.h
create mode 100644 tools/testing/selftests/bpf/progs/task_kfunc_failure.c
create mode 100644 tools/testing/selftests/bpf/progs/task_kfunc_success.c
--
2.37.3
Now that BPF supports adding new kernel functions with kfuncs, and
storing kernel objects in maps with kptrs, we can add a set of kfuncs
which allow struct task_struct objects to be stored in maps as
referenced kptrs. The possible use-cases for doing this are plentiful.
During tracing, for example, it would be useful to be able to collect
some tasks that performed a certain operation, and then periodically
summarize who they are, which cgroup they're in, how much CPU time
they've spent, etc.
In order to enable this, this patch adds three new kfuncs:
struct task_struct *bpf_task_acquire(struct task_struct *p);
struct task_struct *bpf_task_kptr_get(struct task_struct **pp);
void bpf_task_release(struct task_struct *p);
A follow-on patch will add selftests validating these kfuncs.
Signed-off-by: David Vernet <[email protected]>
---
kernel/bpf/helpers.c | 83 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 78 insertions(+), 5 deletions(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index b069517a3da0..36cbe1b8f8b1 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1700,20 +1700,93 @@ bpf_base_func_proto(enum bpf_func_id func_id)
}
}
-BTF_SET8_START(tracing_btf_ids)
+__diag_push();
+__diag_ignore_all("-Wmissing-prototypes",
+ "Global functions as their definitions will be in vmlinux BTF");
+
+/**
+ * bpf_task_acquire - Acquire a reference to a task. A task acquired by this
+ * kfunc which is not stored in a map as a kptr, must be released by calling
+ * bpf_task_release().
+ * @p: The task on which a reference is being acquired.
+ */
+__used noinline
+struct task_struct *bpf_task_acquire(struct task_struct *p)
+{
+ refcount_inc(&p->rcu_users);
+ return p;
+}
+
+/**
+ * bpf_task_kptr_get - Acquire a reference on a struct task_struct kptr. A task
+ * kptr acquired by this kfunc which is not subsequently stored in a map, must
+ * be released by calling bpf_task_release().
+ * @pp: A pointer to a task kptr on which a reference is being acquired.
+ */
+__used noinline
+struct task_struct *bpf_task_kptr_get(struct task_struct **pp)
+{
+ struct task_struct *p;
+
+ rcu_read_lock();
+ p = READ_ONCE(*pp);
+ if (p && !refcount_inc_not_zero(&p->rcu_users))
+ p = NULL;
+ rcu_read_unlock();
+
+ return p;
+}
+
+/**
+ * bpf_task_release - Release the reference acquired on a struct task_struct *.
+ * If this kfunc is invoked in an RCU read region, the task_struct is
+ * guaranteed to not be freed until the current grace period has ended, even if
+ * its refcount drops to 0.
+ * @p: The task on which a reference is being released.
+ */
+__used noinline void bpf_task_release(struct task_struct *p)
+{
+ if (!p)
+ return;
+
+ put_task_struct_rcu_user(p);
+}
+
+__diag_pop();
+
+BTF_SET8_START(generic_kfunc_btf_ids)
#ifdef CONFIG_KEXEC_CORE
BTF_ID_FLAGS(func, crash_kexec, KF_DESTRUCTIVE)
#endif
-BTF_SET8_END(tracing_btf_ids)
+BTF_ID_FLAGS(func, bpf_task_acquire, KF_ACQUIRE)
+BTF_ID_FLAGS(func, bpf_task_kptr_get, KF_ACQUIRE | KF_KPTR_GET | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_task_release, KF_RELEASE | KF_TRUSTED_ARGS)
+BTF_SET8_END(generic_kfunc_btf_ids)
-static const struct btf_kfunc_id_set tracing_kfunc_set = {
+static const struct btf_kfunc_id_set generic_kfunc_set = {
.owner = THIS_MODULE,
- .set = &tracing_btf_ids,
+ .set = &generic_kfunc_btf_ids,
};
+BTF_ID_LIST(generic_kfunc_dtor_ids)
+BTF_ID(struct, task_struct)
+BTF_ID(func, bpf_task_release)
+
static int __init kfunc_init(void)
{
- return register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &tracing_kfunc_set);
+ int ret;
+ const struct btf_id_dtor_kfunc generic_kfunc_dtors[] = {
+ {
+ .btf_id = generic_kfunc_dtor_ids[0],
+ .kfunc_btf_id = generic_kfunc_dtor_ids[1]
+ },
+ };
+
+ ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &generic_kfunc_set);
+ ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &generic_kfunc_set);
+ return ret ?: register_btf_id_dtor_kfuncs(generic_kfunc_dtors,
+ ARRAY_SIZE(generic_kfunc_dtors),
+ THIS_MODULE);
}
late_initcall(kfunc_init);
--
2.37.3
On 10/1/22 7:47 AM, David Vernet wrote:
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index b069517a3da0..36cbe1b8f8b1 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -1700,20 +1700,93 @@ bpf_base_func_proto(enum bpf_func_id func_id)
> }
> }
>
> -BTF_SET8_START(tracing_btf_ids)
> +__diag_push();
> +__diag_ignore_all("-Wmissing-prototypes",
> + "Global functions as their definitions will be in vmlinux BTF");
> +
> +/**
> + * bpf_task_acquire - Acquire a reference to a task. A task acquired by this
> + * kfunc which is not stored in a map as a kptr, must be released by calling
> + * bpf_task_release().
> + * @p: The task on which a reference is being acquired.
> + */
> +__used noinline
> +struct task_struct *bpf_task_acquire(struct task_struct *p)
> +{
> + refcount_inc(&p->rcu_users);
This probably needs to be refcount_inc_not_zero() also for the cases like during
the task free tracepoint ?
On Mon, Oct 03, 2022 at 12:20:57PM -0700, Martin KaFai Lau wrote:
> On 10/1/22 7:47 AM, David Vernet wrote:
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index b069517a3da0..36cbe1b8f8b1 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -1700,20 +1700,93 @@ bpf_base_func_proto(enum bpf_func_id func_id)
> > }
> > }
> > -BTF_SET8_START(tracing_btf_ids)
> > +__diag_push();
> > +__diag_ignore_all("-Wmissing-prototypes",
> > + "Global functions as their definitions will be in vmlinux BTF");
> > +
> > +/**
> > + * bpf_task_acquire - Acquire a reference to a task. A task acquired by this
> > + * kfunc which is not stored in a map as a kptr, must be released by calling
> > + * bpf_task_release().
> > + * @p: The task on which a reference is being acquired.
> > + */
> > +__used noinline
> > +struct task_struct *bpf_task_acquire(struct task_struct *p)
> > +{
> > + refcount_inc(&p->rcu_users);
>
> This probably needs to be refcount_inc_not_zero() also for the cases like
> during the task free tracepoint ?
Thanks, you're probably right. As you pointed out offline as well, some
fentry functions may expect a NULL pointer, so we probably need to
update this to check for NULL and also return KF_RET_NULL. I'll take
care of that in v2 once we've aligned on the RCU / sleepable progs
question we're discussing with Kumar.