On Tue, Sep 15, 2020 at 11:16:49PM -0700, Yonghong Song wrote:
[ ... ]
> diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
> index 4a86ea34f29e..d43c3d6d0693 100644
> --- a/net/core/bpf_sk_storage.c
> +++ b/net/core/bpf_sk_storage.c
> @@ -678,6 +678,7 @@ struct bpf_iter_seq_sk_storage_map_info {
> static struct bpf_local_storage_elem *
> bpf_sk_storage_map_seq_find_next(struct bpf_iter_seq_sk_storage_map_info *info,
> struct bpf_local_storage_elem *prev_selem)
> + __acquires(RCU) __releases(RCU)
> {
> struct bpf_local_storage *sk_storage;
> struct bpf_local_storage_elem *selem;
In the while loop earlier in this function, if I read it correctly,
it is sort of continuing the earlier hlist_for_each_entry_rcu() for the
same bucket, so the hlist_entry_safe() needs to be changed also.
Something like this (uncompiled code):
while (selem) {
- selem = hlist_entry_safe(selem->map_node.next,
+ selem = hlist_entry_safe(rcu_dereference(hlist_next_rcu(&selem->map_node)),
struct bpf_local_storage_elem, map_node);
if (!selem) {
/* not found, unlock and go to the next bucket */
> @@ -701,11 +702,11 @@ bpf_sk_storage_map_seq_find_next(struct bpf_iter_seq_sk_storage_map_info *info,
> if (!selem) {
> /* not found, unlock and go to the next bucket */
> b = &smap->buckets[bucket_id++];
> - raw_spin_unlock_bh(&b->lock);
> + rcu_read_unlock();
> skip_elems = 0;
> break;
> }
> - sk_storage = rcu_dereference_raw(selem->local_storage);
> + sk_storage = rcu_dereference(selem->local_storage);
> if (sk_storage) {
> info->skip_elems = skip_elems + count;
> return selem;
