On 8/18/20 1:56 AM, Martin KaFai Lau wrote:
> On Mon, Aug 03, 2020 at 06:46:49PM +0200, KP Singh wrote:
>> From: KP Singh <kpsi...@google.com>
>>
>> Flags/consts:
>>
>> SK_STORAGE_CREATE_FLAG_MASK BPF_LOCAL_STORAGE_CREATE_FLAG_MASK
>> BPF_SK_STORAGE_CACHE_SIZE BPF_LOCAL_STORAGE_CACHE_SIZE
>> MAX_VALUE_SIZE BPF_LOCAL_STORAGE_MAX_VALUE_SIZE
>>
>> Structs:
>>
>> bucket bpf_local_storage_map_bucket
>> bpf_sk_storage_map bpf_local_storage_map
>> bpf_sk_storage_data bpf_local_storage_data
>> bpf_sk_storage_elem bpf_local_storage_elem
>> bpf_sk_storage bpf_local_storage
>>
>> The "sk" member in bpf_local_storage is also updated to "owner"
>> in preparation for changing the type to void * in a subsequent patch.
>>
>> Functions:
>>
>> selem_linked_to_sk selem_linked_to_storage
>> selem_alloc bpf_selem_alloc
>> __selem_unlink_sk bpf_selem_unlink_storage
>> __selem_link_sk bpf_selem_link_storage
>> selem_unlink_sk __bpf_selem_unlink_storage
>> sk_storage_update bpf_local_storage_update
>> __sk_storage_lookup bpf_local_storage_lookup
>> bpf_sk_storage_map_free bpf_local_storage_map_free
>> bpf_sk_storage_map_alloc bpf_local_storage_map_alloc
>> bpf_sk_storage_map_alloc_check bpf_local_storage_map_alloc_check
>> bpf_sk_storage_map_check_btf bpf_local_storage_map_check_btf
>>
>
> [ ... ]
>
>> @@ -147,85 +148,86 @@ static struct bpf_sk_storage_elem *selem_alloc(struct
>> bpf_sk_storage_map *smap,
>> * The caller must ensure selem->smap is still valid to be
>> * dereferenced for its smap->elem_size and smap->cache_idx.
>> */
>> -static bool __selem_unlink_sk(struct bpf_sk_storage *sk_storage,
>> - struct bpf_sk_storage_elem *selem,
>> - bool uncharge_omem)
>> +static bool bpf_selem_unlink_storage(struct bpf_local_storage
>> *local_storage,
>> + struct bpf_local_storage_elem *selem,
>> + bool uncharge_omem)
> Please add a "_nolock()" suffix, just to be clear that the unlink_map()
> counter part is locked. It could be a follow up later.
Done.
>
>> {
>> - struct bpf_sk_storage_map *smap;
>> - bool free_sk_storage;
>> + struct bpf_local_storage_map *smap;
>> + bool free_local_storage;
[...]
>> + if (unlikely(!selem_linked_to_storage(selem)))
>> /* selem has already been unlinked from sk */
>> return;
>>
>> - sk_storage = rcu_dereference(selem->sk_storage);
>> - raw_spin_lock_bh(&sk_storage->lock);
>> - if (likely(selem_linked_to_sk(selem)))
>> - free_sk_storage = __selem_unlink_sk(sk_storage, selem, true);
>> - raw_spin_unlock_bh(&sk_storage->lock);
>> + local_storage = rcu_dereference(selem->local_storage);
>> + raw_spin_lock_bh(&local_storage->lock);
>> + if (likely(selem_linked_to_storage(selem)))
>> + free_local_storage =
>> + bpf_selem_unlink_storage(local_storage, selem, true);
>> + raw_spin_unlock_bh(&local_storage->lock);
>>
>> - if (free_sk_storage)
>> - kfree_rcu(sk_storage, rcu);
>> + if (free_local_storage)
>> + kfree_rcu(local_storage, rcu);
>> }
>>
>> -static void __selem_link_sk(struct bpf_sk_storage *sk_storage,
>> - struct bpf_sk_storage_elem *selem)
>> +static void bpf_selem_link_storage(struct bpf_local_storage *local_storage,
>> + struct bpf_local_storage_elem *selem)
> Same here. bpf_selem_link_storage"_nolock"().
Done.
>
> Please tag the Subject line with "bpf:".
Done. Changed it to:
bpf: Renames in preparation for bpf_local_storage
A purely mechanical change to split the renaming from the actual
generalization.
[...]
>
> Acked-by: Martin KaFai Lau <ka...@fb.com>
>
