lkml.org 
[lkml]   [2022]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.17 72/75] bpf: Fix usage of trace RCU in local storage.
    Date
    From: KP Singh <kpsingh@kernel.org>

    commit dcf456c9a095a6e71f53d6f6f004133ee851ee70 upstream.

    bpf_{sk,task,inode}_storage_free() do not need to use
    call_rcu_tasks_trace as no BPF program should be accessing the owner
    as it's being destroyed. The only other reader at this point is
    bpf_local_storage_map_free() which uses normal RCU.

    The only path that needs trace RCU are:

    * bpf_local_storage_{delete,update} helpers
    * map_{delete,update}_elem() syscalls

    Fixes: 0fe4b381a59e ("bpf: Allow bpf_local_storage to be used by sleepable programs")
    Signed-off-by: KP Singh <kpsingh@kernel.org>
    Signed-off-by: Alexei Starovoitov <ast@kernel.org>
    Acked-by: Martin KaFai Lau <kafai@fb.com>
    Link: https://lore.kernel.org/bpf/20220418155158.2865678-1-kpsingh@kernel.org
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    include/linux/bpf_local_storage.h | 4 ++--
    kernel/bpf/bpf_inode_storage.c | 4 ++--
    kernel/bpf/bpf_local_storage.c | 29 +++++++++++++++++++----------
    kernel/bpf/bpf_task_storage.c | 4 ++--
    net/core/bpf_sk_storage.c | 6 +++---
    5 files changed, 28 insertions(+), 19 deletions(-)

    --- a/include/linux/bpf_local_storage.h
    +++ b/include/linux/bpf_local_storage.h
    @@ -143,9 +143,9 @@ void bpf_selem_link_storage_nolock(struc

    bool bpf_selem_unlink_storage_nolock(struct bpf_local_storage *local_storage,
    struct bpf_local_storage_elem *selem,
    - bool uncharge_omem);
    + bool uncharge_omem, bool use_trace_rcu);

    -void bpf_selem_unlink(struct bpf_local_storage_elem *selem);
    +void bpf_selem_unlink(struct bpf_local_storage_elem *selem, bool use_trace_rcu);

    void bpf_selem_link_map(struct bpf_local_storage_map *smap,
    struct bpf_local_storage_elem *selem);
    --- a/kernel/bpf/bpf_inode_storage.c
    +++ b/kernel/bpf/bpf_inode_storage.c
    @@ -90,7 +90,7 @@ void bpf_inode_storage_free(struct inode
    */
    bpf_selem_unlink_map(selem);
    free_inode_storage = bpf_selem_unlink_storage_nolock(
    - local_storage, selem, false);
    + local_storage, selem, false, false);
    }
    raw_spin_unlock_bh(&local_storage->lock);
    rcu_read_unlock();
    @@ -149,7 +149,7 @@ static int inode_storage_delete(struct i
    if (!sdata)
    return -ENOENT;

    - bpf_selem_unlink(SELEM(sdata));
    + bpf_selem_unlink(SELEM(sdata), true);

    return 0;
    }
    --- a/kernel/bpf/bpf_local_storage.c
    +++ b/kernel/bpf/bpf_local_storage.c
    @@ -106,7 +106,7 @@ static void bpf_selem_free_rcu(struct rc
    */
    bool bpf_selem_unlink_storage_nolock(struct bpf_local_storage *local_storage,
    struct bpf_local_storage_elem *selem,
    - bool uncharge_mem)
    + bool uncharge_mem, bool use_trace_rcu)
    {
    struct bpf_local_storage_map *smap;
    bool free_local_storage;
    @@ -150,11 +150,16 @@ bool bpf_selem_unlink_storage_nolock(str
    SDATA(selem))
    RCU_INIT_POINTER(local_storage->cache[smap->cache_idx], NULL);

    - call_rcu_tasks_trace(&selem->rcu, bpf_selem_free_rcu);
    + if (use_trace_rcu)
    + call_rcu_tasks_trace(&selem->rcu, bpf_selem_free_rcu);
    + else
    + kfree_rcu(selem, rcu);
    +
    return free_local_storage;
    }

    -static void __bpf_selem_unlink_storage(struct bpf_local_storage_elem *selem)
    +static void __bpf_selem_unlink_storage(struct bpf_local_storage_elem *selem,
    + bool use_trace_rcu)
    {
    struct bpf_local_storage *local_storage;
    bool free_local_storage = false;
    @@ -169,12 +174,16 @@ static void __bpf_selem_unlink_storage(s
    raw_spin_lock_irqsave(&local_storage->lock, flags);
    if (likely(selem_linked_to_storage(selem)))
    free_local_storage = bpf_selem_unlink_storage_nolock(
    - local_storage, selem, true);
    + local_storage, selem, true, use_trace_rcu);
    raw_spin_unlock_irqrestore(&local_storage->lock, flags);

    - if (free_local_storage)
    - call_rcu_tasks_trace(&local_storage->rcu,
    + if (free_local_storage) {
    + if (use_trace_rcu)
    + call_rcu_tasks_trace(&local_storage->rcu,
    bpf_local_storage_free_rcu);
    + else
    + kfree_rcu(local_storage, rcu);
    + }
    }

    void bpf_selem_link_storage_nolock(struct bpf_local_storage *local_storage,
    @@ -214,14 +223,14 @@ void bpf_selem_link_map(struct bpf_local
    raw_spin_unlock_irqrestore(&b->lock, flags);
    }

    -void bpf_selem_unlink(struct bpf_local_storage_elem *selem)
    +void bpf_selem_unlink(struct bpf_local_storage_elem *selem, bool use_trace_rcu)
    {
    /* Always unlink from map before unlinking from local_storage
    * because selem will be freed after successfully unlinked from
    * the local_storage.
    */
    bpf_selem_unlink_map(selem);
    - __bpf_selem_unlink_storage(selem);
    + __bpf_selem_unlink_storage(selem, use_trace_rcu);
    }

    struct bpf_local_storage_data *
    @@ -454,7 +463,7 @@ bpf_local_storage_update(void *owner, st
    if (old_sdata) {
    bpf_selem_unlink_map(SELEM(old_sdata));
    bpf_selem_unlink_storage_nolock(local_storage, SELEM(old_sdata),
    - false);
    + false, true);
    }

    unlock:
    @@ -532,7 +541,7 @@ void bpf_local_storage_map_free(struct b
    migrate_disable();
    __this_cpu_inc(*busy_counter);
    }
    - bpf_selem_unlink(selem);
    + bpf_selem_unlink(selem, false);
    if (busy_counter) {
    __this_cpu_dec(*busy_counter);
    migrate_enable();
    --- a/kernel/bpf/bpf_task_storage.c
    +++ b/kernel/bpf/bpf_task_storage.c
    @@ -102,7 +102,7 @@ void bpf_task_storage_free(struct task_s
    */
    bpf_selem_unlink_map(selem);
    free_task_storage = bpf_selem_unlink_storage_nolock(
    - local_storage, selem, false);
    + local_storage, selem, false, false);
    }
    raw_spin_unlock_irqrestore(&local_storage->lock, flags);
    bpf_task_storage_unlock();
    @@ -191,7 +191,7 @@ static int task_storage_delete(struct ta
    if (!sdata)
    return -ENOENT;

    - bpf_selem_unlink(SELEM(sdata));
    + bpf_selem_unlink(SELEM(sdata), true);

    return 0;
    }
    --- a/net/core/bpf_sk_storage.c
    +++ b/net/core/bpf_sk_storage.c
    @@ -40,7 +40,7 @@ static int bpf_sk_storage_del(struct soc
    if (!sdata)
    return -ENOENT;

    - bpf_selem_unlink(SELEM(sdata));
    + bpf_selem_unlink(SELEM(sdata), true);

    return 0;
    }
    @@ -75,8 +75,8 @@ void bpf_sk_storage_free(struct sock *sk
    * sk_storage.
    */
    bpf_selem_unlink_map(selem);
    - free_sk_storage = bpf_selem_unlink_storage_nolock(sk_storage,
    - selem, true);
    + free_sk_storage = bpf_selem_unlink_storage_nolock(
    + sk_storage, selem, true, false);
    }
    raw_spin_unlock_bh(&sk_storage->lock);
    rcu_read_unlock();

    \
     
     \ /
      Last update: 2022-06-03 20:14    [W:4.942 / U:0.100 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site