On Fri, Nov 04, 2022 at 02:30:55AM IST, Yonghong Song wrote:
>
>
> On 11/3/22 7:28 AM, KP Singh wrote:
> > On Thu, Nov 3, 2022 at 8:21 AM Yonghong Song <yhs@xxxxxx> wrote:
> > >
> > > Add bpf_rcu_read_lock() and bpf_rcu_read_unlock() helpers.
> > > Both helpers are available to all program types with
> > > CAP_BPF capability.
> > >
> > > Signed-off-by: Yonghong Song <yhs@xxxxxx>
> > > ---
> > > include/linux/bpf.h | 2 ++
> > > include/uapi/linux/bpf.h | 14 ++++++++++++++
> > > kernel/bpf/core.c | 2 ++
> > > kernel/bpf/helpers.c | 26 ++++++++++++++++++++++++++
> > > tools/include/uapi/linux/bpf.h | 14 ++++++++++++++
> > > 5 files changed, 58 insertions(+)
> > >
> > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > > index 8d948bfcb984..a9bda4c91fc7 100644
> > > --- a/include/linux/bpf.h
> > > +++ b/include/linux/bpf.h
> > > @@ -2554,6 +2554,8 @@ extern const struct bpf_func_proto bpf_get_retval_proto;
> > > extern const struct bpf_func_proto bpf_user_ringbuf_drain_proto;
> > > extern const struct bpf_func_proto bpf_cgrp_storage_get_proto;
> > > extern const struct bpf_func_proto bpf_cgrp_storage_delete_proto;
> > > +extern const struct bpf_func_proto bpf_rcu_read_lock_proto;
> > > +extern const struct bpf_func_proto bpf_rcu_read_unlock_proto;
> > >
> > > const struct bpf_func_proto *tracing_prog_func_proto(
> > > enum bpf_func_id func_id, const struct bpf_prog *prog);
> > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > index 94659f6b3395..e86389cd6133 100644
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -5481,6 +5481,18 @@ union bpf_attr {
> > > * 0 on success.
> > > *
> > > * **-ENOENT** if the bpf_local_storage cannot be found.
> > > + *
> > > + * void bpf_rcu_read_lock(void)
> > > + * Description
> > > + * Call kernel rcu_read_lock().
> >
> > Simple wrapper around rcu_read_lock() and maybe explain where and how
> > it is supposed to
> > be used.
> >
> > e.g. the verifier will check if __rcu pointers are being accessed with
> > bpf_rcu_read_lock in
> > sleepable programs.
>
> Okay, I can add more descriptions.
>
> >
> > Calling the helper from a non-sleepable program is inconsequential,
> > but maybe we can even
> > avoid exposing it to non-sleepable programs?
>
> I actually debated myself whether to make bpf_rcu_read_lock()/unlock()
> to be sleepable only. Although it won't hurt for non-sleepable program,
> I guess I can make it as sleepable only so users don't make mistake
> to use them in non-sleepable programs.
>
It's better to let it be a noop in non-sleepable programs but still allow
calling it. It allows writing common helper functions in the BPF program that
work in both sleepable and non-sleepable cases by holding the RCU read lock.
