[PATCH bpf-next v2 0/2] bpf: Minor fixes for non-preallocated memory

[PATCH bpf-next v2 0/2] bpf: Minor fixes for non-preallocated memory

[Yafang Shao]

When I was implementing bpf recharge[1], I found some other issues.
These issues are independent, so I send them separately.

[1]. https://lore.kernel.org/bpf/20220619155032.32515-1-laoar.shao@gmail.com/

v2:
- fix GFP_HIGH consistently over the bpf code. (Daniel, Roman)
- get rid of an error patch (Hao)

Yafang Shao (2):
  bpf: Make non-preallocated allocation low priority
  bpf: Warn on non-preallocated case for missed trace types

 kernel/bpf/devmap.c        | 3 ++-
 kernel/bpf/hashtab.c       | 8 +++++---
 kernel/bpf/local_storage.c | 3 ++-
 kernel/bpf/lpm_trie.c      | 3 ++-
 kernel/bpf/verifier.c      | 2 ++
 5 files changed, 13 insertions(+), 6 deletions(-)

-- 
2.17.1

[PATCH bpf-next v2 1/2] bpf: Make non-preallocated allocation low priority

[Yafang Shao]

GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially
if we allocate too much GFP_ATOMIC memory. For example, when we set the
memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can
easily break the memcg limit by force charge. So it is very dangerous to
use GFP_ATOMIC in non-preallocated case. One way to make it safe is to
remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC |
__GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate
too much memory.

We introduced BPF_F_NO_PREALLOC is because full map pre-allocation is
too memory expensive for some cases. That means removing __GFP_HIGH
doesn't break the rule of BPF_F_NO_PREALLOC, but has the same goal with
it-avoiding issues caused by too much memory. So let's remove it.

