| Message ID | 20250115021746.34691-8-alexei.starovoitov@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | bpf, mm: Introduce try_alloc_pages() | expand |
On 1/15/25 03:17, Alexei Starovoitov wrote: > From: Alexei Starovoitov <ast@kernel.org> > > Use try_alloc_pages() and free_pages_nolock() > > Signed-off-by: Alexei Starovoitov <ast@kernel.org> > --- > kernel/bpf/syscall.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index 0daf098e3207..8bcf48e31a5a 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -582,14 +582,14 @@ int bpf_map_alloc_pages(const struct bpf_map *map, gfp_t gfp, int nid, This makes the gfp parameter unused? And the callers are passing GFP_KERNEL anyway? Isn't try_alloc_pages() rather useful for some context that did not even try to allocate until now, but now it could? Also unless my concerns about page_owner were wrong, this is where they could manifest. > old_memcg = set_active_memcg(memcg); > #endif > for (i = 0; i < nr_pages; i++) { > - pg = alloc_pages_node(nid, gfp | __GFP_ACCOUNT, 0); > + pg = try_alloc_pages(nid, 0); > > if (pg) { > pages[i] = pg; > continue; > } > for (j = 0; j < i; j++) > - __free_page(pages[j]); > + free_pages_nolock(pages[j], 0); > ret = -ENOMEM; > break; > }
On Wed, Jan 15, 2025 at 10:03 AM Vlastimil Babka <vbabka@suse.cz> wrote: > > On 1/15/25 03:17, Alexei Starovoitov wrote: > > From: Alexei Starovoitov <ast@kernel.org> > > > > Use try_alloc_pages() and free_pages_nolock() > > > > Signed-off-by: Alexei Starovoitov <ast@kernel.org> > > --- > > kernel/bpf/syscall.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > index 0daf098e3207..8bcf48e31a5a 100644 > > --- a/kernel/bpf/syscall.c > > +++ b/kernel/bpf/syscall.c > > @@ -582,14 +582,14 @@ int bpf_map_alloc_pages(const struct bpf_map *map, gfp_t gfp, int nid, > > This makes the gfp parameter unused? And the callers are passing GFP_KERNEL > anyway? Isn't try_alloc_pages() rather useful for some context that did not > even try to allocate until now, but now it could? Correct. gfp arg is unused and currently it's only called from sleepable bpf kfunc with GFP_KERNEL. I have more patches on top that change all that: remove gfp argument, etc. Just didn't send them as part of this set, since it's all bpf internal stuff.
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 0daf098e3207..8bcf48e31a5a 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -582,14 +582,14 @@ int bpf_map_alloc_pages(const struct bpf_map *map, gfp_t gfp, int nid, old_memcg = set_active_memcg(memcg); #endif for (i = 0; i < nr_pages; i++) { - pg = alloc_pages_node(nid, gfp | __GFP_ACCOUNT, 0); + pg = try_alloc_pages(nid, 0); if (pg) { pages[i] = pg; continue; } for (j = 0; j < i; j++) - __free_page(pages[j]); + free_pages_nolock(pages[j], 0); ret = -ENOMEM; break; }