Re: [PATCH bpf-next 2/3] bpf: Remove KF_KPTR_GET kfunc flag

[Alexei Starovoitov]

On Sat, Apr 15, 2023 at 05:32:30AM -0500, David Vernet wrote:
> We've managed to improve the UX for kptrs significantly over the last 9
> months. All of the existing use cases which previously had KF_KPTR_GET
> kfuncs (struct bpf_cpumask *, struct task_struct *, and struct cgroup *)
> have all been updated to be synchronized using RCU. In other words,
> their KF_KPTR_GET kfuncs have been removed in favor of KF_RU |

typo: KF_RCU.
same typo is in cover letter.

> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 495250162422..7721c90ead5b 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -18,7 +18,6 @@
>  #define KF_ACQUIRE	(1 << 0) /* kfunc is an acquire function */
>  #define KF_RELEASE	(1 << 1) /* kfunc is a release function */
>  #define KF_RET_NULL	(1 << 2) /* kfunc returns a pointer that may be NULL */
> -#define KF_KPTR_GET	(1 << 3) /* kfunc returns reference to a kptr */
>  /* Trusted arguments are those which are guaranteed to be valid when passed to
>   * the kfunc. It is used to enforce that pointers obtained from either acquire
>   * kfuncs, or from the main kernel on a tracepoint or struct_ops callback
> @@ -67,14 +66,14 @@
>   *	return 0;
>   * }
>   */
> -#define KF_TRUSTED_ARGS (1 << 4) /* kfunc only takes trusted pointer arguments */
> -#define KF_SLEEPABLE    (1 << 5) /* kfunc may sleep */
> -#define KF_DESTRUCTIVE  (1 << 6) /* kfunc performs destructive actions */
> -#define KF_RCU          (1 << 7) /* kfunc takes either rcu or trusted pointer arguments */
> +#define KF_TRUSTED_ARGS (1 << 3) /* kfunc only takes trusted pointer arguments */
> +#define KF_SLEEPABLE    (1 << 4) /* kfunc may sleep */
> +#define KF_DESTRUCTIVE  (1 << 5) /* kfunc performs destructive actions */
> +#define KF_RCU          (1 << 6) /* kfunc takes either rcu or trusted pointer arguments */
>  /* only one of KF_ITER_{NEW,NEXT,DESTROY} could be specified per kfunc */
> -#define KF_ITER_NEW     (1 << 8) /* kfunc implements BPF iter constructor */
> -#define KF_ITER_NEXT    (1 << 9) /* kfunc implements BPF iter next method */
> -#define KF_ITER_DESTROY (1 << 10) /* kfunc implements BPF iter destructor */
> +#define KF_ITER_NEW     (1 << 7) /* kfunc implements BPF iter constructor */
> +#define KF_ITER_NEXT    (1 << 8) /* kfunc implements BPF iter next method */
> +#define KF_ITER_DESTROY (1 << 9) /* kfunc implements BPF iter destructor */

Great cleanup! but let's not renumber flags to reduce the churn.
Just delete #define KF_KPTR_GET  (1 << 3)
I bet soon enough we will add another KF_ flag in that place.