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

[Yonghong Song <yhs@meta.com>]

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@fb.com>
> 
> [...]
> 
>> +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;
>   +}