Commit d6bae125 authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'use-kmalloc_nolock-universally-in-bpf-local-storage'

Amery Hung says:

====================
Use kmalloc_nolock() universally in BPF local storage

Socket local storage did not convert to use kmalloc_nolock() since there
were observable performance degredation due to kfree_nolock() hitting the
slow path and the lack of kfree_rcu()-like batching freeing. Now that
these concern were addressed in slub, convert all remaining local storage
flavors to use kmalloc_nolock().

v1 -> v2:
  - Fix build (CI, Alexei)
====================

Link: https://patch.msgid.link/20260411015419.114016-1-ameryhung@gmail.com


Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 497fa510 136deea4
Loading
Loading
Loading
Loading
+5 −10
Original line number Diff line number Diff line
@@ -54,7 +54,6 @@ struct bpf_local_storage_map {
	u32 bucket_log;
	u16 elem_size;
	u16 cache_idx;
	bool use_kmalloc_nolock;
};

struct bpf_local_storage_data {
@@ -86,8 +85,7 @@ struct bpf_local_storage_elem {
						 */
	};
	atomic_t state;
	bool use_kmalloc_nolock;
	/* 3 bytes hole */
	/* 4 bytes hole */
	/* The data is stored in another cacheline to minimize
	 * the number of cachelines access during a cache hit.
	 */
@@ -104,7 +102,6 @@ struct bpf_local_storage {
	rqspinlock_t lock;	/* Protect adding/removing from the "list" */
	u64 mem_charge;		/* Copy of mem charged to owner. Protected by "lock" */
	refcount_t owner_refcnt;/* Used to pin owner when map_free is uncharging */
	bool use_kmalloc_nolock;
};

/* U16_MAX is much more than enough for sk local storage
@@ -137,8 +134,7 @@ int bpf_local_storage_map_alloc_check(union bpf_attr *attr);

struct bpf_map *
bpf_local_storage_map_alloc(union bpf_attr *attr,
			    struct bpf_local_storage_cache *cache,
			    bool use_kmalloc_nolock);
			    struct bpf_local_storage_cache *cache);

void __bpf_local_storage_insert_cache(struct bpf_local_storage *local_storage,
				      struct bpf_local_storage_map *smap,
@@ -192,7 +188,7 @@ int bpf_selem_link_map(struct bpf_local_storage_map *smap,

struct bpf_local_storage_elem *
bpf_selem_alloc(struct bpf_local_storage_map *smap, void *owner, void *value,
		bool swap_uptrs, gfp_t gfp_flags);
		bool swap_uptrs);

void bpf_selem_free(struct bpf_local_storage_elem *selem,
		    bool reuse_now);
@@ -200,12 +196,11 @@ void bpf_selem_free(struct bpf_local_storage_elem *selem,
int
bpf_local_storage_alloc(void *owner,
			struct bpf_local_storage_map *smap,
			struct bpf_local_storage_elem *first_selem,
			gfp_t gfp_flags);
			struct bpf_local_storage_elem *first_selem);

struct bpf_local_storage_data *
bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
			 void *value, u64 map_flags, bool swap_uptrs, gfp_t gfp_flags);
			 void *value, u64 map_flags, bool swap_uptrs);

u64 bpf_local_storage_map_mem_usage(const struct bpf_map *map);

+5 −6
Original line number Diff line number Diff line
@@ -76,7 +76,7 @@ static long bpf_cgrp_storage_update_elem(struct bpf_map *map, void *key,
		return PTR_ERR(cgroup);

	sdata = bpf_local_storage_update(cgroup, (struct bpf_local_storage_map *)map,
					 value, map_flags, false, GFP_ATOMIC);
					 value, map_flags, false);
	cgroup_put(cgroup);
	return PTR_ERR_OR_ZERO(sdata);
}
@@ -114,7 +114,7 @@ static int notsupp_get_next_key(struct bpf_map *map, void *key, void *next_key)

static struct bpf_map *cgroup_storage_map_alloc(union bpf_attr *attr)
{
	return bpf_local_storage_map_alloc(attr, &cgroup_cache, true);
	return bpf_local_storage_map_alloc(attr, &cgroup_cache);
}

static void cgroup_storage_map_free(struct bpf_map *map)
@@ -122,9 +122,8 @@ static void cgroup_storage_map_free(struct bpf_map *map)
	bpf_local_storage_map_free(map, &cgroup_cache);
}

/* *gfp_flags* is a hidden argument provided by the verifier */
BPF_CALL_5(bpf_cgrp_storage_get, struct bpf_map *, map, struct cgroup *, cgroup,
	   void *, value, u64, flags, gfp_t, gfp_flags)
BPF_CALL_4(bpf_cgrp_storage_get, struct bpf_map *, map, struct cgroup *, cgroup,
	   void *, value, u64, flags)
{
	struct bpf_local_storage_data *sdata;

@@ -143,7 +142,7 @@ BPF_CALL_5(bpf_cgrp_storage_get, struct bpf_map *, map, struct cgroup *, cgroup,
	if (!percpu_ref_is_dying(&cgroup->self.refcnt) &&
	    (flags & BPF_LOCAL_STORAGE_GET_F_CREATE))
		sdata = bpf_local_storage_update(cgroup, (struct bpf_local_storage_map *)map,
						 value, BPF_NOEXIST, false, gfp_flags);
						 value, BPF_NOEXIST, false);

out:
	return IS_ERR_OR_NULL(sdata) ? (unsigned long)NULL : (unsigned long)sdata->data;
+5 −6
Original line number Diff line number Diff line
@@ -98,7 +98,7 @@ static long bpf_fd_inode_storage_update_elem(struct bpf_map *map, void *key,

	sdata = bpf_local_storage_update(file_inode(fd_file(f)),
					 (struct bpf_local_storage_map *)map,
					 value, map_flags, false, GFP_ATOMIC);
					 value, map_flags, false);
	return PTR_ERR_OR_ZERO(sdata);
}

@@ -122,9 +122,8 @@ static long bpf_fd_inode_storage_delete_elem(struct bpf_map *map, void *key)
	return inode_storage_delete(file_inode(fd_file(f)), map);
}