The force charge of GFP_ATOMIC was introduced in
commit 869712fd3de5 ("mm: memcontrol: fix network errors from failing
__GFP_ATOMIC charges") by checking __GFP_ATOMIC, then got improved in
commit 1461e8c2b6af ("memcg: unify force charging conditions") by
checking __GFP_HIGH (that is no problem because both __GFP_HIGH and
__GFP_ATOMIC are set in GFP_AOMIC). So, if we want to fix it in memcg,
we have to carefully verify all the callsites. Now that we can fix it in
BPF, we'd better not modify the memcg code.

This fix can also apply to other run-time allocations, for example, the
allocation in lpm trie, local storage and devmap. So let fix it
consistently over the bpf code

__GFP_KSWAPD_RECLAIM doesn't cooperate well with memcg pressure neither
currently. But the memcg code can be improved to make
__GFP_KSWAPD_RECLAIM work well under memcg pressure if desired.

It also fixes a typo in the comment.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>
---
 kernel/bpf/devmap.c        | 3 ++-
 kernel/bpf/hashtab.c       | 8 +++++---
 kernel/bpf/local_storage.c | 3 ++-
 kernel/bpf/lpm_trie.c      | 3 ++-
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index c2867068e5bd..7672946126d5 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -845,7 +845,8 @@ static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
 	struct bpf_dtab_netdev *dev;
 
 	dev = bpf_map_kmalloc_node(&dtab->map, sizeof(*dev),
-				   GFP_ATOMIC | __GFP_NOWARN,
+				   __GFP_ATOMIC | __GFP_NOWARN |
+				   __GFP_KSWAPD_RECLAIM,
 				   dtab->map.numa_node);
 	if (!dev)
 		return ERR_PTR(-ENOMEM);
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 17fb69c0e0dc..9d4559a1c032 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -61,7 +61,7 @@
  *
  * As regular device interrupt handlers and soft interrupts are forced into
  * thread context, the existing code which does
- *   spin_lock*(); alloc(GPF_ATOMIC); spin_unlock*();
+ *   spin_lock*(); alloc(GFP_ATOMIC); spin_unlock*();
  * just works.
  *
  * In theory the BPF locks could be converted to regular spinlocks as well,
@@ -978,7 +978,8 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key,
 				goto dec_count;
 			}
 		l_new = bpf_map_kmalloc_node(&htab->map, htab->elem_size,
-					     GFP_ATOMIC | __GFP_NOWARN,
+					     __GFP_ATOMIC | __GFP_NOWARN |
+					     __GFP_KSWAPD_RECLAIM,
 					     htab->map.numa_node);
 		if (!l_new) {
 			l_new = ERR_PTR(-ENOMEM);
@@ -996,7 +997,8 @@ static struct htab_elem *alloc_htab_elem(struct bpf_htab *htab, void *key,
 		} else {
 			/* alloc_percpu zero-fills */
 			pptr = bpf_map_alloc_percpu(&htab->map, size, 8,
-						    GFP_ATOMIC | __GFP_NOWARN);
+						    __GFP_ATOMIC | __GFP_NOWARN |
+						    __GFP_KSWAPD_RECLAIM);
 			if (!pptr) {
 				kfree(l_new);
 				l_new = ERR_PTR(-ENOMEM);
diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
index 8654fc97f5fe..534b69682b17 100644
--- a/kernel/bpf/local_storage.c
+++ b/kernel/bpf/local_storage.c
@@ -165,7 +165,8 @@ static int cgroup_storage_update_elem(struct bpf_map *map, void *key,
 	}
 
 	new = bpf_map_kmalloc_node(map, struct_size(new, data, map->value_size),
-				   __GFP_ZERO | GFP_ATOMIC | __GFP_NOWARN,
+				   __GFP_ZERO | __GFP_ATOMIC | __GFP_NOWARN |
+				   __GFP_KSWAPD_RECLAIM,
 				   map->numa_node);
 	if (!new)
 		return -ENOMEM;
diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
index f0d05a3cc4b9..7bae7133f1dd 100644
--- a/kernel/bpf/lpm_trie.c
+++ b/kernel/bpf/lpm_trie.c
@@ -285,7 +285,8 @@ static struct lpm_trie_node *lpm_trie_node_alloc(const struct lpm_trie *trie,
 	if (value)
 		size += trie->map.value_size;
 
-	node = bpf_map_kmalloc_node(&trie->map, size, GFP_ATOMIC | __GFP_NOWARN,
+	node = bpf_map_kmalloc_node(&trie->map, size, __GFP_ATOMIC |
+				    __GFP_KSWAPD_RECLAIM | __GFP_NOWARN,
 				    trie->map.numa_node);
 	if (!node)
 		return NULL;
-- 
2.17.1

[PATCH bpf-next v2 2/2] bpf: Warn on non-preallocated case for missed trace types

[Yafang Shao]

BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE and BPF_PROG_TYPE_TRACING are
trace type as well, which may also cause unexpected memory allocation if
we set BPF_F_NO_PREALLOC.
Let's also warn on both of them.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 kernel/bpf/verifier.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index df3ec6b05f05..f9c0f4889a3a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12570,6 +12570,8 @@ static bool is_tracing_prog_type(enum bpf_prog_type type)
 	case BPF_PROG_TYPE_TRACEPOINT:
 	case BPF_PROG_TYPE_PERF_EVENT:
 	case BPF_PROG_TYPE_RAW_TRACEPOINT:
+	case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE:
+	case BPF_PROG_TYPE_TRACING:
 		return true;
 	default:
 		return false;
-- 
2.17.1

Re: [PATCH bpf-next v2 1/2] bpf: Make non-preallocated allocation low priority

[Alexei Starovoitov]

On Wed, Jul 6, 2022 at 8:59 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially
> if we allocate too much GFP_ATOMIC memory. For example, when we set the
> memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can
> easily break the memcg limit by force charge. So it is very dangerous to
> use GFP_ATOMIC in non-preallocated case. One way to make it safe is to
> remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC |
> __GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate
> too much memory.
>
> We introduced BPF_F_NO_PREALLOC is because full map pre-allocation is
> too memory expensive for some cases. That means removing __GFP_HIGH
> doesn't break the rule of BPF_F_NO_PREALLOC, but has the same goal with
> it-avoiding issues caused by too much memory. So let's remove it.
>
> The force charge of GFP_ATOMIC was introduced in
> commit 869712fd3de5 ("mm: memcontrol: fix network errors from failing
> __GFP_ATOMIC charges") by checking __GFP_ATOMIC, then got improved in
> commit 1461e8c2b6af ("memcg: unify force charging conditions") by
> checking __GFP_HIGH (that is no problem because both __GFP_HIGH and
> __GFP_ATOMIC are set in GFP_AOMIC). So, if we want to fix it in memcg,
> we have to carefully verify all the callsites. Now that we can fix it in
> BPF, we'd better not modify the memcg code.
>
> This fix can also apply to other run-time allocations, for example, the
> allocation in lpm trie, local storage and devmap. So let fix it
> consistently over the bpf code
>
> __GFP_KSWAPD_RECLAIM doesn't cooperate well with memcg pressure neither
> currently. But the memcg code can be improved to make
> __GFP_KSWAPD_RECLAIM work well under memcg pressure if desired.

Could you elaborate ?

> It also fixes a typo in the comment.
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>

Roman, do you agree with this change ?

Re: [PATCH bpf-next v2 2/2] bpf: Warn on non-preallocated case for missed trace types

[Alexei Starovoitov]

On Wed, Jul 6, 2022 at 8:59 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE and BPF_PROG_TYPE_TRACING are
> trace type as well, which may also cause unexpected memory allocation if
> we set BPF_F_NO_PREALLOC.
> Let's also warn on both of them.
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>  kernel/bpf/verifier.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index df3ec6b05f05..f9c0f4889a3a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -12570,6 +12570,8 @@ static bool is_tracing_prog_type(enum bpf_prog_type type)
>         case BPF_PROG_TYPE_TRACEPOINT:
>         case BPF_PROG_TYPE_PERF_EVENT:
>         case BPF_PROG_TYPE_RAW_TRACEPOINT:
> +       case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE:
> +       case BPF_PROG_TYPE_TRACING:

BPF_TRACE_ITER should probably be excluded.

Re: [PATCH bpf-next v2 1/2] bpf: Make non-preallocated allocation low priority

[Roman Gushchin]

On Wed, Jul 06, 2022 at 09:47:32AM -0700, Alexei Starovoitov wrote:
> On Wed, Jul 6, 2022 at 8:59 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially
> > if we allocate too much GFP_ATOMIC memory. For example, when we set the
> > memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can
> > easily break the memcg limit by force charge. So it is very dangerous to
> > use GFP_ATOMIC in non-preallocated case. One way to make it safe is to
> > remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC |
> > __GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate
> > too much memory.
> >
> > We introduced BPF_F_NO_PREALLOC is because full map pre-allocation is
> > too memory expensive for some cases. That means removing __GFP_HIGH
> > doesn't break the rule of BPF_F_NO_PREALLOC, but has the same goal with
> > it-avoiding issues caused by too much memory. So let's remove it.
> >
> > The force charge of GFP_ATOMIC was introduced in
> > commit 869712fd3de5 ("mm: memcontrol: fix network errors from failing
> > __GFP_ATOMIC charges") by checking __GFP_ATOMIC, then got improved in
> > commit 1461e8c2b6af ("memcg: unify force charging conditions") by
> > checking __GFP_HIGH (that is no problem because both __GFP_HIGH and
> > __GFP_ATOMIC are set in GFP_AOMIC). So, if we want to fix it in memcg,
> > we have to carefully verify all the callsites. Now that we can fix it in
> > BPF, we'd better not modify the memcg code.
> >
> > This fix can also apply to other run-time allocations, for example, the
> > allocation in lpm trie, local storage and devmap. So let fix it
> > consistently over the bpf code
> >
> > __GFP_KSWAPD_RECLAIM doesn't cooperate well with memcg pressure neither
> > currently. But the memcg code can be improved to make
> > __GFP_KSWAPD_RECLAIM work well under memcg pressure if desired.
> 
> Could you elaborate ?
> 
> > It also fixes a typo in the comment.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>
> 
> Roman, do you agree with this change ?

Yes, removing __GFP_HIGH makes sense to me. I can imagine we might want
it for *some* bpf allocations, but applying it unconditionally looks wrong.

Thanks!

Re: [PATCH bpf-next v2 1/2] bpf: Make non-preallocated allocation low priority

[Alexei Starovoitov]

On Wed, Jul 6, 2022 at 12:09 PM Roman Gushchin <roman.gushchin@linux.dev> wrote:
>
> On Wed, Jul 06, 2022 at 09:47:32AM -0700, Alexei Starovoitov wrote:
> > On Wed, Jul 6, 2022 at 8:59 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > >
> > > GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially
> > > if we allocate too much GFP_ATOMIC memory. For example, when we set the
> > > memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can
> > > easily break the memcg limit by force charge. So it is very dangerous to
> > > use GFP_ATOMIC in non-preallocated case. One way to make it safe is to
> > > remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC |
> > > __GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate
> > > too much memory.
> > >
> > > We introduced BPF_F_NO_PREALLOC is because full map pre-allocation is
> > > too memory expensive for some cases. That means removing __GFP_HIGH
> > > doesn't break the rule of BPF_F_NO_PREALLOC, but has the same goal with
> > > it-avoiding issues caused by too much memory. So let's remove it.
> > >
> > > The force charge of GFP_ATOMIC was introduced in
> > > commit 869712fd3de5 ("mm: memcontrol: fix network errors from failing
> > > __GFP_ATOMIC charges") by checking __GFP_ATOMIC, then got improved in
> > > commit 1461e8c2b6af ("memcg: unify force charging conditions") by
> > > checking __GFP_HIGH (that is no problem because both __GFP_HIGH and
> > > __GFP_ATOMIC are set in GFP_AOMIC). So, if we want to fix it in memcg,
> > > we have to carefully verify all the callsites. Now that we can fix it in
> > > BPF, we'd better not modify the memcg code.
> > >
> > > This fix can also apply to other run-time allocations, for example, the
> > > allocation in lpm trie, local storage and devmap. So let fix it
> > > consistently over the bpf code
> > >
> > > __GFP_KSWAPD_RECLAIM doesn't cooperate well with memcg pressure neither
> > > currently. But the memcg code can be improved to make
> > > __GFP_KSWAPD_RECLAIM work well under memcg pressure if desired.
> >
> > Could you elaborate ?
> >
> > > It also fixes a typo in the comment.
> > >
> > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > > Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>
> >
> > Roman, do you agree with this change ?
>
> Yes, removing __GFP_HIGH makes sense to me. I can imagine we might want
> it for *some* bpf allocations, but applying it unconditionally looks wrong.

Yeah. It's a difficult trade-off to make without having the data
to decide whether removing __GFP_HIGH can cause issues or not,
but do you agree that __GFP_HIGH doesn't cooperate well with memcg ?
If so it's a bug on memcg side, right? but we should probably
apply this band-aid on bpf side to fix the bleeding.
Later we can add a knob to allow __GFP_HIGH usage on demand from
bpf prog.

Re: [PATCH bpf-next v2 1/2] bpf: Make non-preallocated allocation low priority

[Roman Gushchin]

On Wed, Jul 06, 2022 at 03:11:46PM -0700, Alexei Starovoitov wrote:
> On Wed, Jul 6, 2022 at 12:09 PM Roman Gushchin <roman.gushchin@linux.dev> wrote:
> >
> > On Wed, Jul 06, 2022 at 09:47:32AM -0700, Alexei Starovoitov wrote:
> > > On Wed, Jul 6, 2022 at 8:59 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > > >
> > > > GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially
> > > > if we allocate too much GFP_ATOMIC memory. For example, when we set the
> > > > memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can
> > > > easily break the memcg limit by force charge. So it is very dangerous to
> > > > use GFP_ATOMIC in non-preallocated case. One way to make it safe is to
> > > > remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC |
> > > > __GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate
> > > > too much memory.
> > > >
> > > > We introduced BPF_F_NO_PREALLOC is because full map pre-allocation is
> > > > too memory expensive for some cases. That means removing __GFP_HIGH
> > > > doesn't break the rule of BPF_F_NO_PREALLOC, but has the same goal with
> > > > it-avoiding issues caused by too much memory. So let's remove it.
> > > >
> > > > The force charge of GFP_ATOMIC was introduced in
> > > > commit 869712fd3de5 ("mm: memcontrol: fix network errors from failing
> > > > __GFP_ATOMIC charges") by checking __GFP_ATOMIC, then got improved in
> > > > commit 1461e8c2b6af ("memcg: unify force charging conditions") by
> > > > checking __GFP_HIGH (that is no problem because both __GFP_HIGH and
> > > > __GFP_ATOMIC are set in GFP_AOMIC). So, if we want to fix it in memcg,
> > > > we have to carefully verify all the callsites. Now that we can fix it in
> > > > BPF, we'd better not modify the memcg code.
> > > >
> > > > This fix can also apply to other run-time allocations, for example, the
> > > > allocation in lpm trie, local storage and devmap. So let fix it
> > > > consistently over the bpf code
> > > >
> > > > __GFP_KSWAPD_RECLAIM doesn't cooperate well with memcg pressure neither
> > > > currently. But the memcg code can be improved to make
> > > > __GFP_KSWAPD_RECLAIM work well under memcg pressure if desired.
> > >
> > > Could you elaborate ?
> > >
> > > > It also fixes a typo in the comment.
> > > >
> > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > > > Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>
> > >
> > > Roman, do you agree with this change ?
> >
> > Yes, removing __GFP_HIGH makes sense to me. I can imagine we might want
> > it for *some* bpf allocations, but applying it unconditionally looks wrong.
> 
> Yeah. It's a difficult trade-off to make without having the data
> to decide whether removing __GFP_HIGH can cause issues or not,

Yeah, the change looks reasonable, but it's hard to say without giving
it a good testing in (something close to) a production environment.

> but do you agree that __GFP_HIGH doesn't cooperate well with memcg ?
> If so it's a bug on memcg side, right?

No. Historically we allowed high-prio allocations to exceed the memcg limit
because otherwise there were too many stability and performance issues.
It's not a memcg bug, it's a way to avoid exposing ENOMEM handling bugs all over
the kernel code. Without memory cgroups GFP_ATOMIC allocations rarely fail
and a lot of code paths in the kernel are not really ready for it (at least
it was the case several years ago, maybe things are better now).

But it was usually thought in the context of small(ish) allocations which do not
change the global memory usage picture. Subsequent "normal" allocations are
triggering reclaim/OOM, so from a user's POV the limit works as expected.

But with the ownership model and size of bpf maps it's a different story:
if a bpf map belongs to an abandoned cgroup, it might consume a lot of memory
and there will be no "normal" allocations. So cgroup memory limit will be never
applied. It's a valid issue, I agree with Yafang here.

> but we should probably
> apply this band-aid on bpf side to fix the bleeding.
> Later we can add a knob to allow __GFP_HIGH usage on demand from
> bpf prog.

Yes, it sounds like a good idea. I have to think what's the best approach
here, it's not obvious for me.

Thanks!

Re: [PATCH bpf-next v2 1/2] bpf: Make non-preallocated allocation low priority

[Alexei Starovoitov]

On Wed, Jul 6, 2022 at 3:54 PM Roman Gushchin <roman.gushchin@linux.dev> wrote:
>
> On Wed, Jul 06, 2022 at 03:11:46PM -0700, Alexei Starovoitov wrote:
> > On Wed, Jul 6, 2022 at 12:09 PM Roman Gushchin <roman.gushchin@linux.dev> wrote:
> > >
> > > On Wed, Jul 06, 2022 at 09:47:32AM -0700, Alexei Starovoitov wrote:
> > > > On Wed, Jul 6, 2022 at 8:59 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > > > >
> > > > > GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially
> > > > > if we allocate too much GFP_ATOMIC memory. For example, when we set the
> > > > > memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can
> > > > > easily break the memcg limit by force charge. So it is very dangerous to
> > > > > use GFP_ATOMIC in non-preallocated case. One way to make it safe is to
> > > > > remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC |
> > > > > __GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate
> > > > > too much memory.
> > > > >
> > > > > We introduced BPF_F_NO_PREALLOC is because full map pre-allocation is
> > > > > too memory expensive for some cases. That means removing __GFP_HIGH
> > > > > doesn't break the rule of BPF_F_NO_PREALLOC, but has the same goal with
> > > > > it-avoiding issues caused by too much memory. So let's remove it.
> > > > >
> > > > > The force charge of GFP_ATOMIC was introduced in
> > > > > commit 869712fd3de5 ("mm: memcontrol: fix network errors from failing
> > > > > __GFP_ATOMIC charges") by checking __GFP_ATOMIC, then got improved in
> > > > > commit 1461e8c2b6af ("memcg: unify force charging conditions") by
> > > > > checking __GFP_HIGH (that is no problem because both __GFP_HIGH and
> > > > > __GFP_ATOMIC are set in GFP_AOMIC). So, if we want to fix it in memcg,
> > > > > we have to carefully verify all the callsites. Now that we can fix it in
> > > > > BPF, we'd better not modify the memcg code.
> > > > >
> > > > > This fix can also apply to other run-time allocations, for example, the
> > > > > allocation in lpm trie, local storage and devmap. So let fix it
> > > > > consistently over the bpf code
> > > > >
> > > > > __GFP_KSWAPD_RECLAIM doesn't cooperate well with memcg pressure neither
> > > > > currently. But the memcg code can be improved to make
> > > > > __GFP_KSWAPD_RECLAIM work well under memcg pressure if desired.
> > > >
> > > > Could you elaborate ?
> > > >
> > > > > It also fixes a typo in the comment.
> > > > >
> > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > > > > Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>
> > > >
> > > > Roman, do you agree with this change ?
> > >
> > > Yes, removing __GFP_HIGH makes sense to me. I can imagine we might want
> > > it for *some* bpf allocations, but applying it unconditionally looks wrong.
> >
> > Yeah. It's a difficult trade-off to make without having the data
> > to decide whether removing __GFP_HIGH can cause issues or not,
>
> Yeah, the change looks reasonable, but it's hard to say without giving
> it a good testing in (something close to) a production environment.
>
> > but do you agree that __GFP_HIGH doesn't cooperate well with memcg ?
> > If so it's a bug on memcg side, right?
>
> No. Historically we allowed high-prio allocations to exceed the memcg limit
> because otherwise there were too many stability and performance issues.
> It's not a memcg bug, it's a way to avoid exposing ENOMEM handling bugs all over
> the kernel code. Without memory cgroups GFP_ATOMIC allocations rarely fail
> and a lot of code paths in the kernel are not really ready for it (at least
> it was the case several years ago, maybe things are better now).
>
> But it was usually thought in the context of small(ish) allocations which do not
> change the global memory usage picture. Subsequent "normal" allocations are
> triggering reclaim/OOM, so from a user's POV the limit works as expected.
>
> But with the ownership model and size of bpf maps it's a different story:
> if a bpf map belongs to an abandoned cgroup, it might consume a lot of memory
> and there will be no "normal" allocations. So cgroup memory limit will be never
> applied. It's a valid issue, I agree with Yafang here.

Understood.

> > but we should probably
> > apply this band-aid on bpf side to fix the bleeding.
> > Later we can add a knob to allow __GFP_HIGH usage on demand from
> > bpf prog.
>
> Yes, it sounds like a good idea. I have to think what's the best approach
> here, it's not obvious for me.

Ok. Applied this patch for now.

Re: [PATCH bpf-next v2 1/2] bpf: Make non-preallocated allocation low priority

[Shakeel Butt]

On Wed, Jul 06, 2022 at 03:58:47PM +0000, Yafang Shao wrote:
> GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially
> if we allocate too much GFP_ATOMIC memory. For example, when we set the
> memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can
> easily break the memcg limit by force charge. So it is very dangerous to
> use GFP_ATOMIC in non-preallocated case. One way to make it safe is to
> remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC |
> __GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate
> too much memory.

Please use GFP_NOWAIT instead of (__GFP_ATOMIC | __GFP_KSWAPD_RECLAIM).
There is already a plan to completely remove __GFP_ATOMIC and mm-tree
already have a patch for that.

> 
> We introduced BPF_F_NO_PREALLOC is because full map pre-allocation is
> too memory expensive for some cases. That means removing __GFP_HIGH
> doesn't break the rule of BPF_F_NO_PREALLOC, but has the same goal with
> it-avoiding issues caused by too much memory. So let's remove it.
> 
> The force charge of GFP_ATOMIC was introduced in
> commit 869712fd3de5 ("mm: memcontrol: fix network errors from failing
> __GFP_ATOMIC charges") by checking __GFP_ATOMIC, then got improved in
> commit 1461e8c2b6af ("memcg: unify force charging conditions") by
> checking __GFP_HIGH (that is no problem because both __GFP_HIGH and
> __GFP_ATOMIC are set in GFP_AOMIC). So, if we want to fix it in memcg,
> we have to carefully verify all the callsites. Now that we can fix it in
> BPF, we'd better not modify the memcg code.
> 
> This fix can also apply to other run-time allocations, for example, the
> allocation in lpm trie, local storage and devmap. So let fix it
> consistently over the bpf code
> 
> __GFP_KSWAPD_RECLAIM doesn't cooperate well with memcg pressure neither
> currently. But the memcg code can be improved to make
> __GFP_KSWAPD_RECLAIM work well under memcg pressure if desired.
> 

IMO there is no need to give all this detail and background on
GFP_ATOMIC and __GFP_KSWAPD_RECLAIM. Just say kernel allows GFP_ATOMIC
allocations to exceed memcg limits which we don't want in this case. So,
replace with GFP_NOWAIT which obey memcg limits. Both of these flags
tell kernel that the caller can not sleep.

Re: [PATCH bpf-next v2 1/2] bpf: Make non-preallocated allocation low priority

[Alexei Starovoitov]

On Wed, Jul 6, 2022 at 5:07 PM Shakeel Butt <shakeelb@google.com> wrote:
>
> On Wed, Jul 06, 2022 at 03:58:47PM +0000, Yafang Shao wrote:
> > GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially
> > if we allocate too much GFP_ATOMIC memory. For example, when we set the
> > memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can
> > easily break the memcg limit by force charge. So it is very dangerous to
> > use GFP_ATOMIC in non-preallocated case. One way to make it safe is to
> > remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC |
> > __GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate
> > too much memory.
>
> Please use GFP_NOWAIT instead of (__GFP_ATOMIC | __GFP_KSWAPD_RECLAIM).
> There is already a plan to completely remove __GFP_ATOMIC and mm-tree
> already have a patch for that.

hmm. ok. reverted.

Re: [PATCH bpf-next v2 1/2] bpf: Make non-preallocated allocation low priority

[Roman Gushchin]

On Thu, Jul 07, 2022 at 12:07:21AM +0000, Shakeel Butt wrote:
> On Wed, Jul 06, 2022 at 03:58:47PM +0000, Yafang Shao wrote:
> > GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially
> > if we allocate too much GFP_ATOMIC memory. For example, when we set the
> > memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can
> > easily break the memcg limit by force charge. So it is very dangerous to
> > use GFP_ATOMIC in non-preallocated case. One way to make it safe is to
> > remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC |
> > __GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate
> > too much memory.
> 
> Please use GFP_NOWAIT instead of (__GFP_ATOMIC | __GFP_KSWAPD_RECLAIM).
> There is already a plan to completely remove __GFP_ATOMIC and mm-tree
> already have a patch for that.

Oh, I didn't know this, thanks for heads up!
I agree that GFP_NOWAIT is the best choice then.

Btw, we probably shouldn't even add GFP_NOWAIT if the allocation is performed
from the bpf syscall context. Why would we fail to pre-allocate a map if
we can easily go into the reclaim? But probably better to leave it for
a separate change.

Thanks!

Re: [PATCH bpf-next v2 1/2] bpf: Make non-preallocated allocation low priority

[Alexei Starovoitov]

On Wed, Jul 6, 2022 at 5:25 PM Roman Gushchin <roman.gushchin@linux.dev> wrote:
>
> On Thu, Jul 07, 2022 at 12:07:21AM +0000, Shakeel Butt wrote:
> > On Wed, Jul 06, 2022 at 03:58:47PM +0000, Yafang Shao wrote:
> > > GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially
> > > if we allocate too much GFP_ATOMIC memory. For example, when we set the
> > > memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can
> > > easily break the memcg limit by force charge. So it is very dangerous to
> > > use GFP_ATOMIC in non-preallocated case. One way to make it safe is to
> > > remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC |
> > > __GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate
> > > too much memory.
> >
> > Please use GFP_NOWAIT instead of (__GFP_ATOMIC | __GFP_KSWAPD_RECLAIM).
> > There is already a plan to completely remove __GFP_ATOMIC and mm-tree
> > already have a patch for that.
>
> Oh, I didn't know this, thanks for heads up!
> I agree that GFP_NOWAIT is the best choice then.
>
> Btw, we probably shouldn't even add GFP_NOWAIT if the allocation is performed
> from the bpf syscall context. Why would we fail to pre-allocate a map if
> we can easily go into the reclaim? But probably better to leave it for
> a separate change.

The places affected by this patch are in atomic context.
Prealloc path from syscall is using GFP_USER.

Re: [PATCH bpf-next v2 1/2] bpf: Make non-preallocated allocation low priority

[Roman Gushchin]

On Wed, Jul 06, 2022 at 07:09:22PM -0700, Alexei Starovoitov wrote:
> On Wed, Jul 6, 2022 at 5:25 PM Roman Gushchin <roman.gushchin@linux.dev> wrote:
> >
> > On Thu, Jul 07, 2022 at 12:07:21AM +0000, Shakeel Butt wrote:
> > > On Wed, Jul 06, 2022 at 03:58:47PM +0000, Yafang Shao wrote:
> > > > GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially
> > > > if we allocate too much GFP_ATOMIC memory. For example, when we set the
> > > > memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can
> > > > easily break the memcg limit by force charge. So it is very dangerous to
> > > > use GFP_ATOMIC in non-preallocated case. One way to make it safe is to
> > > > remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC |
> > > > __GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate
> > > > too much memory.
> > >
> > > Please use GFP_NOWAIT instead of (__GFP_ATOMIC | __GFP_KSWAPD_RECLAIM).
> > > There is already a plan to completely remove __GFP_ATOMIC and mm-tree
> > > already have a patch for that.
> >
> > Oh, I didn't know this, thanks for heads up!
> > I agree that GFP_NOWAIT is the best choice then.
> >
> > Btw, we probably shouldn't even add GFP_NOWAIT if the allocation is performed
> > from the bpf syscall context. Why would we fail to pre-allocate a map if
> > we can easily go into the reclaim? But probably better to leave it for
> > a separate change.
> 
> The places affected by this patch are in atomic context.
> Prealloc path from syscall is using GFP_USER.

Right. Sorry, my bad, for some reason I was under an impression it's a common
path for all allocations.

Thanks!

Re: [PATCH bpf-next v2 1/2] bpf: Make non-preallocated allocation low priority

[Yafang Shao]

On Thu, Jul 7, 2022 at 8:07 AM Shakeel Butt <shakeelb@google.com> wrote:
>
> On Wed, Jul 06, 2022 at 03:58:47PM +0000, Yafang Shao wrote:
> > GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially
> > if we allocate too much GFP_ATOMIC memory. For example, when we set the
> > memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can
> > easily break the memcg limit by force charge. So it is very dangerous to
> > use GFP_ATOMIC in non-preallocated case. One way to make it safe is to
> > remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC |
> > __GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate
> > too much memory.
>
> Please use GFP_NOWAIT instead of (__GFP_ATOMIC | __GFP_KSWAPD_RECLAIM).
> There is already a plan to completely remove __GFP_ATOMIC and mm-tree
> already have a patch for that.
>

After reading the discussion[1], it looks good to me to use GFP_NOWAIT
instead. I will update it.

[1]. https://lore.kernel.org/linux-mm/163712397076.13692.4727608274002939094@noble.neil.brown.name/

> >
> > We introduced BPF_F_NO_PREALLOC is because full map pre-allocation is
> > too memory expensive for some cases. That means removing __GFP_HIGH
> > doesn't break the rule of BPF_F_NO_PREALLOC, but has the same goal with
> > it-avoiding issues caused by too much memory. So let's remove it.
> >
> > The force charge of GFP_ATOMIC was introduced in
> > commit 869712fd3de5 ("mm: memcontrol: fix network errors from failing
> > __GFP_ATOMIC charges") by checking __GFP_ATOMIC, then got improved in
> > commit 1461e8c2b6af ("memcg: unify force charging conditions") by
> > checking __GFP_HIGH (that is no problem because both __GFP_HIGH and
> > __GFP_ATOMIC are set in GFP_AOMIC). So, if we want to fix it in memcg,
> > we have to carefully verify all the callsites. Now that we can fix it in
> > BPF, we'd better not modify the memcg code.
> >
> > This fix can also apply to other run-time allocations, for example, the
> > allocation in lpm trie, local storage and devmap. So let fix it
> > consistently over the bpf code
> >
> > __GFP_KSWAPD_RECLAIM doesn't cooperate well with memcg pressure neither
> > currently. But the memcg code can be improved to make
> > __GFP_KSWAPD_RECLAIM work well under memcg pressure if desired.
> >
>
> IMO there is no need to give all this detail and background on
> GFP_ATOMIC and __GFP_KSWAPD_RECLAIM. Just say kernel allows GFP_ATOMIC
> allocations to exceed memcg limits which we don't want in this case. So,
> replace with GFP_NOWAIT which obey memcg limits. Both of these flags
> tell kernel that the caller can not sleep.
>

Sure, thanks.

-- 
Regards
Yafang

Re: [PATCH bpf-next v2 2/2] bpf: Warn on non-preallocated case for missed trace types

[Yafang Shao]

On Thu, Jul 7, 2022 at 12:50 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Jul 6, 2022 at 8:59 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE and BPF_PROG_TYPE_TRACING are
> > trace type as well, which may also cause unexpected memory allocation if
> > we set BPF_F_NO_PREALLOC.
> > Let's also warn on both of them.
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> >  kernel/bpf/verifier.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index df3ec6b05f05..f9c0f4889a3a 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -12570,6 +12570,8 @@ static bool is_tracing_prog_type(enum bpf_prog_type type)
> >         case BPF_PROG_TYPE_TRACEPOINT:
> >         case BPF_PROG_TYPE_PERF_EVENT:
> >         case BPF_PROG_TYPE_RAW_TRACEPOINT:
> > +       case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE:
> > +       case BPF_PROG_TYPE_TRACING:
>
> BPF_TRACE_ITER should probably be excluded.

Right, I have verified that BPF_TRACE_ITER can be excluded.
Will change it.

-- 
Regards
Yafang

Re: [PATCH bpf-next v2 1/2] bpf: Make non-preallocated allocation low priority

[Alexei Starovoitov]

On Thu, Jul 7, 2022 at 3:28 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> On Thu, Jul 7, 2022 at 8:07 AM Shakeel Butt <shakeelb@google.com> wrote:
> >
> > On Wed, Jul 06, 2022 at 03:58:47PM +0000, Yafang Shao wrote:
> > > GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially
> > > if we allocate too much GFP_ATOMIC memory. For example, when we set the
> > > memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can
> > > easily break the memcg limit by force charge. So it is very dangerous to
> > > use GFP_ATOMIC in non-preallocated case. One way to make it safe is to
> > > remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC |
> > > __GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate
> > > too much memory.
> >
> > Please use GFP_NOWAIT instead of (__GFP_ATOMIC | __GFP_KSWAPD_RECLAIM).
> > There is already a plan to completely remove __GFP_ATOMIC and mm-tree
> > already have a patch for that.
> >
>
> After reading the discussion[1], it looks good to me to use GFP_NOWAIT
> instead. I will update it.

Should we use GFP_ATOMIC | __GFP_NOMEMALLOC instead
to align with its usage in the networking stack?

Re: [PATCH bpf-next v2 2/2] bpf: Warn on non-preallocated case for missed trace types

[Alexei Starovoitov]

On Thu, Jul 7, 2022 at 3:30 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> On Thu, Jul 7, 2022 at 12:50 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Wed, Jul 6, 2022 at 8:59 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > >
> > > BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE and BPF_PROG_TYPE_TRACING are
> > > trace type as well, which may also cause unexpected memory allocation if
> > > we set BPF_F_NO_PREALLOC.
> > > Let's also warn on both of them.
> > >
> > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > > ---
> > >  kernel/bpf/verifier.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > index df3ec6b05f05..f9c0f4889a3a 100644
> > > --- a/kernel/bpf/verifier.c
> > > +++ b/kernel/bpf/verifier.c
> > > @@ -12570,6 +12570,8 @@ static bool is_tracing_prog_type(enum bpf_prog_type type)
> > >         case BPF_PROG_TYPE_TRACEPOINT:
> > >         case BPF_PROG_TYPE_PERF_EVENT:
> > >         case BPF_PROG_TYPE_RAW_TRACEPOINT:
> > > +       case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE:
> > > +       case BPF_PROG_TYPE_TRACING:
> >
> > BPF_TRACE_ITER should probably be excluded.
>
> Right, I have verified that BPF_TRACE_ITER can be excluded.
> Will change it.

Probably more than that. See that your change broke BPF CI
and selftests are failing.
Which means it breaks existing bpf programs.

Re: [PATCH bpf-next v2 1/2] bpf: Make non-preallocated allocation low priority

[Yafang Shao]

On Thu, Jul 7, 2022 at 11:44 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Jul 7, 2022 at 3:28 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > On Thu, Jul 7, 2022 at 8:07 AM Shakeel Butt <shakeelb@google.com> wrote:
> > >
> > > On Wed, Jul 06, 2022 at 03:58:47PM +0000, Yafang Shao wrote:
> > > > GFP_ATOMIC doesn't cooperate well with memcg pressure so far, especially
> > > > if we allocate too much GFP_ATOMIC memory. For example, when we set the
> > > > memcg limit to limit a non-preallocated bpf memory, the GFP_ATOMIC can
> > > > easily break the memcg limit by force charge. So it is very dangerous to
> > > > use GFP_ATOMIC in non-preallocated case. One way to make it safe is to
> > > > remove __GFP_HIGH from GFP_ATOMIC, IOW, use (__GFP_ATOMIC |
> > > > __GFP_KSWAPD_RECLAIM) instead, then it will be limited if we allocate
> > > > too much memory.
> > >
> > > Please use GFP_NOWAIT instead of (__GFP_ATOMIC | __GFP_KSWAPD_RECLAIM).
> > > There is already a plan to completely remove __GFP_ATOMIC and mm-tree
> > > already have a patch for that.
> > >
> >
> > After reading the discussion[1], it looks good to me to use GFP_NOWAIT
> > instead. I will update it.
>
> Should we use GFP_ATOMIC | __GFP_NOMEMALLOC instead
> to align with its usage in the networking stack?

GFP_ATOMIC | __GFP_NOMEMALLOC will continue to break the memcg limit,
so we have to modify the try_charge_memcg() code to make sure
__GFP_NOMEMALLOC takes precedence over the __GFP_HIGH flag, IOW, if
both of them are set we won't allow it to break memcg limit.  That
will need more verification.

-- 
Regards
Yafang

Re: [PATCH bpf-next v2 2/2] bpf: Warn on non-preallocated case for missed trace types

[Yafang Shao]

On Thu, Jul 7, 2022 at 11:45 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Jul 7, 2022 at 3:30 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > On Thu, Jul 7, 2022 at 12:50 AM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Wed, Jul 6, 2022 at 8:59 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > > >
> > > > BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE and BPF_PROG_TYPE_TRACING are
> > > > trace type as well, which may also cause unexpected memory allocation if
> > > > we set BPF_F_NO_PREALLOC.
> > > > Let's also warn on both of them.
> > > >
> > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > > > ---
> > > >  kernel/bpf/verifier.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > > index df3ec6b05f05..f9c0f4889a3a 100644
> > > > --- a/kernel/bpf/verifier.c
> > > > +++ b/kernel/bpf/verifier.c
> > > > @@ -12570,6 +12570,8 @@ static bool is_tracing_prog_type(enum bpf_prog_type type)
> > > >         case BPF_PROG_TYPE_TRACEPOINT:
> > > >         case BPF_PROG_TYPE_PERF_EVENT:
> > > >         case BPF_PROG_TYPE_RAW_TRACEPOINT:
> > > > +       case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE:
> > > > +       case BPF_PROG_TYPE_TRACING:
> > >
> > > BPF_TRACE_ITER should probably be excluded.
> >
> > Right, I have verified that BPF_TRACE_ITER can be excluded.
> > Will change it.
>
> Probably more than that. See that your change broke BPF CI
> and selftests are failing.
> Which means it breaks existing bpf programs.

Ah, yes, "#194 timer:FAIL" which is caused by this change.
I will be more careful here.

-- 
Regards
Yafang