On Sun, Nov 26, 2023 at 6:47 PM Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote:
>
>
> Before posting v4, do the following code-snippets match your suggestions ?
>
> resolve_pseudo_ldimm64()
> if (env->prog->aux->sleepable)
> /* The max number of program is INT_MAX -
> 1, so atomic_t will be enough */
> atomic_inc(&map->sleepable_refcnt);
>
> bpf_map_fd_put_ptr()
> if (deferred) {
> if (atomic_read(&map->sleepable_refcnt))
> WRITE_ONCE(map->free_after_tt_rcu_gp, true);
> else
> WRITE_ONCE(map->free_after_rcu_gp, true);
> }
Yes. That was an idea and I hope you see that in this form it's
much easier to understand, right?
and regarding your other question:
> Could we replace sleepable_refcnt by
> a bool (e.g, access_in_sleepable), so the check can be simplified further ?
I think you're trying to optimize too soon.
How would that bool access_in_sleepable work?
Something needs to set it and the last sleepable to unset it.
You might need a refcnt to do that.
>
> bpf_map_put()
> if (READ_ONCE(map->free_after_tt_rcu_gp))
> call_rcu_tasks_trace(&map->rcu,
> bpf_map_free_mult_rcu_gp);
> else if (READ_ONCE(map->free_after_rcu_gp))
> call_rcu(&map->rcu, bpf_map_free_rcu_gp);
> else
> bpf_map_free_in_work(map);
>
> And are you OK with using call_rcu()/call_rcu_tasks_trace() instead of
> synchronize_rcu()/synchronize_rcu_mult() ?
Of course. Not sure what you meant by that.
bpf_map_put() cannot call sync_rcu.
Are you asking about doing queue_work first and then sync_rcu* inside?
I think it will not scale compared to call_rcu approach.
