Re: [PATCH bpf-next v7 4/4] selftests/bpf: Add tests for bpf_rcu_read_lock()

[Yonghong Song <yhs@xxxxxxxx>]

On 11/21/22 5:59 PM, John Fastabend wrote:
> Yonghong Song wrote:
> > Add a few positive/negative tests to test bpf_rcu_read_lock()
> > and its corresponding verifier support. The new test will fail
> > on s390x and aarch64, so an entry is added to each of their
> > respective deny lists.
> >
> > Signed-off-by: Yonghong Song <yhs@xxxxxx>
>
> [...]
>
> > +SEC("?fentry.s/" SYS_PREFIX "sys_nanosleep")
> > +int nested_rcu_region(void *ctx)
> > +{
> > +	struct task_struct *task, *real_parent;
> > +
> > +	/* nested rcu read lock regions */
> > +	task = bpf_get_current_task_btf();
> > +	bpf_rcu_read_lock();
> > +	bpf_rcu_read_lock();
> > +	real_parent = task->real_parent;
> > +	(void)bpf_task_storage_get(&map_a, real_parent, 0, 0);
> > +	bpf_rcu_read_unlock();
> > +	bpf_rcu_read_unlock();
> > +	return 0;
> > +}
>
> I think you also need the nested imbalance case is this
> handled? It looks like the active_rcu is just a bool?

Currently we don't support nested bpf_rcu_read_lock()
regions. So the error will appear for the second
bpf_rcu_read_lock() in the above code, regardless of
the eventual balance or not-balance.

>   +SEC("?fentry.s/" SYS_PREFIX "sys_nanosleep")
>   +int nested_rcu_region(void *ctx)
>   +{
>   +	struct task_struct *task, *real_parent;
>   +
>   +	/* nested rcu read lock regions */
>   +	task = bpf_get_current_task_btf();
>   +	bpf_rcu_read_lock();
>   +	bpf_rcu_read_lock();
>   +	real_parent = task->real_parent;
>   +	(void)bpf_task_storage_get(&map_a, real_parent, 0, 0);
>   +      // imbalance unlock()
>   +	bpf_rcu_read_unlock();
>   +	return 0;
>   +}