/* *gfp_flags* is a hidden argument provided by the verifier */
BPF_CALL_5(bpf_inode_storage_get, struct bpf_map *, map, struct inode *, inode,
	   void *, value, u64, flags, gfp_t, gfp_flags)
BPF_CALL_4(bpf_inode_storage_get, struct bpf_map *, map, struct inode *, inode,
	   void *, value, u64, flags)
{
	struct bpf_local_storage_data *sdata;

@@ -150,7 +149,7 @@ BPF_CALL_5(bpf_inode_storage_get, struct bpf_map *, map, struct inode *, inode,
	if (flags & BPF_LOCAL_STORAGE_GET_F_CREATE) {
		sdata = bpf_local_storage_update(
			inode, (struct bpf_local_storage_map *)map, value,
			BPF_NOEXIST, false, gfp_flags);
			BPF_NOEXIST, false);
		return IS_ERR(sdata) ? (unsigned long)NULL :
					     (unsigned long)sdata->data;
	}
@@ -179,7 +178,7 @@ static int notsupp_get_next_key(struct bpf_map *map, void *key,

static struct bpf_map *inode_storage_map_alloc(union bpf_attr *attr)
{
	return bpf_local_storage_map_alloc(attr, &inode_cache, false);
	return bpf_local_storage_map_alloc(attr, &inode_cache);
}

static void inode_storage_map_free(struct bpf_map *map)
+21 −125
Original line number Diff line number Diff line
@@ -68,25 +68,19 @@ static bool selem_linked_to_map(const struct bpf_local_storage_elem *selem)

struct bpf_local_storage_elem *
bpf_selem_alloc(struct bpf_local_storage_map *smap, void *owner,
		void *value, bool swap_uptrs, gfp_t gfp_flags)
		void *value, bool swap_uptrs)
{
	struct bpf_local_storage_elem *selem;

	if (mem_charge(smap, owner, smap->elem_size))
		return NULL;

	if (smap->use_kmalloc_nolock) {
	selem = bpf_map_kmalloc_nolock(&smap->map, smap->elem_size,
				       __GFP_ZERO, NUMA_NO_NODE);
	} else {
		selem = bpf_map_kzalloc(&smap->map, smap->elem_size,
					gfp_flags | __GFP_NOWARN);
	}

	if (selem) {
		RCU_INIT_POINTER(SDATA(selem)->smap, smap);
		atomic_set(&selem->state, 0);
		selem->use_kmalloc_nolock = smap->use_kmalloc_nolock;

		if (value) {
			/* No need to call check_and_init_map_value as memory is zero init */
@@ -102,8 +96,7 @@ bpf_selem_alloc(struct bpf_local_storage_map *smap, void *owner,
	return NULL;
}

/* rcu tasks trace callback for use_kmalloc_nolock == false */
static void __bpf_local_storage_free_trace_rcu(struct rcu_head *rcu)
static void bpf_local_storage_free_trace_rcu(struct rcu_head *rcu)
{
	struct bpf_local_storage *local_storage;

@@ -115,47 +108,14 @@ static void __bpf_local_storage_free_trace_rcu(struct rcu_head *rcu)
	kfree(local_storage);
}

/* Handle use_kmalloc_nolock == false */
static void __bpf_local_storage_free(struct bpf_local_storage *local_storage,
				     bool vanilla_rcu)
{
	if (vanilla_rcu)
		kfree_rcu(local_storage, rcu);
	else
		call_rcu_tasks_trace(&local_storage->rcu,
				     __bpf_local_storage_free_trace_rcu);
}

static void bpf_local_storage_free_rcu(struct rcu_head *rcu)
{
	struct bpf_local_storage *local_storage;

	local_storage = container_of(rcu, struct bpf_local_storage, rcu);
	kfree_nolock(local_storage);
}

static void bpf_local_storage_free_trace_rcu(struct rcu_head *rcu)
{
	/*
	 * RCU Tasks Trace grace period implies RCU grace period, do
	 * kfree() directly.
	 */
	bpf_local_storage_free_rcu(rcu);
}

static void bpf_local_storage_free(struct bpf_local_storage *local_storage,
				   bool reuse_now)
{
	if (!local_storage)
		return;

	if (!local_storage->use_kmalloc_nolock) {
		__bpf_local_storage_free(local_storage, reuse_now);
		return;
	}

	if (reuse_now) {
		call_rcu(&local_storage->rcu, bpf_local_storage_free_rcu);
		kfree_rcu(local_storage, rcu);
		return;
	}

@@ -163,83 +123,35 @@ static void bpf_local_storage_free(struct bpf_local_storage *local_storage,
			     bpf_local_storage_free_trace_rcu);
}

/* rcu callback for use_kmalloc_nolock == false */
static void __bpf_selem_free_rcu(struct rcu_head *rcu)
static void bpf_selem_free_trace_rcu(struct rcu_head *rcu)
{
	struct bpf_local_storage_elem *selem;
	struct bpf_local_storage_map *smap;

	selem = container_of(rcu, struct bpf_local_storage_elem, rcu);
	/* bpf_selem_unlink_nofail may have already cleared smap and freed fields. */
	/* The bpf_local_storage_map_free will wait for rcu_barrier */
	smap = rcu_dereference_check(SDATA(selem)->smap, 1);

	if (smap)
		bpf_obj_free_fields(smap->map.record, SDATA(selem)->data);
	kfree(selem);
}

/* rcu tasks trace callback for use_kmalloc_nolock == false */
static void __bpf_selem_free_trace_rcu(struct rcu_head *rcu)
{
	/*
	 * RCU Tasks Trace grace period implies RCU grace period, do
	 * kfree() directly.
	 */
	__bpf_selem_free_rcu(rcu);
}

/* Handle use_kmalloc_nolock == false */
static void __bpf_selem_free(struct bpf_local_storage_elem *selem,
			     bool vanilla_rcu)
{
	if (vanilla_rcu)
		call_rcu(&selem->rcu, __bpf_selem_free_rcu);
	else
		call_rcu_tasks_trace(&selem->rcu, __bpf_selem_free_trace_rcu);
	kfree(selem);
}

static void bpf_selem_free_rcu(struct rcu_head *rcu)
void bpf_selem_free(struct bpf_local_storage_elem *selem,
		    bool reuse_now)
{
	struct bpf_local_storage_elem *selem;
	struct bpf_local_storage_map *smap;

	selem = container_of(rcu, struct bpf_local_storage_elem, rcu);
	/* The bpf_local_storage_map_free will wait for rcu_barrier */
	smap = rcu_dereference_check(SDATA(selem)->smap, 1);

	if (reuse_now) {
		if (smap)
			bpf_obj_free_fields(smap->map.record, SDATA(selem)->data);
	kfree_nolock(selem);
}

static void bpf_selem_free_trace_rcu(struct rcu_head *rcu)
{
	/*
	 * RCU Tasks Trace grace period implies RCU grace period, do
	 * kfree() directly.
	 */
	bpf_selem_free_rcu(rcu);
}

void bpf_selem_free(struct bpf_local_storage_elem *selem,
		    bool reuse_now)
{
	if (!selem->use_kmalloc_nolock) {
		/*
		 * No uptr will be unpin even when reuse_now == false since uptr
		 * is only supported in task local storage, where
		 * smap->use_kmalloc_nolock == true.
		 */
		__bpf_selem_free(selem, reuse_now);
		return;
	}

	if (reuse_now) {
		/*
		 * While it is okay to call bpf_obj_free_fields() that unpins uptr when
		 * reuse_now == true, keep it in bpf_selem_free_rcu() for simplicity.
		 */
		call_rcu(&selem->rcu, bpf_selem_free_rcu);
		kfree_rcu(selem, rcu);
		return;
	}

@@ -563,8 +475,7 @@ static int check_flags(const struct bpf_local_storage_data *old_sdata,

int bpf_local_storage_alloc(void *owner,
			    struct bpf_local_storage_map *smap,
			    struct bpf_local_storage_elem *first_selem,
			    gfp_t gfp_flags)
			    struct bpf_local_storage_elem *first_selem)
{
	struct bpf_local_storage *prev_storage, *storage;
	struct bpf_local_storage **owner_storage_ptr;
@@ -576,12 +487,8 @@ int bpf_local_storage_alloc(void *owner,
	if (err)
		return err;

	if (smap->use_kmalloc_nolock)
	storage = bpf_map_kmalloc_nolock(&smap->map, sizeof(*storage),
					 __GFP_ZERO, NUMA_NO_NODE);
	else
		storage = bpf_map_kzalloc(&smap->map, sizeof(*storage),
					  gfp_flags | __GFP_NOWARN);
	if (!storage) {
		err = -ENOMEM;
		goto uncharge;
@@ -591,7 +498,6 @@ int bpf_local_storage_alloc(void *owner,
	raw_res_spin_lock_init(&storage->lock);
	storage->owner = owner;
	storage->mem_charge = sizeof(*storage);
	storage->use_kmalloc_nolock = smap->use_kmalloc_nolock;
	refcount_set(&storage->owner_refcnt, 1);

	bpf_selem_link_storage_nolock(storage, first_selem);
@@ -639,7 +545,7 @@ int bpf_local_storage_alloc(void *owner,
 */
struct bpf_local_storage_data *
bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
			 void *value, u64 map_flags, bool swap_uptrs, gfp_t gfp_flags)
			 void *value, u64 map_flags, bool swap_uptrs)
{
	struct bpf_local_storage_data *old_sdata = NULL;
	struct bpf_local_storage_elem *alloc_selem, *selem = NULL;
@@ -656,9 +562,6 @@ bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
		     !btf_record_has_field(smap->map.record, BPF_SPIN_LOCK)))
		return ERR_PTR(-EINVAL);

	if (gfp_flags == GFP_KERNEL && (map_flags & ~BPF_F_LOCK) != BPF_NOEXIST)
		return ERR_PTR(-EINVAL);

	local_storage = rcu_dereference_check(*owner_storage(smap, owner),
					      bpf_rcu_lock_held());
	if (!local_storage || hlist_empty(&local_storage->list)) {
@@ -667,11 +570,11 @@ bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
		if (err)
			return ERR_PTR(err);

		selem = bpf_selem_alloc(smap, owner, value, swap_uptrs, gfp_flags);
		selem = bpf_selem_alloc(smap, owner, value, swap_uptrs);
		if (!selem)
			return ERR_PTR(-ENOMEM);

		err = bpf_local_storage_alloc(owner, smap, selem, gfp_flags);
		err = bpf_local_storage_alloc(owner, smap, selem);
		if (err) {
			bpf_selem_free(selem, true);
			mem_uncharge(smap, owner, smap->elem_size);
@@ -701,7 +604,7 @@ bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
	/* A lookup has just been done before and concluded a new selem is
	 * needed. The chance of an unnecessary alloc is unlikely.
	 */
	alloc_selem = selem = bpf_selem_alloc(smap, owner, value, swap_uptrs, gfp_flags);
	alloc_selem = selem = bpf_selem_alloc(smap, owner, value, swap_uptrs);
	if (!alloc_selem)
		return ERR_PTR(-ENOMEM);

@@ -868,8 +771,7 @@ u64 bpf_local_storage_map_mem_usage(const struct bpf_map *map)

struct bpf_map *
bpf_local_storage_map_alloc(union bpf_attr *attr,
			    struct bpf_local_storage_cache *cache,
			    bool use_kmalloc_nolock)
			    struct bpf_local_storage_cache *cache)
{
	struct bpf_local_storage_map *smap;
	unsigned int i;
@@ -901,12 +803,6 @@ bpf_local_storage_map_alloc(union bpf_attr *attr,
	smap->elem_size = offsetof(struct bpf_local_storage_elem,
				   sdata.data[attr->value_size]);

	/* In PREEMPT_RT, kmalloc(GFP_ATOMIC) is still not safe in non
	 * preemptible context. Thus, enforce all storages to use
	 * kmalloc_nolock() when CONFIG_PREEMPT_RT is enabled.
	 */
	smap->use_kmalloc_nolock = IS_ENABLED(CONFIG_PREEMPT_RT) ? true : use_kmalloc_nolock;

	smap->cache_idx = bpf_local_storage_cache_idx_get(cache);
	return &smap->map;

+5 −6
Original line number Diff line number Diff line
@@ -118,7 +118,7 @@ static long bpf_pid_task_storage_update_elem(struct bpf_map *map, void *key,

	sdata = bpf_local_storage_update(
		task, (struct bpf_local_storage_map *)map, value, map_flags,
		true, GFP_ATOMIC);
		true);

	err = PTR_ERR_OR_ZERO(sdata);
out:
@@ -165,9 +165,8 @@ static long bpf_pid_task_storage_delete_elem(struct bpf_map *map, void *key)
	return err;
}

/* *gfp_flags* is a hidden argument provided by the verifier */
BPF_CALL_5(bpf_task_storage_get, struct bpf_map *, map, struct task_struct *,
	   task, void *, value, u64, flags, gfp_t, gfp_flags)
BPF_CALL_4(bpf_task_storage_get, struct bpf_map *, map, struct task_struct *,
	   task, void *, value, u64, flags)
{
	struct bpf_local_storage_data *sdata;

@@ -184,7 +183,7 @@ BPF_CALL_5(bpf_task_storage_get, struct bpf_map *, map, struct task_struct *,
	    (flags & BPF_LOCAL_STORAGE_GET_F_CREATE)) {
		sdata = bpf_local_storage_update(
			task, (struct bpf_local_storage_map *)map, value,
			BPF_NOEXIST, false, gfp_flags);
			BPF_NOEXIST, false);
		return IS_ERR(sdata) ? (unsigned long)NULL : (unsigned long)sdata->data;
	}

@@ -212,7 +211,7 @@ static int notsupp_get_next_key(struct bpf_map *map, void *key, void *next_key)

static struct bpf_map *task_storage_map_alloc(union bpf_attr *attr)
{
	return bpf_local_storage_map_alloc(attr, &task_cache, true);
	return bpf_local_storage_map_alloc(attr, &task_cache);
}

static void task_storage_map_free(struct bpf_map *map)
Loading