* [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag()
@ 2024-10-08 11:20 Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 01/14] mm: page_frag: add a test module for page_frag Yunsheng Lin
` (13 more replies)
0 siblings, 14 replies; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni; +Cc: netdev, linux-kernel, Yunsheng Lin, Alexander Duyck
After [1], there are still two implementations for page frag:
1. mm/page_alloc.c: net stack seems to be using it in the
rx part with 'struct page_frag_cache' and the main API
being page_frag_alloc_align().
2. net/core/sock.c: net stack seems to be using it in the
tx part with 'struct page_frag' and the main API being
skb_page_frag_refill().
This patchset tries to unfiy the page frag implementation
by replacing page_frag with page_frag_cache for sk_page_frag()
first. net_high_order_alloc_disable_key for the implementation
in net/core/sock.c doesn't seems matter that much now as pcp
is also supported for high-order pages:
commit 44042b449872 ("mm/page_alloc: allow high-order pages to
be stored on the per-cpu lists")
As the related change is mostly related to networking, so
targeting the net-next. And will try to replace the rest
of page_frag in the follow patchset.
After this patchset:
1. Unify the page frag implementation by taking the best out of
two the existing implementations: we are able to save some space
for the 'page_frag_cache' API user, and avoid 'get_page()' for
the old 'page_frag' API user.
2. Future bugfix and performance can be done in one place, hence
improving maintainability of page_frag's implementation.
Kernel Image changing:
Linux Kernel total | text data bss
------------------------------------------------------
after 45250307 | 27274279 17209996 766032
before 45254134 | 27278118 17209984 766032
delta -3827 | -3839 +12 +0
Performance validation:
1. Using micro-benchmark ko added in patch 1 to test aligned and
non-aligned API performance impact for the existing users, there
is no notiable performance degradation. Instead we seems to have
some major performance boot for both aligned and non-aligned API
after switching to ptr_ring for testing, respectively about 200%
and 10% improvement in arm64 server as below.
2. Use the below netcat test case, we also have some minor
performance boot for replacing 'page_frag' with 'page_frag_cache'
after this patchset.
server: taskset -c 32 nc -l -k 1234 > /dev/null
client: perf stat -r 200 -- taskset -c 0 head -c 20G /dev/zero | taskset -c 1 nc 127.0.0.1 1234
In order to avoid performance noise as much as possible, the testing
is done in system without any other load and have enough iterations to
prove the data is stable enough, complete log for testing is below:
perf stat -r 200 -- insmod ./page_frag_test.ko test_push_cpu=16 test_pop_cpu=17 test_alloc_len=12 nr_test=51200000
perf stat -r 200 -- insmod ./page_frag_test.ko test_push_cpu=16 test_pop_cpu=17 test_alloc_len=12 nr_test=51200000 test_align=1
taskset -c 32 nc -l -k 1234 > /dev/null
perf stat -r 200 -- taskset -c 0 head -c 20G /dev/zero | taskset -c 1 nc 127.0.0.1 1234
*After* this patchset:
Performance counter stats for 'insmod ./page_frag_test.ko test_push_cpu=16 test_pop_cpu=17 test_alloc_len=12 nr_test=51200000' (200 runs):
17.758393 task-clock (msec) # 0.004 CPUs utilized ( +- 0.51% )
5 context-switches # 0.293 K/sec ( +- 0.65% )
0 cpu-migrations # 0.008 K/sec ( +- 17.21% )
74 page-faults # 0.004 M/sec ( +- 0.12% )
46128650 cycles # 2.598 GHz ( +- 0.51% )
60810511 instructions # 1.32 insn per cycle ( +- 0.04% )
14764914 branches # 831.433 M/sec ( +- 0.04% )
19281 branch-misses # 0.13% of all branches ( +- 0.13% )
4.240273854 seconds time elapsed ( +- 0.13% )
Performance counter stats for 'insmod ./page_frag_test.ko test_push_cpu=16 test_pop_cpu=17 test_alloc_len=12 nr_test=51200000 test_align=1' (200 runs):
17.348690 task-clock (msec) # 0.019 CPUs utilized ( +- 0.66% )
5 context-switches # 0.310 K/sec ( +- 0.84% )
0 cpu-migrations # 0.009 K/sec ( +- 16.55% )
74 page-faults # 0.004 M/sec ( +- 0.11% )
45065287 cycles # 2.598 GHz ( +- 0.66% )
60755389 instructions # 1.35 insn per cycle ( +- 0.05% )
14747865 branches # 850.085 M/sec ( +- 0.05% )
19272 branch-misses # 0.13% of all branches ( +- 0.13% )
0.935251375 seconds time elapsed ( +- 0.07% )
Performance counter stats for 'taskset -c 0 head -c 20G /dev/zero' (200 runs):
16626.042731 task-clock (msec) # 0.607 CPUs utilized ( +- 0.03% )
3291020 context-switches # 0.198 M/sec ( +- 0.05% )
1 cpu-migrations # 0.000 K/sec ( +- 0.50% )
85 page-faults # 0.005 K/sec ( +- 0.16% )
30581044838 cycles # 1.839 GHz ( +- 0.05% )
34962744631 instructions # 1.14 insn per cycle ( +- 0.01% )
6483883671 branches # 389.984 M/sec ( +- 0.02% )
99624551 branch-misses # 1.54% of all branches ( +- 0.17% )
27.370305077 seconds time elapsed ( +- 0.01% )
*Before* this patchset:
Performance counter stats for 'insmod ./page_frag_test.ko test_push_cpu=16 test_pop_cpu=17 test_alloc_len=12 nr_test=51200000' (200 runs):
21.587934 task-clock (msec) # 0.005 CPUs utilized ( +- 0.72% )
6 context-switches # 0.281 K/sec ( +- 0.28% )
1 cpu-migrations # 0.047 K/sec ( +- 0.50% )
73 page-faults # 0.003 M/sec ( +- 0.12% )
56080697 cycles # 2.598 GHz ( +- 0.72% )
61605150 instructions # 1.10 insn per cycle ( +- 0.05% )
14950196 branches # 692.526 M/sec ( +- 0.05% )
19410 branch-misses # 0.13% of all branches ( +- 0.18% )
4.603530546 seconds time elapsed ( +- 0.11% )
Performance counter stats for 'insmod ./page_frag_test.ko test_push_cpu=16 test_pop_cpu=17 test_alloc_len=12 nr_test=51200000 test_align=1' (200 runs):
20.988297 task-clock (msec) # 0.006 CPUs utilized ( +- 0.81% )
7 context-switches # 0.316 K/sec ( +- 0.54% )
1 cpu-migrations # 0.048 K/sec ( +- 0.70% )
73 page-faults # 0.003 M/sec ( +- 0.11% )
54512166 cycles # 2.597 GHz ( +- 0.81% )
61440941 instructions # 1.13 insn per cycle ( +- 0.08% )
14906043 branches # 710.207 M/sec ( +- 0.08% )
19927 branch-misses # 0.13% of all branches ( +- 0.17% )
3.438041238 seconds time elapsed ( +- 1.11% )
Performance counter stats for 'taskset -c 0 head -c 20G /dev/zero' (200 runs):
17364.040855 task-clock (msec) # 0.624 CPUs utilized ( +- 0.02% )
3340375 context-switches # 0.192 M/sec ( +- 0.06% )
1 cpu-migrations # 0.000 K/sec
85 page-faults # 0.005 K/sec ( +- 0.15% )
32077623335 cycles # 1.847 GHz ( +- 0.03% )
35121047596 instructions # 1.09 insn per cycle ( +- 0.01% )
6519872824 branches # 375.481 M/sec ( +- 0.02% )
101877022 branch-misses # 1.56% of all branches ( +- 0.14% )
27.842745343 seconds time elapsed ( +- 0.02% )
Note, ipv4-udp, ipv6-tcp and ipv6-udp is also tested with the below script:
nc -u -l -k 1234 > /dev/null
perf stat -r 4 -- head -c 51200000000 /dev/zero | nc -N -u 127.0.0.1 1234
nc -l6 -k 1234 > /dev/null
perf stat -r 4 -- head -c 51200000000 /dev/zero | nc -N ::1 1234
nc -l6 -k -u 1234 > /dev/null
perf stat -r 4 -- head -c 51200000000 /dev/zero | nc -u -N ::1 1234
CC: Alexander Duyck <alexander.duyck@gmail.com>
1. https://lore.kernel.org/all/20240228093013.8263-1-linyunsheng@huawei.com/
Change log:
V20:
1. Rename skb_copy_to_page_nocache() to skb_add_frag_nocache().
2. Define the PFMEMALLOC_BIT as the ORDER_MASK + 1 as suggested by
Alexander.
V19:
1. Rebased on latest net-next.
2. Use wait_for_completion_timeout() instead of wait_for_completion()
in page_frag_test.c
V18:
1. Fix a typo in test_page_frag.sh pointed out by Alexander.
2. Move some inline helper into c file, use ternary operator and
move the getting of the size as suggested by Alexander.
V17:
1. Add TEST_FILES in Makefile for test_page_frag.sh.
V16:
1. Add test_page_frag.sh to handle page_frag_test.ko and add testing
for prepare API.
2. Move inline helper unneeded outside of the page_frag_cache.c to
page_frag_cache.c.
3. Reset nc->offset when reusing an old page.
V15:
1. Fix the compile error pointed out by Simon.
2. Fix Other mistakes when using new API naming and refactoring.
V14:
1. Drop '_va' Renaming patch and use new API naming.
2. Use new refactoring to enable more codes to be reusable.
3. And other minor suggestions from Alexander.
V13:
1. Move page_frag_test from mm/ to tools/testing/selftest/mm
2. Use ptr_ring to replace ptr_pool for page_frag_test.c
3. Retest based on the new testing ko, which shows a big different
result than using ptr_pool.
V12:
1. Do not treat page_frag_test ko as DEBUG feature.
2. Make some improvement for the refactoring in patch 8.
3. Some other minor improvement as Alexander's comment.
RFC v11:
1. Fold 'page_frag_cache' moving change into patch 2.
2. Optimizate patch 3 according to discussion in v9.
V10:
1. Change Subject to "Replace page_frag with page_frag_cache for sk_page_frag()".
2. Move 'struct page_frag_cache' to sched.h as suggested by Alexander.
3. Rename skb_copy_to_page_nocache().
4. Adjust change between patches to make it more reviewable as Alexander's comment.
5. Use 'aligned_remaining' variable to generate virtual address as Alexander's
comment.
6. Some included header and typo fix as Alexander's comment.
7. Add back the get_order() opt patch for xtensa arch
V9:
1. Add check for test_alloc_len and change perm of module_param()
to 0 as Wang Wei' comment.
2. Rebased on latest net-next.
V8: Remove patch 2 & 3 in V7, as free_unref_page() is changed to call
pcp_allowed_order() and used in page_frag API recently in:
commit 5b8d75913a0e ("mm: combine free_the_page() and free_unref_page()")
V7: Fix doc build warning and error.
V6:
1. Fix some typo and compiler error for x86 pointed out by Jakub and
Simon.
2. Add two refactoring and optimization patches.
V5:
1. Add page_frag_alloc_pg() API for tls_device.c case and refactor
some implementation, update kernel bin size changing as bin size
is increased after that.
2. Add ack from Mat.
RFC v4:
1. Update doc according to Randy and Mat's suggestion.
2. Change probe API to "probe" for a specific amount of available space,
rather than "nonzero" space according to Mat's suggestion.
3. Retest and update the test result.
v3:
1. Use new layout for 'struct page_frag_cache' as the discussion
with Alexander and other sugeestions from Alexander.
2. Add probe API to address Mat' comment about mptcp use case.
3. Some doc updating according to Bagas' suggestion.
v2:
1. reorder test module to patch 1.
2. split doc and maintainer updating to two patches.
3. refactor the page_frag before moving.
4. fix a type and 'static' warning in test module.
5. add a patch for xtensa arch to enable using get_order() in
BUILD_BUG_ON().
6. Add test case and performance data for the socket code.
Yunsheng Lin (14):
mm: page_frag: add a test module for page_frag
mm: move the page fragment allocator from page_alloc into its own file
mm: page_frag: use initial zero offset for page_frag_alloc_align()
mm: page_frag: avoid caller accessing 'page_frag_cache' directly
xtensa: remove the get_order() implementation
mm: page_frag: reuse existing space for 'size' and 'pfmemalloc'
mm: page_frag: some minor refactoring before adding new API
mm: page_frag: use __alloc_pages() to replace alloc_pages_node()
net: rename skb_copy_to_page_nocache() helper
mm: page_frag: introduce prepare/probe/commit API
mm: page_frag: add testing for the newly added prepare API
net: replace page_frag with page_frag_cache
mm: page_frag: update documentation for page_frag
mm: page_frag: add an entry in MAINTAINERS for page_frag
Documentation/mm/page_frags.rst | 177 ++++++-
MAINTAINERS | 12 +
arch/xtensa/include/asm/page.h | 18 -
.../chelsio/inline_crypto/chtls/chtls.h | 3 -
.../chelsio/inline_crypto/chtls/chtls_io.c | 101 +---
.../chelsio/inline_crypto/chtls/chtls_main.c | 3 -
drivers/net/tun.c | 47 +-
drivers/vhost/net.c | 2 +-
include/linux/gfp.h | 22 -
include/linux/mm_types.h | 18 -
include/linux/mm_types_task.h | 21 +
include/linux/page_frag_cache.h | 471 ++++++++++++++++++
include/linux/sched.h | 2 +-
include/linux/skbuff.h | 1 +
include/net/sock.h | 30 +-
kernel/exit.c | 3 +-
kernel/fork.c | 3 +-
mm/Makefile | 1 +
mm/page_alloc.c | 136 -----
mm/page_frag_cache.c | 243 +++++++++
net/core/skbuff.c | 64 ++-
net/core/skmsg.c | 12 +-
net/core/sock.c | 32 +-
net/ipv4/ip_output.c | 28 +-
net/ipv4/tcp.c | 26 +-
net/ipv4/tcp_output.c | 25 +-
net/ipv6/ip6_output.c | 28 +-
net/kcm/kcmsock.c | 21 +-
net/mptcp/protocol.c | 47 +-
net/rxrpc/conn_object.c | 4 +-
net/rxrpc/local_object.c | 4 +-
net/sched/em_meta.c | 2 +-
net/sunrpc/svcsock.c | 6 +-
net/tls/tls_device.c | 100 ++--
tools/testing/selftests/mm/Makefile | 3 +
tools/testing/selftests/mm/page_frag/Makefile | 18 +
.../selftests/mm/page_frag/page_frag_test.c | 229 +++++++++
tools/testing/selftests/mm/run_vmtests.sh | 12 +
tools/testing/selftests/mm/test_page_frag.sh | 202 ++++++++
39 files changed, 1696 insertions(+), 481 deletions(-)
create mode 100644 include/linux/page_frag_cache.h
create mode 100644 mm/page_frag_cache.c
create mode 100644 tools/testing/selftests/mm/page_frag/Makefile
create mode 100644 tools/testing/selftests/mm/page_frag/page_frag_test.c
create mode 100755 tools/testing/selftests/mm/test_page_frag.sh
--
2.33.0
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH net-next v20 01/14] mm: page_frag: add a test module for page_frag
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
@ 2024-10-08 11:20 ` Yunsheng Lin
2024-10-08 19:56 ` Shuah Khan
2024-10-08 11:20 ` [PATCH net-next v20 02/14] mm: move the page fragment allocator from page_alloc into its own file Yunsheng Lin
` (12 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Alexander Duyck,
Alexander Duyck, Andrew Morton, Shuah Khan, linux-mm,
linux-kselftest
The testing is done by ensuring that the fragment allocated
from a frag_frag_cache instance is pushed into a ptr_ring
instance in a kthread binded to a specified cpu, and a kthread
binded to a specified cpu will pop the fragment from the
ptr_ring and free the fragment.
CC: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
---
tools/testing/selftests/mm/Makefile | 3 +
tools/testing/selftests/mm/page_frag/Makefile | 18 ++
.../selftests/mm/page_frag/page_frag_test.c | 173 ++++++++++++++++++
tools/testing/selftests/mm/run_vmtests.sh | 8 +
tools/testing/selftests/mm/test_page_frag.sh | 171 +++++++++++++++++
5 files changed, 373 insertions(+)
create mode 100644 tools/testing/selftests/mm/page_frag/Makefile
create mode 100644 tools/testing/selftests/mm/page_frag/page_frag_test.c
create mode 100755 tools/testing/selftests/mm/test_page_frag.sh
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index 02e1204971b0..acec529baaca 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -36,6 +36,8 @@ MAKEFLAGS += --no-builtin-rules
CFLAGS = -Wall -I $(top_srcdir) $(EXTRA_CFLAGS) $(KHDR_INCLUDES) $(TOOLS_INCLUDES)
LDLIBS = -lrt -lpthread -lm
+TEST_GEN_MODS_DIR := page_frag
+
TEST_GEN_FILES = cow
TEST_GEN_FILES += compaction_test
TEST_GEN_FILES += gup_longterm
@@ -126,6 +128,7 @@ TEST_FILES += test_hmm.sh
TEST_FILES += va_high_addr_switch.sh
TEST_FILES += charge_reserved_hugetlb.sh
TEST_FILES += hugetlb_reparenting_test.sh
+TEST_FILES += test_page_frag.sh
# required by charge_reserved_hugetlb.sh
TEST_FILES += write_hugetlb_memory.sh
diff --git a/tools/testing/selftests/mm/page_frag/Makefile b/tools/testing/selftests/mm/page_frag/Makefile
new file mode 100644
index 000000000000..58dda74d50a3
--- /dev/null
+++ b/tools/testing/selftests/mm/page_frag/Makefile
@@ -0,0 +1,18 @@
+PAGE_FRAG_TEST_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
+KDIR ?= $(abspath $(PAGE_FRAG_TEST_DIR)/../../../../..)
+
+ifeq ($(V),1)
+Q =
+else
+Q = @
+endif
+
+MODULES = page_frag_test.ko
+
+obj-m += page_frag_test.o
+
+all:
+ +$(Q)make -C $(KDIR) M=$(PAGE_FRAG_TEST_DIR) modules
+
+clean:
+ +$(Q)make -C $(KDIR) M=$(PAGE_FRAG_TEST_DIR) clean
diff --git a/tools/testing/selftests/mm/page_frag/page_frag_test.c b/tools/testing/selftests/mm/page_frag/page_frag_test.c
new file mode 100644
index 000000000000..eeb2b6bc681a
--- /dev/null
+++ b/tools/testing/selftests/mm/page_frag/page_frag_test.c
@@ -0,0 +1,173 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Test module for page_frag cache
+ *
+ * Copyright (C) 2024 Yunsheng Lin <linyunsheng@huawei.com>
+ */
+
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/cpumask.h>
+#include <linux/completion.h>
+#include <linux/ptr_ring.h>
+#include <linux/kthread.h>
+
+static struct ptr_ring ptr_ring;
+static int nr_objs = 512;
+static atomic_t nthreads;
+static struct completion wait;
+static struct page_frag_cache test_nc;
+static int test_popped;
+static int test_pushed;
+
+static int nr_test = 2000000;
+module_param(nr_test, int, 0);
+MODULE_PARM_DESC(nr_test, "number of iterations to test");
+
+static bool test_align;
+module_param(test_align, bool, 0);
+MODULE_PARM_DESC(test_align, "use align API for testing");
+
+static int test_alloc_len = 2048;
+module_param(test_alloc_len, int, 0);
+MODULE_PARM_DESC(test_alloc_len, "alloc len for testing");
+
+static int test_push_cpu;
+module_param(test_push_cpu, int, 0);
+MODULE_PARM_DESC(test_push_cpu, "test cpu for pushing fragment");
+
+static int test_pop_cpu;
+module_param(test_pop_cpu, int, 0);
+MODULE_PARM_DESC(test_pop_cpu, "test cpu for popping fragment");
+
+static int page_frag_pop_thread(void *arg)
+{
+ struct ptr_ring *ring = arg;
+
+ pr_info("page_frag pop test thread begins on cpu %d\n",
+ smp_processor_id());
+
+ while (test_popped < nr_test) {
+ void *obj = __ptr_ring_consume(ring);
+
+ if (obj) {
+ test_popped++;
+ page_frag_free(obj);
+ } else {
+ cond_resched();
+ }
+ }
+
+ if (atomic_dec_and_test(&nthreads))
+ complete(&wait);
+
+ pr_info("page_frag pop test thread exits on cpu %d\n",
+ smp_processor_id());
+
+ return 0;
+}
+
+static int page_frag_push_thread(void *arg)
+{
+ struct ptr_ring *ring = arg;
+
+ pr_info("page_frag push test thread begins on cpu %d\n",
+ smp_processor_id());
+
+ while (test_pushed < nr_test) {
+ void *va;
+ int ret;
+
+ if (test_align) {
+ va = page_frag_alloc_align(&test_nc, test_alloc_len,
+ GFP_KERNEL, SMP_CACHE_BYTES);
+
+ WARN_ONCE((unsigned long)va & (SMP_CACHE_BYTES - 1),
+ "unaligned va returned\n");
+ } else {
+ va = page_frag_alloc(&test_nc, test_alloc_len, GFP_KERNEL);
+ }
+
+ if (!va)
+ continue;
+
+ ret = __ptr_ring_produce(ring, va);
+ if (ret) {
+ page_frag_free(va);
+ cond_resched();
+ } else {
+ test_pushed++;
+ }
+ }
+
+ pr_info("page_frag push test thread exits on cpu %d\n",
+ smp_processor_id());
+
+ if (atomic_dec_and_test(&nthreads))
+ complete(&wait);
+
+ return 0;
+}
+
+static int __init page_frag_test_init(void)
+{
+ struct task_struct *tsk_push, *tsk_pop;
+ ktime_t start;
+ u64 duration;
+ int ret;
+
+ test_nc.va = NULL;
+ atomic_set(&nthreads, 2);
+ init_completion(&wait);
+
+ if (test_alloc_len > PAGE_SIZE || test_alloc_len <= 0 ||
+ !cpu_active(test_push_cpu) || !cpu_active(test_pop_cpu))
+ return -EINVAL;
+
+ ret = ptr_ring_init(&ptr_ring, nr_objs, GFP_KERNEL);
+ if (ret)
+ return ret;
+
+ tsk_push = kthread_create_on_cpu(page_frag_push_thread, &ptr_ring,
+ test_push_cpu, "page_frag_push");
+ if (IS_ERR(tsk_push))
+ return PTR_ERR(tsk_push);
+
+ tsk_pop = kthread_create_on_cpu(page_frag_pop_thread, &ptr_ring,
+ test_pop_cpu, "page_frag_pop");
+ if (IS_ERR(tsk_pop)) {
+ kthread_stop(tsk_push);
+ return PTR_ERR(tsk_pop);
+ }
+
+ start = ktime_get();
+ wake_up_process(tsk_push);
+ wake_up_process(tsk_pop);
+
+ pr_info("waiting for test to complete\n");
+
+ while (!wait_for_completion_timeout(&wait, msecs_to_jiffies(10000)))
+ pr_info("page_frag_test progress: pushed = %d, popped = %d\n",
+ test_pushed, test_popped);
+
+ duration = (u64)ktime_us_delta(ktime_get(), start);
+ pr_info("%d of iterations for %s testing took: %lluus\n", nr_test,
+ test_align ? "aligned" : "non-aligned", duration);
+
+ ptr_ring_cleanup(&ptr_ring, NULL);
+ page_frag_cache_drain(&test_nc);
+
+ return -EAGAIN;
+}
+
+static void __exit page_frag_test_exit(void)
+{
+}
+
+module_init(page_frag_test_init);
+module_exit(page_frag_test_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Yunsheng Lin <linyunsheng@huawei.com>");
+MODULE_DESCRIPTION("Test module for page_frag");
diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
index c5797ad1d37b..2c5394584af4 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -75,6 +75,8 @@ separated by spaces:
read-only VMAs
- mdwe
test prctl(PR_SET_MDWE, ...)
+- page_frag
+ test handling of page fragment allocation and freeing
example: ./run_vmtests.sh -t "hmm mmap ksm"
EOF
@@ -456,6 +458,12 @@ CATEGORY="mkdirty" run_test ./mkdirty
CATEGORY="mdwe" run_test ./mdwe_test
+CATEGORY="page_frag" run_test ./test_page_frag.sh smoke
+
+CATEGORY="page_frag" run_test ./test_page_frag.sh aligned
+
+CATEGORY="page_frag" run_test ./test_page_frag.sh nonaligned
+
echo "SUMMARY: PASS=${count_pass} SKIP=${count_skip} FAIL=${count_fail}" | tap_prefix
echo "1..${count_total}" | tap_output
diff --git a/tools/testing/selftests/mm/test_page_frag.sh b/tools/testing/selftests/mm/test_page_frag.sh
new file mode 100755
index 000000000000..d750d910c899
--- /dev/null
+++ b/tools/testing/selftests/mm/test_page_frag.sh
@@ -0,0 +1,171 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) 2024 Yunsheng Lin <linyunsheng@huawei.com>
+# Copyright (C) 2018 Uladzislau Rezki (Sony) <urezki@gmail.com>
+#
+# This is a test script for the kernel test driver to test the
+# correctness and performance of page_frag's implementation.
+# Therefore it is just a kernel module loader. You can specify
+# and pass different parameters in order to:
+# a) analyse performance of page fragment allocations;
+# b) stressing and stability check of page_frag subsystem.
+
+DRIVER="./page_frag/page_frag_test.ko"
+CPU_LIST=$(grep -m 2 processor /proc/cpuinfo | cut -d ' ' -f 2)
+TEST_CPU_0=$(echo $CPU_LIST | awk '{print $1}')
+
+if [ $(echo $CPU_LIST | wc -w) -gt 1 ]; then
+ TEST_CPU_1=$(echo $CPU_LIST | awk '{print $2}')
+ NR_TEST=100000000
+else
+ TEST_CPU_1=$TEST_CPU_0
+ NR_TEST=1000000
+fi
+
+# 1 if fails
+exitcode=1
+
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+
+#
+# Static templates for testing of page_frag APIs.
+# Also it is possible to pass any supported parameters manually.
+#
+SMOKE_PARAM="test_push_cpu=$TEST_CPU_0 test_pop_cpu=$TEST_CPU_1"
+NONALIGNED_PARAM="$SMOKE_PARAM test_alloc_len=75 nr_test=$NR_TEST"
+ALIGNED_PARAM="$NONALIGNED_PARAM test_align=1"
+
+check_test_requirements()
+{
+ uid=$(id -u)
+ if [ $uid -ne 0 ]; then
+ echo "$0: Must be run as root"
+ exit $ksft_skip
+ fi
+
+ if ! which insmod > /dev/null 2>&1; then
+ echo "$0: You need insmod installed"
+ exit $ksft_skip
+ fi
+
+ if [ ! -f $DRIVER ]; then
+ echo "$0: You need to compile page_frag_test module"
+ exit $ksft_skip
+ fi
+}
+
+run_nonaligned_check()
+{
+ echo "Run performance tests to evaluate how fast nonaligned alloc API is."
+
+ insmod $DRIVER $NONALIGNED_PARAM > /dev/null 2>&1
+ echo "Done."
+ echo "Check the kernel ring buffer to see the summary."
+}
+
+run_aligned_check()
+{
+ echo "Run performance tests to evaluate how fast aligned alloc API is."
+
+ insmod $DRIVER $ALIGNED_PARAM > /dev/null 2>&1
+ echo "Done."
+ echo "Check the kernel ring buffer to see the summary."
+}
+
+run_smoke_check()
+{
+ echo "Run smoke test."
+
+ insmod $DRIVER $SMOKE_PARAM > /dev/null 2>&1
+ echo "Done."
+ echo "Check the kernel ring buffer to see the summary."
+}
+
+usage()
+{
+ echo -n "Usage: $0 [ aligned ] | [ nonaligned ] | | [ smoke ] | "
+ echo "manual parameters"
+ echo
+ echo "Valid tests and parameters:"
+ echo
+ modinfo $DRIVER
+ echo
+ echo "Example usage:"
+ echo
+ echo "# Shows help message"
+ echo "$0"
+ echo
+ echo "# Smoke testing"
+ echo "$0 smoke"
+ echo
+ echo "# Performance testing for nonaligned alloc API"
+ echo "$0 nonaligned"
+ echo
+ echo "# Performance testing for aligned alloc API"
+ echo "$0 aligned"
+ echo
+ exit 0
+}
+
+function validate_passed_args()
+{
+ VALID_ARGS=`modinfo $DRIVER | awk '/parm:/ {print $2}' | sed 's/:.*//'`
+
+ #
+ # Something has been passed, check it.
+ #
+ for passed_arg in $@; do
+ key=${passed_arg//=*/}
+ valid=0
+
+ for valid_arg in $VALID_ARGS; do
+ if [[ $key = $valid_arg ]]; then
+ valid=1
+ break
+ fi
+ done
+
+ if [[ $valid -ne 1 ]]; then
+ echo "Error: key is not correct: ${key}"
+ exit $exitcode
+ fi
+ done
+}
+
+function run_manual_check()
+{
+ #
+ # Validate passed parameters. If there is wrong one,
+ # the script exists and does not execute further.
+ #
+ validate_passed_args $@
+
+ echo "Run the test with following parameters: $@"
+ insmod $DRIVER $@ > /dev/null 2>&1
+ echo "Done."
+ echo "Check the kernel ring buffer to see the summary."
+}
+
+function run_test()
+{
+ if [ $# -eq 0 ]; then
+ usage
+ else
+ if [[ "$1" = "smoke" ]]; then
+ run_smoke_check
+ elif [[ "$1" = "nonaligned" ]]; then
+ run_nonaligned_check
+ elif [[ "$1" = "aligned" ]]; then
+ run_aligned_check
+ else
+ run_manual_check $@
+ fi
+ fi
+}
+
+check_test_requirements
+run_test $@
+
+exit 0
--
2.33.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next v20 02/14] mm: move the page fragment allocator from page_alloc into its own file
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 01/14] mm: page_frag: add a test module for page_frag Yunsheng Lin
@ 2024-10-08 11:20 ` Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 03/14] mm: page_frag: use initial zero offset for page_frag_alloc_align() Yunsheng Lin
` (11 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, David Howells,
Alexander Duyck, Andrew Morton, Alexander Duyck, Eric Dumazet,
Shuah Khan, linux-mm, linux-kselftest
Inspired by [1], move the page fragment allocator from page_alloc
into its own c file and header file, as we are about to make more
change for it to replace another page_frag implementation in
sock.c
As this patchset is going to replace 'struct page_frag' with
'struct page_frag_cache' in sched.h, including page_frag_cache.h
in sched.h has a compiler error caused by interdependence between
mm_types.h and mm.h for asm-offsets.c, see [2]. So avoid the compiler
error by moving 'struct page_frag_cache' to mm_types_task.h as
suggested by Alexander, see [3].
1. https://lore.kernel.org/all/20230411160902.4134381-3-dhowells@redhat.com/
2. https://lore.kernel.org/all/15623dac-9358-4597-b3ee-3694a5956920@gmail.com/
3. https://lore.kernel.org/all/CAKgT0UdH1yD=LSCXFJ=YM_aiA4OomD-2wXykO42bizaWMt_HOA@mail.gmail.com/
CC: David Howells <dhowells@redhat.com>
CC: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Acked-by: Andrew Morton <akpm@linux-foundation.org>
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
---
include/linux/gfp.h | 22 ---
include/linux/mm_types.h | 18 ---
include/linux/mm_types_task.h | 18 +++
include/linux/page_frag_cache.h | 31 ++++
include/linux/skbuff.h | 1 +
mm/Makefile | 1 +
mm/page_alloc.c | 136 ----------------
mm/page_frag_cache.c | 145 ++++++++++++++++++
.../selftests/mm/page_frag/page_frag_test.c | 2 +-
9 files changed, 197 insertions(+), 177 deletions(-)
create mode 100644 include/linux/page_frag_cache.h
create mode 100644 mm/page_frag_cache.c
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index a951de920e20..a0a6d25f883f 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -371,28 +371,6 @@ __meminit void *alloc_pages_exact_nid_noprof(int nid, size_t size, gfp_t gfp_mas
extern void __free_pages(struct page *page, unsigned int order);
extern void free_pages(unsigned long addr, unsigned int order);
-struct page_frag_cache;
-void page_frag_cache_drain(struct page_frag_cache *nc);
-extern void __page_frag_cache_drain(struct page *page, unsigned int count);
-void *__page_frag_alloc_align(struct page_frag_cache *nc, unsigned int fragsz,
- gfp_t gfp_mask, unsigned int align_mask);
-
-static inline void *page_frag_alloc_align(struct page_frag_cache *nc,
- unsigned int fragsz, gfp_t gfp_mask,
- unsigned int align)
-{
- WARN_ON_ONCE(!is_power_of_2(align));
- return __page_frag_alloc_align(nc, fragsz, gfp_mask, -align);
-}
-
-static inline void *page_frag_alloc(struct page_frag_cache *nc,
- unsigned int fragsz, gfp_t gfp_mask)
-{
- return __page_frag_alloc_align(nc, fragsz, gfp_mask, ~0u);
-}
-
-extern void page_frag_free(void *addr);
-
#define __free_page(page) __free_pages((page), 0)
#define free_page(addr) free_pages((addr), 0)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 6e3bdf8e38bc..92314ef2d978 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -521,9 +521,6 @@ static_assert(sizeof(struct ptdesc) <= sizeof(struct page));
*/
#define STRUCT_PAGE_MAX_SHIFT (order_base_2(sizeof(struct page)))
-#define PAGE_FRAG_CACHE_MAX_SIZE __ALIGN_MASK(32768, ~PAGE_MASK)
-#define PAGE_FRAG_CACHE_MAX_ORDER get_order(PAGE_FRAG_CACHE_MAX_SIZE)
-
/*
* page_private can be used on tail pages. However, PagePrivate is only
* checked by the VM on the head page. So page_private on the tail pages
@@ -542,21 +539,6 @@ static inline void *folio_get_private(struct folio *folio)
return folio->private;
}
-struct page_frag_cache {
- void * va;
-#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
- __u16 offset;
- __u16 size;
-#else
- __u32 offset;
-#endif
- /* we maintain a pagecount bias, so that we dont dirty cache line
- * containing page->_refcount every time we allocate a fragment.
- */
- unsigned int pagecnt_bias;
- bool pfmemalloc;
-};
-
typedef unsigned long vm_flags_t;
/*
diff --git a/include/linux/mm_types_task.h b/include/linux/mm_types_task.h
index bff5706b76e1..0ac6daebdd5c 100644
--- a/include/linux/mm_types_task.h
+++ b/include/linux/mm_types_task.h
@@ -8,6 +8,7 @@
* (These are defined separately to decouple sched.h from mm_types.h as much as possible.)
*/
+#include <linux/align.h>
#include <linux/types.h>
#include <asm/page.h>
@@ -43,6 +44,23 @@ struct page_frag {
#endif
};
+#define PAGE_FRAG_CACHE_MAX_SIZE __ALIGN_MASK(32768, ~PAGE_MASK)
+#define PAGE_FRAG_CACHE_MAX_ORDER get_order(PAGE_FRAG_CACHE_MAX_SIZE)
+struct page_frag_cache {
+ void *va;
+#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
+ __u16 offset;
+ __u16 size;
+#else
+ __u32 offset;
+#endif
+ /* we maintain a pagecount bias, so that we dont dirty cache line
+ * containing page->_refcount every time we allocate a fragment.
+ */
+ unsigned int pagecnt_bias;
+ bool pfmemalloc;
+};
+
/* Track pages that require TLB flushes */
struct tlbflush_unmap_batch {
#ifdef CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
diff --git a/include/linux/page_frag_cache.h b/include/linux/page_frag_cache.h
new file mode 100644
index 000000000000..67ac8626ed9b
--- /dev/null
+++ b/include/linux/page_frag_cache.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_PAGE_FRAG_CACHE_H
+#define _LINUX_PAGE_FRAG_CACHE_H
+
+#include <linux/log2.h>
+#include <linux/mm_types_task.h>
+#include <linux/types.h>
+
+void page_frag_cache_drain(struct page_frag_cache *nc);
+void __page_frag_cache_drain(struct page *page, unsigned int count);
+void *__page_frag_alloc_align(struct page_frag_cache *nc, unsigned int fragsz,
+ gfp_t gfp_mask, unsigned int align_mask);
+
+static inline void *page_frag_alloc_align(struct page_frag_cache *nc,
+ unsigned int fragsz, gfp_t gfp_mask,
+ unsigned int align)
+{
+ WARN_ON_ONCE(!is_power_of_2(align));
+ return __page_frag_alloc_align(nc, fragsz, gfp_mask, -align);
+}
+
+static inline void *page_frag_alloc(struct page_frag_cache *nc,
+ unsigned int fragsz, gfp_t gfp_mask)
+{
+ return __page_frag_alloc_align(nc, fragsz, gfp_mask, ~0u);
+}
+
+void page_frag_free(void *addr);
+
+#endif
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 39f1d16f3628..560e2b49f98b 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -31,6 +31,7 @@
#include <linux/in6.h>
#include <linux/if_packet.h>
#include <linux/llist.h>
+#include <linux/page_frag_cache.h>
#include <net/flow.h>
#if IS_ENABLED(CONFIG_NF_CONNTRACK)
#include <linux/netfilter/nf_conntrack_common.h>
diff --git a/mm/Makefile b/mm/Makefile
index d5639b036166..dba52bb0da8a 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -65,6 +65,7 @@ page-alloc-$(CONFIG_SHUFFLE_PAGE_ALLOCATOR) += shuffle.o
memory-hotplug-$(CONFIG_MEMORY_HOTPLUG) += memory_hotplug.o
obj-y += page-alloc.o
+obj-y += page_frag_cache.o
obj-y += init-mm.o
obj-y += memblock.o
obj-y += $(memory-hotplug-y)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 8afab64814dc..6ca2abce857b 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4836,142 +4836,6 @@ void free_pages(unsigned long addr, unsigned int order)
EXPORT_SYMBOL(free_pages);
-/*
- * Page Fragment:
- * An arbitrary-length arbitrary-offset area of memory which resides
- * within a 0 or higher order page. Multiple fragments within that page
- * are individually refcounted, in the page's reference counter.
- *
- * The page_frag functions below provide a simple allocation framework for
- * page fragments. This is used by the network stack and network device
- * drivers to provide a backing region of memory for use as either an
- * sk_buff->head, or to be used in the "frags" portion of skb_shared_info.
- */
-static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
- gfp_t gfp_mask)
-{
- struct page *page = NULL;
- gfp_t gfp = gfp_mask;
-
-#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
- gfp_mask = (gfp_mask & ~__GFP_DIRECT_RECLAIM) | __GFP_COMP |
- __GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC;
- page = alloc_pages_node(NUMA_NO_NODE, gfp_mask,
- PAGE_FRAG_CACHE_MAX_ORDER);
- nc->size = page ? PAGE_FRAG_CACHE_MAX_SIZE : PAGE_SIZE;
-#endif
- if (unlikely(!page))
- page = alloc_pages_node(NUMA_NO_NODE, gfp, 0);
-
- nc->va = page ? page_address(page) : NULL;
-
- return page;
-}
-
-void page_frag_cache_drain(struct page_frag_cache *nc)
-{
- if (!nc->va)
- return;
-
- __page_frag_cache_drain(virt_to_head_page(nc->va), nc->pagecnt_bias);
- nc->va = NULL;
-}
-EXPORT_SYMBOL(page_frag_cache_drain);
-
-void __page_frag_cache_drain(struct page *page, unsigned int count)
-{
- VM_BUG_ON_PAGE(page_ref_count(page) == 0, page);
-
- if (page_ref_sub_and_test(page, count))
- free_unref_page(page, compound_order(page));
-}
-EXPORT_SYMBOL(__page_frag_cache_drain);
-
-void *__page_frag_alloc_align(struct page_frag_cache *nc,
- unsigned int fragsz, gfp_t gfp_mask,
- unsigned int align_mask)
-{
- unsigned int size = PAGE_SIZE;
- struct page *page;
- int offset;
-
- if (unlikely(!nc->va)) {
-refill:
- page = __page_frag_cache_refill(nc, gfp_mask);
- if (!page)
- return NULL;
-
-#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
- /* if size can vary use size else just use PAGE_SIZE */
- size = nc->size;
-#endif
- /* Even if we own the page, we do not use atomic_set().
- * This would break get_page_unless_zero() users.
- */
- page_ref_add(page, PAGE_FRAG_CACHE_MAX_SIZE);
-
- /* reset page count bias and offset to start of new frag */
- nc->pfmemalloc = page_is_pfmemalloc(page);
- nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
- nc->offset = size;
- }
-
- offset = nc->offset - fragsz;
- if (unlikely(offset < 0)) {
- page = virt_to_page(nc->va);
-
- if (!page_ref_sub_and_test(page, nc->pagecnt_bias))
- goto refill;
-
- if (unlikely(nc->pfmemalloc)) {
- free_unref_page(page, compound_order(page));
- goto refill;
- }
-
-#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
- /* if size can vary use size else just use PAGE_SIZE */
- size = nc->size;
-#endif
- /* OK, page count is 0, we can safely set it */
- set_page_count(page, PAGE_FRAG_CACHE_MAX_SIZE + 1);
-
- /* reset page count bias and offset to start of new frag */
- nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
- offset = size - fragsz;
- if (unlikely(offset < 0)) {
- /*
- * The caller is trying to allocate a fragment
- * with fragsz > PAGE_SIZE but the cache isn't big
- * enough to satisfy the request, this may
- * happen in low memory conditions.
- * We don't release the cache page because
- * it could make memory pressure worse
- * so we simply return NULL here.
- */
- return NULL;
- }
- }
-
- nc->pagecnt_bias--;
- offset &= align_mask;
- nc->offset = offset;
-
- return nc->va + offset;
-}
-EXPORT_SYMBOL(__page_frag_alloc_align);
-
-/*
- * Frees a page fragment allocated out of either a compound or order 0 page.
- */
-void page_frag_free(void *addr)
-{
- struct page *page = virt_to_head_page(addr);
-
- if (unlikely(put_page_testzero(page)))
- free_unref_page(page, compound_order(page));
-}
-EXPORT_SYMBOL(page_frag_free);
-
static void *make_alloc_exact(unsigned long addr, unsigned int order,
size_t size)
{
diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
new file mode 100644
index 000000000000..609a485cd02a
--- /dev/null
+++ b/mm/page_frag_cache.c
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Page fragment allocator
+ *
+ * Page Fragment:
+ * An arbitrary-length arbitrary-offset area of memory which resides within a
+ * 0 or higher order page. Multiple fragments within that page are
+ * individually refcounted, in the page's reference counter.
+ *
+ * The page_frag functions provide a simple allocation framework for page
+ * fragments. This is used by the network stack and network device drivers to
+ * provide a backing region of memory for use as either an sk_buff->head, or to
+ * be used in the "frags" portion of skb_shared_info.
+ */
+
+#include <linux/export.h>
+#include <linux/gfp_types.h>
+#include <linux/init.h>
+#include <linux/mm.h>
+#include <linux/page_frag_cache.h>
+#include "internal.h"
+
+static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
+ gfp_t gfp_mask)
+{
+ struct page *page = NULL;
+ gfp_t gfp = gfp_mask;
+
+#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
+ gfp_mask = (gfp_mask & ~__GFP_DIRECT_RECLAIM) | __GFP_COMP |
+ __GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC;
+ page = alloc_pages_node(NUMA_NO_NODE, gfp_mask,
+ PAGE_FRAG_CACHE_MAX_ORDER);
+ nc->size = page ? PAGE_FRAG_CACHE_MAX_SIZE : PAGE_SIZE;
+#endif
+ if (unlikely(!page))
+ page = alloc_pages_node(NUMA_NO_NODE, gfp, 0);
+
+ nc->va = page ? page_address(page) : NULL;
+
+ return page;
+}
+
+void page_frag_cache_drain(struct page_frag_cache *nc)
+{
+ if (!nc->va)
+ return;
+
+ __page_frag_cache_drain(virt_to_head_page(nc->va), nc->pagecnt_bias);
+ nc->va = NULL;
+}
+EXPORT_SYMBOL(page_frag_cache_drain);
+
+void __page_frag_cache_drain(struct page *page, unsigned int count)
+{
+ VM_BUG_ON_PAGE(page_ref_count(page) == 0, page);
+
+ if (page_ref_sub_and_test(page, count))
+ free_unref_page(page, compound_order(page));
+}
+EXPORT_SYMBOL(__page_frag_cache_drain);
+
+void *__page_frag_alloc_align(struct page_frag_cache *nc,
+ unsigned int fragsz, gfp_t gfp_mask,
+ unsigned int align_mask)
+{
+ unsigned int size = PAGE_SIZE;
+ struct page *page;
+ int offset;
+
+ if (unlikely(!nc->va)) {
+refill:
+ page = __page_frag_cache_refill(nc, gfp_mask);
+ if (!page)
+ return NULL;
+
+#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
+ /* if size can vary use size else just use PAGE_SIZE */
+ size = nc->size;
+#endif
+ /* Even if we own the page, we do not use atomic_set().
+ * This would break get_page_unless_zero() users.
+ */
+ page_ref_add(page, PAGE_FRAG_CACHE_MAX_SIZE);
+
+ /* reset page count bias and offset to start of new frag */
+ nc->pfmemalloc = page_is_pfmemalloc(page);
+ nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
+ nc->offset = size;
+ }
+
+ offset = nc->offset - fragsz;
+ if (unlikely(offset < 0)) {
+ page = virt_to_page(nc->va);
+
+ if (!page_ref_sub_and_test(page, nc->pagecnt_bias))
+ goto refill;
+
+ if (unlikely(nc->pfmemalloc)) {
+ free_unref_page(page, compound_order(page));
+ goto refill;
+ }
+
+#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
+ /* if size can vary use size else just use PAGE_SIZE */
+ size = nc->size;
+#endif
+ /* OK, page count is 0, we can safely set it */
+ set_page_count(page, PAGE_FRAG_CACHE_MAX_SIZE + 1);
+
+ /* reset page count bias and offset to start of new frag */
+ nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
+ offset = size - fragsz;
+ if (unlikely(offset < 0)) {
+ /*
+ * The caller is trying to allocate a fragment
+ * with fragsz > PAGE_SIZE but the cache isn't big
+ * enough to satisfy the request, this may
+ * happen in low memory conditions.
+ * We don't release the cache page because
+ * it could make memory pressure worse
+ * so we simply return NULL here.
+ */
+ return NULL;
+ }
+ }
+
+ nc->pagecnt_bias--;
+ offset &= align_mask;
+ nc->offset = offset;
+
+ return nc->va + offset;
+}
+EXPORT_SYMBOL(__page_frag_alloc_align);
+
+/*
+ * Frees a page fragment allocated out of either a compound or order 0 page.
+ */
+void page_frag_free(void *addr)
+{
+ struct page *page = virt_to_head_page(addr);
+
+ if (unlikely(put_page_testzero(page)))
+ free_unref_page(page, compound_order(page));
+}
+EXPORT_SYMBOL(page_frag_free);
diff --git a/tools/testing/selftests/mm/page_frag/page_frag_test.c b/tools/testing/selftests/mm/page_frag/page_frag_test.c
index eeb2b6bc681a..fdf204550c9a 100644
--- a/tools/testing/selftests/mm/page_frag/page_frag_test.c
+++ b/tools/testing/selftests/mm/page_frag/page_frag_test.c
@@ -6,12 +6,12 @@
* Copyright (C) 2024 Yunsheng Lin <linyunsheng@huawei.com>
*/
-#include <linux/mm.h>
#include <linux/module.h>
#include <linux/cpumask.h>
#include <linux/completion.h>
#include <linux/ptr_ring.h>
#include <linux/kthread.h>
+#include <linux/page_frag_cache.h>
static struct ptr_ring ptr_ring;
static int nr_objs = 512;
--
2.33.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next v20 03/14] mm: page_frag: use initial zero offset for page_frag_alloc_align()
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 01/14] mm: page_frag: add a test module for page_frag Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 02/14] mm: move the page fragment allocator from page_alloc into its own file Yunsheng Lin
@ 2024-10-08 11:20 ` Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 04/14] mm: page_frag: avoid caller accessing 'page_frag_cache' directly Yunsheng Lin
` (10 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Alexander Duyck,
Alexander Duyck, Andrew Morton, linux-mm
We are about to use page_frag_alloc_*() API to not just
allocate memory for skb->data, but also use them to do
the memory allocation for skb frag too. Currently the
implementation of page_frag in mm subsystem is running
the offset as a countdown rather than count-up value,
there may have several advantages to that as mentioned
in [1], but it may have some disadvantages, for example,
it may disable skb frag coalescing and more correct cache
prefetching
We have a trade-off to make in order to have a unified
implementation and API for page_frag, so use a initial zero
offset in this patch, and the following patch will try to
make some optimization to avoid the disadvantages as much
as possible.
1. https://lore.kernel.org/all/f4abe71b3439b39d17a6fb2d410180f367cadf5c.camel@gmail.com/
CC: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
---
mm/page_frag_cache.c | 46 ++++++++++++++++++++++----------------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
index 609a485cd02a..4c8e04379cb3 100644
--- a/mm/page_frag_cache.c
+++ b/mm/page_frag_cache.c
@@ -63,9 +63,13 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
unsigned int fragsz, gfp_t gfp_mask,
unsigned int align_mask)
{
+#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
+ unsigned int size = nc->size;
+#else
unsigned int size = PAGE_SIZE;
+#endif
+ unsigned int offset;
struct page *page;
- int offset;
if (unlikely(!nc->va)) {
refill:
@@ -85,11 +89,24 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
/* reset page count bias and offset to start of new frag */
nc->pfmemalloc = page_is_pfmemalloc(page);
nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
- nc->offset = size;
+ nc->offset = 0;
}
- offset = nc->offset - fragsz;
- if (unlikely(offset < 0)) {
+ offset = __ALIGN_KERNEL_MASK(nc->offset, ~align_mask);
+ if (unlikely(offset + fragsz > size)) {
+ if (unlikely(fragsz > PAGE_SIZE)) {
+ /*
+ * The caller is trying to allocate a fragment
+ * with fragsz > PAGE_SIZE but the cache isn't big
+ * enough to satisfy the request, this may
+ * happen in low memory conditions.
+ * We don't release the cache page because
+ * it could make memory pressure worse
+ * so we simply return NULL here.
+ */
+ return NULL;
+ }
+
page = virt_to_page(nc->va);
if (!page_ref_sub_and_test(page, nc->pagecnt_bias))
@@ -100,33 +117,16 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
goto refill;
}
-#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
- /* if size can vary use size else just use PAGE_SIZE */
- size = nc->size;
-#endif
/* OK, page count is 0, we can safely set it */
set_page_count(page, PAGE_FRAG_CACHE_MAX_SIZE + 1);
/* reset page count bias and offset to start of new frag */
nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
- offset = size - fragsz;
- if (unlikely(offset < 0)) {
- /*
- * The caller is trying to allocate a fragment
- * with fragsz > PAGE_SIZE but the cache isn't big
- * enough to satisfy the request, this may
- * happen in low memory conditions.
- * We don't release the cache page because
- * it could make memory pressure worse
- * so we simply return NULL here.
- */
- return NULL;
- }
+ offset = 0;
}
nc->pagecnt_bias--;
- offset &= align_mask;
- nc->offset = offset;
+ nc->offset = offset + fragsz;
return nc->va + offset;
}
--
2.33.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next v20 04/14] mm: page_frag: avoid caller accessing 'page_frag_cache' directly
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
` (2 preceding siblings ...)
2024-10-08 11:20 ` [PATCH net-next v20 03/14] mm: page_frag: use initial zero offset for page_frag_alloc_align() Yunsheng Lin
@ 2024-10-08 11:20 ` Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 05/14] xtensa: remove the get_order() implementation Yunsheng Lin
` (9 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Alexander Duyck,
Alexander Duyck, Chuck Lever, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Andrew Morton, Eric Dumazet, David Howells,
Marc Dionne, Trond Myklebust, Anna Schumaker, Jeff Layton,
Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey, Shuah Khan,
kvm, virtualization, linux-mm, linux-afs, linux-nfs,
linux-kselftest
Use appropriate frag_page API instead of caller accessing
'page_frag_cache' directly.
CC: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
Acked-by: Chuck Lever <chuck.lever@oracle.com>
---
drivers/vhost/net.c | 2 +-
include/linux/page_frag_cache.h | 10 ++++++++++
net/core/skbuff.c | 6 +++---
net/rxrpc/conn_object.c | 4 +---
net/rxrpc/local_object.c | 4 +---
net/sunrpc/svcsock.c | 6 ++----
tools/testing/selftests/mm/page_frag/page_frag_test.c | 2 +-
7 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index f16279351db5..9ad37c012189 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1325,7 +1325,7 @@ static int vhost_net_open(struct inode *inode, struct file *f)
vqs[VHOST_NET_VQ_RX]);
f->private_data = n;
- n->pf_cache.va = NULL;
+ page_frag_cache_init(&n->pf_cache);
return 0;
}
diff --git a/include/linux/page_frag_cache.h b/include/linux/page_frag_cache.h
index 67ac8626ed9b..0a52f7a179c8 100644
--- a/include/linux/page_frag_cache.h
+++ b/include/linux/page_frag_cache.h
@@ -7,6 +7,16 @@
#include <linux/mm_types_task.h>
#include <linux/types.h>
+static inline void page_frag_cache_init(struct page_frag_cache *nc)
+{
+ nc->va = NULL;
+}
+
+static inline bool page_frag_cache_is_pfmemalloc(struct page_frag_cache *nc)
+{
+ return !!nc->pfmemalloc;
+}
+
void page_frag_cache_drain(struct page_frag_cache *nc);
void __page_frag_cache_drain(struct page *page, unsigned int count);
void *__page_frag_alloc_align(struct page_frag_cache *nc, unsigned int fragsz,
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 74149dc4ee31..ca01880c7ad0 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -753,14 +753,14 @@ struct sk_buff *__netdev_alloc_skb(struct net_device *dev, unsigned int len,
if (in_hardirq() || irqs_disabled()) {
nc = this_cpu_ptr(&netdev_alloc_cache);
data = page_frag_alloc(nc, len, gfp_mask);
- pfmemalloc = nc->pfmemalloc;
+ pfmemalloc = page_frag_cache_is_pfmemalloc(nc);
} else {
local_bh_disable();
local_lock_nested_bh(&napi_alloc_cache.bh_lock);
nc = this_cpu_ptr(&napi_alloc_cache.page);
data = page_frag_alloc(nc, len, gfp_mask);
- pfmemalloc = nc->pfmemalloc;
+ pfmemalloc = page_frag_cache_is_pfmemalloc(nc);
local_unlock_nested_bh(&napi_alloc_cache.bh_lock);
local_bh_enable();
@@ -850,7 +850,7 @@ struct sk_buff *napi_alloc_skb(struct napi_struct *napi, unsigned int len)
len = SKB_HEAD_ALIGN(len);
data = page_frag_alloc(&nc->page, len, gfp_mask);
- pfmemalloc = nc->page.pfmemalloc;
+ pfmemalloc = page_frag_cache_is_pfmemalloc(&nc->page);
}
local_unlock_nested_bh(&napi_alloc_cache.bh_lock);
diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c
index 1539d315afe7..694c4df7a1a3 100644
--- a/net/rxrpc/conn_object.c
+++ b/net/rxrpc/conn_object.c
@@ -337,9 +337,7 @@ static void rxrpc_clean_up_connection(struct work_struct *work)
*/
rxrpc_purge_queue(&conn->rx_queue);
- if (conn->tx_data_alloc.va)
- __page_frag_cache_drain(virt_to_page(conn->tx_data_alloc.va),
- conn->tx_data_alloc.pagecnt_bias);
+ page_frag_cache_drain(&conn->tx_data_alloc);
call_rcu(&conn->rcu, rxrpc_rcu_free_connection);
}
diff --git a/net/rxrpc/local_object.c b/net/rxrpc/local_object.c
index 504453c688d7..a8cffe47cf01 100644
--- a/net/rxrpc/local_object.c
+++ b/net/rxrpc/local_object.c
@@ -452,9 +452,7 @@ void rxrpc_destroy_local(struct rxrpc_local *local)
#endif
rxrpc_purge_queue(&local->rx_queue);
rxrpc_purge_client_connections(local);
- if (local->tx_alloc.va)
- __page_frag_cache_drain(virt_to_page(local->tx_alloc.va),
- local->tx_alloc.pagecnt_bias);
+ page_frag_cache_drain(&local->tx_alloc);
}
/*
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 825ec5357691..b785425c3315 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1608,7 +1608,6 @@ static void svc_tcp_sock_detach(struct svc_xprt *xprt)
static void svc_sock_free(struct svc_xprt *xprt)
{
struct svc_sock *svsk = container_of(xprt, struct svc_sock, sk_xprt);
- struct page_frag_cache *pfc = &svsk->sk_frag_cache;
struct socket *sock = svsk->sk_sock;
trace_svcsock_free(svsk, sock);
@@ -1618,8 +1617,7 @@ static void svc_sock_free(struct svc_xprt *xprt)
sockfd_put(sock);
else
sock_release(sock);
- if (pfc->va)
- __page_frag_cache_drain(virt_to_head_page(pfc->va),
- pfc->pagecnt_bias);
+
+ page_frag_cache_drain(&svsk->sk_frag_cache);
kfree(svsk);
}
diff --git a/tools/testing/selftests/mm/page_frag/page_frag_test.c b/tools/testing/selftests/mm/page_frag/page_frag_test.c
index fdf204550c9a..36543a129e40 100644
--- a/tools/testing/selftests/mm/page_frag/page_frag_test.c
+++ b/tools/testing/selftests/mm/page_frag/page_frag_test.c
@@ -117,7 +117,7 @@ static int __init page_frag_test_init(void)
u64 duration;
int ret;
- test_nc.va = NULL;
+ page_frag_cache_init(&test_nc);
atomic_set(&nthreads, 2);
init_completion(&wait);
--
2.33.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next v20 05/14] xtensa: remove the get_order() implementation
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
` (3 preceding siblings ...)
2024-10-08 11:20 ` [PATCH net-next v20 04/14] mm: page_frag: avoid caller accessing 'page_frag_cache' directly Yunsheng Lin
@ 2024-10-08 11:20 ` Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 06/14] mm: page_frag: reuse existing space for 'size' and 'pfmemalloc' Yunsheng Lin
` (8 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Alexander Duyck, Max Filippov,
Alexander Duyck, Chris Zankel
As the get_order() implemented by xtensa supporting 'nsau'
instruction seems be the same as the generic implementation
in include/asm-generic/getorder.h when size is not a constant
value as the generic implementation calling the fls*() is also
utilizing the 'nsau' instruction for xtensa.
So remove the get_order() implemented by xtensa, as using the
generic implementation may enable the compiler to do the
computing when size is a constant value instead of runtime
computing and enable the using of get_order() in BUILD_BUG_ON()
macro in next patch.
CC: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Acked-by: Max Filippov <jcmvbkbc@gmail.com>
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
---
arch/xtensa/include/asm/page.h | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/arch/xtensa/include/asm/page.h b/arch/xtensa/include/asm/page.h
index 4db56ef052d2..8665d57991dd 100644
--- a/arch/xtensa/include/asm/page.h
+++ b/arch/xtensa/include/asm/page.h
@@ -109,26 +109,8 @@ typedef struct page *pgtable_t;
#define __pgd(x) ((pgd_t) { (x) } )
#define __pgprot(x) ((pgprot_t) { (x) } )
-/*
- * Pure 2^n version of get_order
- * Use 'nsau' instructions if supported by the processor or the generic version.
- */
-
-#if XCHAL_HAVE_NSA
-
-static inline __attribute_const__ int get_order(unsigned long size)
-{
- int lz;
- asm ("nsau %0, %1" : "=r" (lz) : "r" ((size - 1) >> PAGE_SHIFT));
- return 32 - lz;
-}
-
-#else
-
# include <asm-generic/getorder.h>
-#endif
-
struct page;
struct vm_area_struct;
extern void clear_page(void *page);
--
2.33.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next v20 06/14] mm: page_frag: reuse existing space for 'size' and 'pfmemalloc'
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
` (4 preceding siblings ...)
2024-10-08 11:20 ` [PATCH net-next v20 05/14] xtensa: remove the get_order() implementation Yunsheng Lin
@ 2024-10-08 11:20 ` Yunsheng Lin
2024-10-09 23:50 ` Alexander Duyck
2024-10-08 11:20 ` [PATCH net-next v20 07/14] mm: page_frag: some minor refactoring before adding new API Yunsheng Lin
` (7 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Alexander Duyck,
Andrew Morton, linux-mm
Currently there is one 'struct page_frag' for every 'struct
sock' and 'struct task_struct', we are about to replace the
'struct page_frag' with 'struct page_frag_cache' for them.
Before begin the replacing, we need to ensure the size of
'struct page_frag_cache' is not bigger than the size of
'struct page_frag', as there may be tens of thousands of
'struct sock' and 'struct task_struct' instances in the
system.
By or'ing the page order & pfmemalloc with lower bits of
'va' instead of using 'u16' or 'u32' for page size and 'u8'
for pfmemalloc, we are able to avoid 3 or 5 bytes space waste.
And page address & pfmemalloc & order is unchanged for the
same page in the same 'page_frag_cache' instance, it makes
sense to fit them together.
After this patch, the size of 'struct page_frag_cache' should be
the same as the size of 'struct page_frag'.
CC: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
include/linux/mm_types_task.h | 19 +++++----
include/linux/page_frag_cache.h | 24 ++++++++++-
mm/page_frag_cache.c | 75 +++++++++++++++++++++++----------
3 files changed, 86 insertions(+), 32 deletions(-)
diff --git a/include/linux/mm_types_task.h b/include/linux/mm_types_task.h
index 0ac6daebdd5c..a82aa80c0ba4 100644
--- a/include/linux/mm_types_task.h
+++ b/include/linux/mm_types_task.h
@@ -47,18 +47,21 @@ struct page_frag {
#define PAGE_FRAG_CACHE_MAX_SIZE __ALIGN_MASK(32768, ~PAGE_MASK)
#define PAGE_FRAG_CACHE_MAX_ORDER get_order(PAGE_FRAG_CACHE_MAX_SIZE)
struct page_frag_cache {
- void *va;
-#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
+ /* encoded_page consists of the virtual address, pfmemalloc bit and
+ * order of a page.
+ */
+ unsigned long encoded_page;
+
+ /* we maintain a pagecount bias, so that we dont dirty cache line
+ * containing page->_refcount every time we allocate a fragment.
+ */
+#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) && (BITS_PER_LONG <= 32)
__u16 offset;
- __u16 size;
+ __u16 pagecnt_bias;
#else
__u32 offset;
+ __u32 pagecnt_bias;
#endif
- /* we maintain a pagecount bias, so that we dont dirty cache line
- * containing page->_refcount every time we allocate a fragment.
- */
- unsigned int pagecnt_bias;
- bool pfmemalloc;
};
/* Track pages that require TLB flushes */
diff --git a/include/linux/page_frag_cache.h b/include/linux/page_frag_cache.h
index 0a52f7a179c8..dba2268e451a 100644
--- a/include/linux/page_frag_cache.h
+++ b/include/linux/page_frag_cache.h
@@ -3,18 +3,38 @@
#ifndef _LINUX_PAGE_FRAG_CACHE_H
#define _LINUX_PAGE_FRAG_CACHE_H
+#include <linux/bits.h>
#include <linux/log2.h>
#include <linux/mm_types_task.h>
#include <linux/types.h>
+#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
+/* Use a full byte here to enable assembler optimization as the shift
+ * operation is usually expecting a byte.
+ */
+#define PAGE_FRAG_CACHE_ORDER_MASK GENMASK(7, 0)
+#else
+/* Compiler should be able to figure out we don't read things as any value
+ * ANDed with 0 is 0.
+ */
+#define PAGE_FRAG_CACHE_ORDER_MASK 0
+#endif
+
+#define PAGE_FRAG_CACHE_PFMEMALLOC_BIT (PAGE_FRAG_CACHE_ORDER_MASK + 1)
+
+static inline bool page_frag_encoded_page_pfmemalloc(unsigned long encoded_page)
+{
+ return !!(encoded_page & PAGE_FRAG_CACHE_PFMEMALLOC_BIT);
+}
+
static inline void page_frag_cache_init(struct page_frag_cache *nc)
{
- nc->va = NULL;
+ nc->encoded_page = 0;
}
static inline bool page_frag_cache_is_pfmemalloc(struct page_frag_cache *nc)
{
- return !!nc->pfmemalloc;
+ return page_frag_encoded_page_pfmemalloc(nc->encoded_page);
}
void page_frag_cache_drain(struct page_frag_cache *nc);
diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
index 4c8e04379cb3..4bff4de58808 100644
--- a/mm/page_frag_cache.c
+++ b/mm/page_frag_cache.c
@@ -12,6 +12,7 @@
* be used in the "frags" portion of skb_shared_info.
*/
+#include <linux/build_bug.h>
#include <linux/export.h>
#include <linux/gfp_types.h>
#include <linux/init.h>
@@ -19,9 +20,41 @@
#include <linux/page_frag_cache.h>
#include "internal.h"
+static unsigned long page_frag_encode_page(struct page *page, unsigned int order,
+ bool pfmemalloc)
+{
+ BUILD_BUG_ON(PAGE_FRAG_CACHE_MAX_ORDER > PAGE_FRAG_CACHE_ORDER_MASK);
+ BUILD_BUG_ON(PAGE_FRAG_CACHE_PFMEMALLOC_BIT >= PAGE_SIZE);
+
+ return (unsigned long)page_address(page) |
+ (order & PAGE_FRAG_CACHE_ORDER_MASK) |
+ ((unsigned long)pfmemalloc * PAGE_FRAG_CACHE_PFMEMALLOC_BIT);
+}
+
+static unsigned long page_frag_encoded_page_order(unsigned long encoded_page)
+{
+ return encoded_page & PAGE_FRAG_CACHE_ORDER_MASK;
+}
+
+static void *page_frag_encoded_page_address(unsigned long encoded_page)
+{
+ return (void *)(encoded_page & PAGE_MASK);
+}
+
+static struct page *page_frag_encoded_page_ptr(unsigned long encoded_page)
+{
+ return virt_to_page((void *)encoded_page);
+}
+
+static unsigned int page_frag_cache_page_size(unsigned long encoded_page)
+{
+ return PAGE_SIZE << page_frag_encoded_page_order(encoded_page);
+}
+
static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
gfp_t gfp_mask)
{
+ unsigned long order = PAGE_FRAG_CACHE_MAX_ORDER;
struct page *page = NULL;
gfp_t gfp = gfp_mask;
@@ -30,23 +63,26 @@ static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
__GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC;
page = alloc_pages_node(NUMA_NO_NODE, gfp_mask,
PAGE_FRAG_CACHE_MAX_ORDER);
- nc->size = page ? PAGE_FRAG_CACHE_MAX_SIZE : PAGE_SIZE;
#endif
- if (unlikely(!page))
+ if (unlikely(!page)) {
page = alloc_pages_node(NUMA_NO_NODE, gfp, 0);
+ order = 0;
+ }
- nc->va = page ? page_address(page) : NULL;
+ nc->encoded_page = page ?
+ page_frag_encode_page(page, order, page_is_pfmemalloc(page)) : 0;
return page;
}
void page_frag_cache_drain(struct page_frag_cache *nc)
{
- if (!nc->va)
+ if (!nc->encoded_page)
return;
- __page_frag_cache_drain(virt_to_head_page(nc->va), nc->pagecnt_bias);
- nc->va = NULL;
+ __page_frag_cache_drain(page_frag_encoded_page_ptr(nc->encoded_page),
+ nc->pagecnt_bias);
+ nc->encoded_page = 0;
}
EXPORT_SYMBOL(page_frag_cache_drain);
@@ -63,35 +99,29 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
unsigned int fragsz, gfp_t gfp_mask,
unsigned int align_mask)
{
-#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
- unsigned int size = nc->size;
-#else
- unsigned int size = PAGE_SIZE;
-#endif
- unsigned int offset;
+ unsigned long encoded_page = nc->encoded_page;
+ unsigned int size, offset;
struct page *page;
- if (unlikely(!nc->va)) {
+ if (unlikely(!encoded_page)) {
refill:
page = __page_frag_cache_refill(nc, gfp_mask);
if (!page)
return NULL;
-#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
- /* if size can vary use size else just use PAGE_SIZE */
- size = nc->size;
-#endif
+ encoded_page = nc->encoded_page;
+
/* Even if we own the page, we do not use atomic_set().
* This would break get_page_unless_zero() users.
*/
page_ref_add(page, PAGE_FRAG_CACHE_MAX_SIZE);
/* reset page count bias and offset to start of new frag */
- nc->pfmemalloc = page_is_pfmemalloc(page);
nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
nc->offset = 0;
}
+ size = page_frag_cache_page_size(encoded_page);
offset = __ALIGN_KERNEL_MASK(nc->offset, ~align_mask);
if (unlikely(offset + fragsz > size)) {
if (unlikely(fragsz > PAGE_SIZE)) {
@@ -107,13 +137,14 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
return NULL;
}
- page = virt_to_page(nc->va);
+ page = page_frag_encoded_page_ptr(encoded_page);
if (!page_ref_sub_and_test(page, nc->pagecnt_bias))
goto refill;
- if (unlikely(nc->pfmemalloc)) {
- free_unref_page(page, compound_order(page));
+ if (unlikely(page_frag_encoded_page_pfmemalloc(encoded_page))) {
+ free_unref_page(page,
+ page_frag_encoded_page_order(encoded_page));
goto refill;
}
@@ -128,7 +159,7 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
nc->pagecnt_bias--;
nc->offset = offset + fragsz;
- return nc->va + offset;
+ return page_frag_encoded_page_address(encoded_page) + offset;
}
EXPORT_SYMBOL(__page_frag_alloc_align);
--
2.33.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next v20 07/14] mm: page_frag: some minor refactoring before adding new API
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
` (5 preceding siblings ...)
2024-10-08 11:20 ` [PATCH net-next v20 06/14] mm: page_frag: reuse existing space for 'size' and 'pfmemalloc' Yunsheng Lin
@ 2024-10-08 11:20 ` Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 08/14] mm: page_frag: use __alloc_pages() to replace alloc_pages_node() Yunsheng Lin
` (6 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Alexander Duyck,
Andrew Morton, linux-mm
Refactor common codes from __page_frag_alloc_va_align() to
__page_frag_cache_prepare() and __page_frag_cache_commit(),
so that the new API can make use of them.
CC: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
include/linux/page_frag_cache.h | 36 +++++++++++++++++++++++++++--
mm/page_frag_cache.c | 40 ++++++++++++++++++++++++++-------
2 files changed, 66 insertions(+), 10 deletions(-)
diff --git a/include/linux/page_frag_cache.h b/include/linux/page_frag_cache.h
index dba2268e451a..a6cb32b1d1ca 100644
--- a/include/linux/page_frag_cache.h
+++ b/include/linux/page_frag_cache.h
@@ -5,6 +5,7 @@
#include <linux/bits.h>
#include <linux/log2.h>
+#include <linux/mmdebug.h>
#include <linux/mm_types_task.h>
#include <linux/types.h>
@@ -39,8 +40,39 @@ static inline bool page_frag_cache_is_pfmemalloc(struct page_frag_cache *nc)
void page_frag_cache_drain(struct page_frag_cache *nc);
void __page_frag_cache_drain(struct page *page, unsigned int count);
-void *__page_frag_alloc_align(struct page_frag_cache *nc, unsigned int fragsz,
- gfp_t gfp_mask, unsigned int align_mask);
+void *__page_frag_cache_prepare(struct page_frag_cache *nc, unsigned int fragsz,
+ struct page_frag *pfrag, gfp_t gfp_mask,
+ unsigned int align_mask);
+unsigned int __page_frag_cache_commit_noref(struct page_frag_cache *nc,
+ struct page_frag *pfrag,
+ unsigned int used_sz);
+
+static inline unsigned int __page_frag_cache_commit(struct page_frag_cache *nc,
+ struct page_frag *pfrag,
+ unsigned int used_sz)
+{
+ VM_BUG_ON(!nc->pagecnt_bias);
+ nc->pagecnt_bias--;
+
+ return __page_frag_cache_commit_noref(nc, pfrag, used_sz);
+}
+
+static inline void *__page_frag_alloc_align(struct page_frag_cache *nc,
+ unsigned int fragsz, gfp_t gfp_mask,
+ unsigned int align_mask)
+{
+ struct page_frag page_frag;
+ void *va;
+
+ va = __page_frag_cache_prepare(nc, fragsz, &page_frag, gfp_mask,
+ align_mask);
+ if (unlikely(!va))
+ return NULL;
+
+ __page_frag_cache_commit(nc, &page_frag, fragsz);
+
+ return va;
+}
static inline void *page_frag_alloc_align(struct page_frag_cache *nc,
unsigned int fragsz, gfp_t gfp_mask,
diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
index 4bff4de58808..e17f4a530af2 100644
--- a/mm/page_frag_cache.c
+++ b/mm/page_frag_cache.c
@@ -95,9 +95,31 @@ void __page_frag_cache_drain(struct page *page, unsigned int count)
}
EXPORT_SYMBOL(__page_frag_cache_drain);
-void *__page_frag_alloc_align(struct page_frag_cache *nc,
- unsigned int fragsz, gfp_t gfp_mask,
- unsigned int align_mask)
+unsigned int __page_frag_cache_commit_noref(struct page_frag_cache *nc,
+ struct page_frag *pfrag,
+ unsigned int used_sz)
+{
+ unsigned int orig_offset;
+
+ VM_BUG_ON(used_sz > pfrag->size);
+ VM_BUG_ON(pfrag->page != page_frag_encoded_page_ptr(nc->encoded_page));
+ VM_BUG_ON(pfrag->offset + pfrag->size >
+ page_frag_cache_page_size(nc->encoded_page));
+
+ /* pfrag->offset might be bigger than the nc->offset due to alignment */
+ VM_BUG_ON(nc->offset > pfrag->offset);
+
+ orig_offset = nc->offset;
+ nc->offset = pfrag->offset + used_sz;
+
+ /* Return true size back to caller considering the offset alignment */
+ return nc->offset - orig_offset;
+}
+EXPORT_SYMBOL(__page_frag_cache_commit_noref);
+
+void *__page_frag_cache_prepare(struct page_frag_cache *nc, unsigned int fragsz,
+ struct page_frag *pfrag, gfp_t gfp_mask,
+ unsigned int align_mask)
{
unsigned long encoded_page = nc->encoded_page;
unsigned int size, offset;
@@ -119,6 +141,8 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
/* reset page count bias and offset to start of new frag */
nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
nc->offset = 0;
+ } else {
+ page = page_frag_encoded_page_ptr(encoded_page);
}
size = page_frag_cache_page_size(encoded_page);
@@ -137,8 +161,6 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
return NULL;
}
- page = page_frag_encoded_page_ptr(encoded_page);
-
if (!page_ref_sub_and_test(page, nc->pagecnt_bias))
goto refill;
@@ -153,15 +175,17 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
/* reset page count bias and offset to start of new frag */
nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
+ nc->offset = 0;
offset = 0;
}
- nc->pagecnt_bias--;
- nc->offset = offset + fragsz;
+ pfrag->page = page;
+ pfrag->offset = offset;
+ pfrag->size = size - offset;
return page_frag_encoded_page_address(encoded_page) + offset;
}
-EXPORT_SYMBOL(__page_frag_alloc_align);
+EXPORT_SYMBOL(__page_frag_cache_prepare);
/*
* Frees a page fragment allocated out of either a compound or order 0 page.
--
2.33.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next v20 08/14] mm: page_frag: use __alloc_pages() to replace alloc_pages_node()
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
` (6 preceding siblings ...)
2024-10-08 11:20 ` [PATCH net-next v20 07/14] mm: page_frag: some minor refactoring before adding new API Yunsheng Lin
@ 2024-10-08 11:20 ` Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 09/14] net: rename skb_copy_to_page_nocache() helper Yunsheng Lin
` (5 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Alexander Duyck,
Alexander Duyck, Andrew Morton, linux-mm
It seems there is about 24Bytes binary size increase for
__page_frag_cache_refill() after refactoring in arm64 system
with 64K PAGE_SIZE. By doing the gdb disassembling, It seems
we can have more than 100Bytes decrease for the binary size
by using __alloc_pages() to replace alloc_pages_node(), as
there seems to be some unnecessary checking for nid being
NUMA_NO_NODE, especially when page_frag is part of the mm
system.
CC: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
---
mm/page_frag_cache.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
index e17f4a530af2..4666dbec38eb 100644
--- a/mm/page_frag_cache.c
+++ b/mm/page_frag_cache.c
@@ -61,11 +61,11 @@ static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
gfp_mask = (gfp_mask & ~__GFP_DIRECT_RECLAIM) | __GFP_COMP |
__GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC;
- page = alloc_pages_node(NUMA_NO_NODE, gfp_mask,
- PAGE_FRAG_CACHE_MAX_ORDER);
+ page = __alloc_pages(gfp_mask, PAGE_FRAG_CACHE_MAX_ORDER,
+ numa_mem_id(), NULL);
#endif
if (unlikely(!page)) {
- page = alloc_pages_node(NUMA_NO_NODE, gfp, 0);
+ page = __alloc_pages(gfp, 0, numa_mem_id(), NULL);
order = 0;
}
--
2.33.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next v20 09/14] net: rename skb_copy_to_page_nocache() helper
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
` (7 preceding siblings ...)
2024-10-08 11:20 ` [PATCH net-next v20 08/14] mm: page_frag: use __alloc_pages() to replace alloc_pages_node() Yunsheng Lin
@ 2024-10-08 11:20 ` Yunsheng Lin
2024-10-09 23:40 ` Alexander Duyck
2024-10-08 11:20 ` [PATCH net-next v20 10/14] mm: page_frag: introduce prepare/probe/commit API Yunsheng Lin
` (4 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Alexander Duyck, Eric Dumazet,
David Ahern
Rename skb_copy_to_page_nocache() to skb_add_frag_nocache()
to avoid calling virt_to_page() as we are about to pass virtual
address directly.
CC: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
include/net/sock.h | 9 +++------
net/ipv4/tcp.c | 7 +++----
net/kcm/kcmsock.c | 7 +++----
3 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index e282127092ab..e0b4e2daca5d 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2192,15 +2192,12 @@ static inline int skb_add_data_nocache(struct sock *sk, struct sk_buff *skb,
return err;
}
-static inline int skb_copy_to_page_nocache(struct sock *sk, struct iov_iter *from,
- struct sk_buff *skb,
- struct page *page,
- int off, int copy)
+static inline int skb_add_frag_nocache(struct sock *sk, struct iov_iter *from,
+ struct sk_buff *skb, char *va, int copy)
{
int err;
- err = skb_do_copy_data_nocache(sk, skb, from, page_address(page) + off,
- copy, skb->len);
+ err = skb_do_copy_data_nocache(sk, skb, from, va, copy, skb->len);
if (err)
return err;
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 82cc4a5633ce..bc36f02cac0d 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1219,10 +1219,9 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
if (!copy)
goto wait_for_space;
- err = skb_copy_to_page_nocache(sk, &msg->msg_iter, skb,
- pfrag->page,
- pfrag->offset,
- copy);
+ err = skb_add_frag_nocache(sk, &msg->msg_iter, skb,
+ page_address(pfrag->page) +
+ pfrag->offset, copy);
if (err)
goto do_error;
diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index d4118c796290..e74ad63f0189 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -856,10 +856,9 @@ static int kcm_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
if (!sk_wmem_schedule(sk, copy))
goto wait_for_memory;
- err = skb_copy_to_page_nocache(sk, &msg->msg_iter, skb,
- pfrag->page,
- pfrag->offset,
- copy);
+ err = skb_add_frag_nocache(sk, &msg->msg_iter, skb,
+ page_address(pfrag->page) +
+ pfrag->offset, copy);
if (err)
goto out_error;
--
2.33.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next v20 10/14] mm: page_frag: introduce prepare/probe/commit API
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
` (8 preceding siblings ...)
2024-10-08 11:20 ` [PATCH net-next v20 09/14] net: rename skb_copy_to_page_nocache() helper Yunsheng Lin
@ 2024-10-08 11:20 ` Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 11/14] mm: page_frag: add testing for the newly added prepare API Yunsheng Lin
` (3 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Alexander Duyck,
Andrew Morton, linux-mm
There are many use cases that need minimum memory in order
for forward progress, but more performant if more memory is
available or need to probe the cache info to use any memory
available for frag caoleasing reason.
Currently skb_page_frag_refill() API is used to solve the
above use cases, but caller needs to know about the internal
detail and access the data field of 'struct page_frag' to
meet the requirement of the above use cases and its
implementation is similar to the one in mm subsystem.
To unify those two page_frag implementations, introduce a
prepare API to ensure minimum memory is satisfied and return
how much the actual memory is available to the caller and a
probe API to report the current available memory to caller
without doing cache refilling. The caller needs to either call
the commit API to report how much memory it actually uses, or
not do so if deciding to not use any memory.
CC: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
include/linux/page_frag_cache.h | 135 ++++++++++++++++++++++++++++++++
mm/page_frag_cache.c | 21 +++++
2 files changed, 156 insertions(+)
diff --git a/include/linux/page_frag_cache.h b/include/linux/page_frag_cache.h
index a6cb32b1d1ca..d91ad53f25d3 100644
--- a/include/linux/page_frag_cache.h
+++ b/include/linux/page_frag_cache.h
@@ -38,6 +38,11 @@ static inline bool page_frag_cache_is_pfmemalloc(struct page_frag_cache *nc)
return page_frag_encoded_page_pfmemalloc(nc->encoded_page);
}
+static inline unsigned int page_frag_cache_page_offset(const struct page_frag_cache *nc)
+{
+ return nc->offset;
+}
+
void page_frag_cache_drain(struct page_frag_cache *nc);
void __page_frag_cache_drain(struct page *page, unsigned int count);
void *__page_frag_cache_prepare(struct page_frag_cache *nc, unsigned int fragsz,
@@ -46,6 +51,10 @@ void *__page_frag_cache_prepare(struct page_frag_cache *nc, unsigned int fragsz,
unsigned int __page_frag_cache_commit_noref(struct page_frag_cache *nc,
struct page_frag *pfrag,
unsigned int used_sz);
+void *__page_frag_alloc_refill_probe_align(struct page_frag_cache *nc,
+ unsigned int fragsz,
+ struct page_frag *pfrag,
+ unsigned int align_mask);
static inline unsigned int __page_frag_cache_commit(struct page_frag_cache *nc,
struct page_frag *pfrag,
@@ -88,6 +97,132 @@ static inline void *page_frag_alloc(struct page_frag_cache *nc,
return __page_frag_alloc_align(nc, fragsz, gfp_mask, ~0u);
}
+static inline bool __page_frag_refill_align(struct page_frag_cache *nc,
+ unsigned int fragsz,
+ struct page_frag *pfrag,
+ gfp_t gfp_mask,
+ unsigned int align_mask)
+{
+ if (unlikely(!__page_frag_cache_prepare(nc, fragsz, pfrag, gfp_mask,
+ align_mask)))
+ return false;
+
+ __page_frag_cache_commit(nc, pfrag, fragsz);
+ return true;
+}
+
+static inline bool page_frag_refill_align(struct page_frag_cache *nc,
+ unsigned int fragsz,
+ struct page_frag *pfrag,
+ gfp_t gfp_mask, unsigned int align)
+{
+ WARN_ON_ONCE(!is_power_of_2(align));
+ return __page_frag_refill_align(nc, fragsz, pfrag, gfp_mask, -align);
+}
+
+static inline bool page_frag_refill(struct page_frag_cache *nc,
+ unsigned int fragsz,
+ struct page_frag *pfrag, gfp_t gfp_mask)
+{
+ return __page_frag_refill_align(nc, fragsz, pfrag, gfp_mask, ~0u);
+}
+
+static inline bool __page_frag_refill_prepare_align(struct page_frag_cache *nc,
+ unsigned int fragsz,
+ struct page_frag *pfrag,
+ gfp_t gfp_mask,
+ unsigned int align_mask)
+{
+ return !!__page_frag_cache_prepare(nc, fragsz, pfrag, gfp_mask,
+ align_mask);
+}
+
+static inline bool page_frag_refill_prepare_align(struct page_frag_cache *nc,
+ unsigned int fragsz,
+ struct page_frag *pfrag,
+ gfp_t gfp_mask,
+ unsigned int align)
+{
+ WARN_ON_ONCE(!is_power_of_2(align));
+ return __page_frag_refill_prepare_align(nc, fragsz, pfrag, gfp_mask,
+ -align);
+}
+
+static inline bool page_frag_refill_prepare(struct page_frag_cache *nc,
+ unsigned int fragsz,
+ struct page_frag *pfrag,
+ gfp_t gfp_mask)
+{
+ return __page_frag_refill_prepare_align(nc, fragsz, pfrag, gfp_mask,
+ ~0u);
+}
+
+static inline void *__page_frag_alloc_refill_prepare_align(struct page_frag_cache *nc,
+ unsigned int fragsz,
+ struct page_frag *pfrag,
+ gfp_t gfp_mask,
+ unsigned int align_mask)
+{
+ return __page_frag_cache_prepare(nc, fragsz, pfrag, gfp_mask, align_mask);
+}
+
+static inline void *page_frag_alloc_refill_prepare_align(struct page_frag_cache *nc,
+ unsigned int fragsz,
+ struct page_frag *pfrag,
+ gfp_t gfp_mask,
+ unsigned int align)
+{
+ WARN_ON_ONCE(!is_power_of_2(align));
+ return __page_frag_alloc_refill_prepare_align(nc, fragsz, pfrag,
+ gfp_mask, -align);
+}
+
+static inline void *page_frag_alloc_refill_prepare(struct page_frag_cache *nc,
+ unsigned int fragsz,
+ struct page_frag *pfrag,
+ gfp_t gfp_mask)
+{
+ return __page_frag_alloc_refill_prepare_align(nc, fragsz, pfrag,
+ gfp_mask, ~0u);
+}
+
+static inline void *page_frag_alloc_refill_probe(struct page_frag_cache *nc,
+ unsigned int fragsz,
+ struct page_frag *pfrag)
+{
+ return __page_frag_alloc_refill_probe_align(nc, fragsz, pfrag, ~0u);
+}
+
+static inline bool page_frag_refill_probe(struct page_frag_cache *nc,
+ unsigned int fragsz,
+ struct page_frag *pfrag)
+{
+ return !!page_frag_alloc_refill_probe(nc, fragsz, pfrag);
+}
+
+static inline void page_frag_commit(struct page_frag_cache *nc,
+ struct page_frag *pfrag,
+ unsigned int used_sz)
+{
+ __page_frag_cache_commit(nc, pfrag, used_sz);
+}
+
+static inline void page_frag_commit_noref(struct page_frag_cache *nc,
+ struct page_frag *pfrag,
+ unsigned int used_sz)
+{
+ __page_frag_cache_commit_noref(nc, pfrag, used_sz);
+}
+
+static inline void page_frag_alloc_abort(struct page_frag_cache *nc,
+ unsigned int fragsz)
+{
+ VM_BUG_ON(fragsz > nc->offset);
+
+ nc->pagecnt_bias++;
+ nc->offset -= fragsz;
+}
+
void page_frag_free(void *addr);
#endif
diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
index 4666dbec38eb..1e7757a433d0 100644
--- a/mm/page_frag_cache.c
+++ b/mm/page_frag_cache.c
@@ -117,6 +117,27 @@ unsigned int __page_frag_cache_commit_noref(struct page_frag_cache *nc,
}
EXPORT_SYMBOL(__page_frag_cache_commit_noref);
+void *__page_frag_alloc_refill_probe_align(struct page_frag_cache *nc,
+ unsigned int fragsz,
+ struct page_frag *pfrag,
+ unsigned int align_mask)
+{
+ unsigned long encoded_page = nc->encoded_page;
+ unsigned int size, offset;
+
+ size = page_frag_cache_page_size(encoded_page);
+ offset = __ALIGN_KERNEL_MASK(nc->offset, ~align_mask);
+ if (unlikely(!encoded_page || offset + fragsz > size))
+ return NULL;
+
+ pfrag->page = page_frag_encoded_page_ptr(encoded_page);
+ pfrag->size = size - offset;
+ pfrag->offset = offset;
+
+ return page_frag_encoded_page_address(encoded_page) + offset;
+}
+EXPORT_SYMBOL(__page_frag_alloc_refill_probe_align);
+
void *__page_frag_cache_prepare(struct page_frag_cache *nc, unsigned int fragsz,
struct page_frag *pfrag, gfp_t gfp_mask,
unsigned int align_mask)
--
2.33.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next v20 11/14] mm: page_frag: add testing for the newly added prepare API
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
` (9 preceding siblings ...)
2024-10-08 11:20 ` [PATCH net-next v20 10/14] mm: page_frag: introduce prepare/probe/commit API Yunsheng Lin
@ 2024-10-08 11:20 ` Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 12/14] net: replace page_frag with page_frag_cache Yunsheng Lin
` (2 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Alexander Duyck,
Andrew Morton, Shuah Khan, linux-mm, linux-kselftest
Add testing for the newly added prepare API, for both aligned
and non-aligned API, also probe API is also tested along with
prepare API.
CC: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
.../selftests/mm/page_frag/page_frag_test.c | 66 +++++++++++++++++--
tools/testing/selftests/mm/run_vmtests.sh | 4 ++
tools/testing/selftests/mm/test_page_frag.sh | 31 +++++++++
3 files changed, 96 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/mm/page_frag/page_frag_test.c b/tools/testing/selftests/mm/page_frag/page_frag_test.c
index 36543a129e40..567bcc6a181e 100644
--- a/tools/testing/selftests/mm/page_frag/page_frag_test.c
+++ b/tools/testing/selftests/mm/page_frag/page_frag_test.c
@@ -29,6 +29,10 @@ static bool test_align;
module_param(test_align, bool, 0);
MODULE_PARM_DESC(test_align, "use align API for testing");
+static bool test_prepare;
+module_param(test_prepare, bool, 0);
+MODULE_PARM_DESC(test_prepare, "use prepare API for testing");
+
static int test_alloc_len = 2048;
module_param(test_alloc_len, int, 0);
MODULE_PARM_DESC(test_alloc_len, "alloc len for testing");
@@ -68,6 +72,18 @@ static int page_frag_pop_thread(void *arg)
return 0;
}
+static void frag_frag_test_commit(struct page_frag_cache *nc,
+ struct page_frag *prepare_pfrag,
+ struct page_frag *probe_pfrag,
+ unsigned int used_sz)
+{
+ WARN_ON_ONCE(prepare_pfrag->page != probe_pfrag->page ||
+ prepare_pfrag->offset != probe_pfrag->offset ||
+ prepare_pfrag->size != probe_pfrag->size);
+
+ page_frag_commit(nc, prepare_pfrag, used_sz);
+}
+
static int page_frag_push_thread(void *arg)
{
struct ptr_ring *ring = arg;
@@ -80,13 +96,52 @@ static int page_frag_push_thread(void *arg)
int ret;
if (test_align) {
- va = page_frag_alloc_align(&test_nc, test_alloc_len,
- GFP_KERNEL, SMP_CACHE_BYTES);
+ if (test_prepare) {
+ struct page_frag prepare_frag, probe_frag;
+ void *probe_va;
+
+ va = page_frag_alloc_refill_prepare_align(&test_nc,
+ test_alloc_len,
+ &prepare_frag,
+ GFP_KERNEL,
+ SMP_CACHE_BYTES);
+
+ probe_va = __page_frag_alloc_refill_probe_align(&test_nc,
+ test_alloc_len,
+ &probe_frag,
+ -SMP_CACHE_BYTES);
+ WARN_ON_ONCE(va != probe_va);
+
+ if (likely(va))
+ frag_frag_test_commit(&test_nc, &prepare_frag,
+ &probe_frag, test_alloc_len);
+ } else {
+ va = page_frag_alloc_align(&test_nc,
+ test_alloc_len,
+ GFP_KERNEL,
+ SMP_CACHE_BYTES);
+ }
WARN_ONCE((unsigned long)va & (SMP_CACHE_BYTES - 1),
"unaligned va returned\n");
} else {
- va = page_frag_alloc(&test_nc, test_alloc_len, GFP_KERNEL);
+ if (test_prepare) {
+ struct page_frag prepare_frag, probe_frag;
+ void *probe_va;
+
+ va = page_frag_alloc_refill_prepare(&test_nc, test_alloc_len,
+ &prepare_frag, GFP_KERNEL);
+
+ probe_va = page_frag_alloc_refill_probe(&test_nc, test_alloc_len,
+ &probe_frag);
+
+ WARN_ON_ONCE(va != probe_va);
+ if (likely(va))
+ frag_frag_test_commit(&test_nc, &prepare_frag,
+ &probe_frag, test_alloc_len);
+ } else {
+ va = page_frag_alloc(&test_nc, test_alloc_len, GFP_KERNEL);
+ }
}
if (!va)
@@ -152,8 +207,9 @@ static int __init page_frag_test_init(void)
test_pushed, test_popped);
duration = (u64)ktime_us_delta(ktime_get(), start);
- pr_info("%d of iterations for %s testing took: %lluus\n", nr_test,
- test_align ? "aligned" : "non-aligned", duration);
+ pr_info("%d of iterations for %s %s API testing took: %lluus\n", nr_test,
+ test_align ? "aligned" : "non-aligned",
+ test_prepare ? "prepare" : "alloc", duration);
ptr_ring_cleanup(&ptr_ring, NULL);
page_frag_cache_drain(&test_nc);
diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
index 2c5394584af4..f6ff9080a6f2 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -464,6 +464,10 @@ CATEGORY="page_frag" run_test ./test_page_frag.sh aligned
CATEGORY="page_frag" run_test ./test_page_frag.sh nonaligned
+CATEGORY="page_frag" run_test ./test_page_frag.sh aligned_prepare
+
+CATEGORY="page_frag" run_test ./test_page_frag.sh nonaligned_prepare
+
echo "SUMMARY: PASS=${count_pass} SKIP=${count_skip} FAIL=${count_fail}" | tap_prefix
echo "1..${count_total}" | tap_output
diff --git a/tools/testing/selftests/mm/test_page_frag.sh b/tools/testing/selftests/mm/test_page_frag.sh
index d750d910c899..71c3531fa38e 100755
--- a/tools/testing/selftests/mm/test_page_frag.sh
+++ b/tools/testing/selftests/mm/test_page_frag.sh
@@ -36,6 +36,8 @@ ksft_skip=4
SMOKE_PARAM="test_push_cpu=$TEST_CPU_0 test_pop_cpu=$TEST_CPU_1"
NONALIGNED_PARAM="$SMOKE_PARAM test_alloc_len=75 nr_test=$NR_TEST"
ALIGNED_PARAM="$NONALIGNED_PARAM test_align=1"
+NONALIGNED_PREPARE_PARAM="$NONALIGNED_PARAM test_prepare=1"
+ALIGNED_PREPARE_PARAM="$ALIGNED_PARAM test_prepare=1"
check_test_requirements()
{
@@ -74,6 +76,24 @@ run_aligned_check()
echo "Check the kernel ring buffer to see the summary."
}
+run_nonaligned_prepare_check()
+{
+ echo "Run performance tests to evaluate how fast nonaligned prepare API is."
+
+ insmod $DRIVER $NONALIGNED_PREPARE_PARAM > /dev/null 2>&1
+ echo "Done."
+ echo "Ccheck the kernel ring buffer to see the summary."
+}
+
+run_aligned_prepare_check()
+{
+ echo "Run performance tests to evaluate how fast aligned prepare API is."
+
+ insmod $DRIVER $ALIGNED_PREPARE_PARAM > /dev/null 2>&1
+ echo "Done."
+ echo "Check the kernel ring buffer to see the summary."
+}
+
run_smoke_check()
{
echo "Run smoke test."
@@ -86,6 +106,7 @@ run_smoke_check()
usage()
{
echo -n "Usage: $0 [ aligned ] | [ nonaligned ] | | [ smoke ] | "
+ echo "[ aligned_prepare ] | [ nonaligned_prepare ] | "
echo "manual parameters"
echo
echo "Valid tests and parameters:"
@@ -106,6 +127,12 @@ usage()
echo "# Performance testing for aligned alloc API"
echo "$0 aligned"
echo
+ echo "# Performance testing for nonaligned prepare API"
+ echo "$0 nonaligned_prepare"
+ echo
+ echo "# Performance testing for aligned prepare API"
+ echo "$0 aligned_prepare"
+ echo
exit 0
}
@@ -159,6 +186,10 @@ function run_test()
run_nonaligned_check
elif [[ "$1" = "aligned" ]]; then
run_aligned_check
+ elif [[ "$1" = "nonaligned_prepare" ]]; then
+ run_nonaligned_prepare_check
+ elif [[ "$1" = "aligned_prepare" ]]; then
+ run_aligned_prepare_check
else
run_manual_check $@
fi
--
2.33.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next v20 12/14] net: replace page_frag with page_frag_cache
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
` (10 preceding siblings ...)
2024-10-08 11:20 ` [PATCH net-next v20 11/14] mm: page_frag: add testing for the newly added prepare API Yunsheng Lin
@ 2024-10-08 11:20 ` Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 13/14] mm: page_frag: update documentation for page_frag Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 14/14] mm: page_frag: add an entry in MAINTAINERS " Yunsheng Lin
13 siblings, 0 replies; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Alexander Duyck, Ayush Sawal,
Eric Dumazet, Willem de Bruijn, Jason Wang, Ingo Molnar,
Peter Zijlstra, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
John Fastabend, Jakub Sitnicki, David Ahern, Matthieu Baerts,
Mat Martineau, Geliang Tang, Jamal Hadi Salim, Cong Wang,
Jiri Pirko, Boris Pismenny, bpf, mptcp
Use the newly introduced prepare/probe/commit API to
replace page_frag with page_frag_cache for sk_page_frag().
CC: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
.../chelsio/inline_crypto/chtls/chtls.h | 3 -
.../chelsio/inline_crypto/chtls/chtls_io.c | 101 +++++-------------
.../chelsio/inline_crypto/chtls/chtls_main.c | 3 -
drivers/net/tun.c | 47 ++++----
include/linux/sched.h | 2 +-
include/net/sock.h | 21 ++--
kernel/exit.c | 3 +-
kernel/fork.c | 3 +-
net/core/skbuff.c | 58 +++++-----
net/core/skmsg.c | 12 ++-
net/core/sock.c | 32 ++++--
net/ipv4/ip_output.c | 28 +++--
net/ipv4/tcp.c | 23 ++--
net/ipv4/tcp_output.c | 25 +++--
net/ipv6/ip6_output.c | 28 +++--
net/kcm/kcmsock.c | 18 ++--
net/mptcp/protocol.c | 47 ++++----
net/sched/em_meta.c | 2 +-
net/tls/tls_device.c | 100 ++++++++++-------
19 files changed, 294 insertions(+), 262 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls.h b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls.h
index 7ff82b6778ba..fe2b6a8ef718 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls.h
+++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls.h
@@ -234,7 +234,6 @@ struct chtls_dev {
struct list_head list_node;
struct list_head rcu_node;
struct list_head na_node;
- unsigned int send_page_order;
int max_host_sndbuf;
u32 round_robin_cnt;
struct key_map kmap;
@@ -453,8 +452,6 @@ enum {
/* The ULP mode/submode of an skbuff */
#define skb_ulp_mode(skb) (ULP_SKB_CB(skb)->ulp_mode)
-#define TCP_PAGE(sk) (sk->sk_frag.page)
-#define TCP_OFF(sk) (sk->sk_frag.offset)
static inline struct chtls_dev *to_chtls_dev(struct tls_toe_device *tlsdev)
{
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_io.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_io.c
index d567e42e1760..48a61b1142a3 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_io.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_io.c
@@ -825,12 +825,6 @@ void skb_entail(struct sock *sk, struct sk_buff *skb, int flags)
ULP_SKB_CB(skb)->flags = flags;
__skb_queue_tail(&csk->txq, skb);
sk->sk_wmem_queued += skb->truesize;
-
- if (TCP_PAGE(sk) && TCP_OFF(sk)) {
- put_page(TCP_PAGE(sk));
- TCP_PAGE(sk) = NULL;
- TCP_OFF(sk) = 0;
- }
}
static struct sk_buff *get_tx_skb(struct sock *sk, int size)
@@ -882,16 +876,12 @@ static void push_frames_if_head(struct sock *sk)
chtls_push_frames(csk, 1);
}
-static int chtls_skb_copy_to_page_nocache(struct sock *sk,
- struct iov_iter *from,
- struct sk_buff *skb,
- struct page *page,
- int off, int copy)
+static int chtls_skb_copy_to_va_nocache(struct sock *sk, struct iov_iter *from,
+ struct sk_buff *skb, char *va, int copy)
{
int err;
- err = skb_do_copy_data_nocache(sk, skb, from, page_address(page) +
- off, copy, skb->len);
+ err = skb_do_copy_data_nocache(sk, skb, from, va, copy, skb->len);
if (err)
return err;
@@ -1114,82 +1104,45 @@ int chtls_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
if (err)
goto do_fault;
} else {
+ struct page_frag_cache *nc = &sk->sk_frag;
+ struct page_frag page_frag, *pfrag;
int i = skb_shinfo(skb)->nr_frags;
- struct page *page = TCP_PAGE(sk);
- int pg_size = PAGE_SIZE;
- int off = TCP_OFF(sk);
- bool merge;
-
- if (page)
- pg_size = page_size(page);
- if (off < pg_size &&
- skb_can_coalesce(skb, i, page, off)) {
+ bool merge = false;
+ void *va;
+
+ pfrag = &page_frag;
+ va = page_frag_alloc_refill_prepare(nc, 32U, pfrag,
+ sk->sk_allocation);
+ if (unlikely(!va))
+ goto wait_for_memory;
+
+ if (skb_can_coalesce(skb, i, pfrag->page,
+ pfrag->offset))
merge = true;
- goto copy;
- }
- merge = false;
- if (i == (is_tls_tx(csk) ? (MAX_SKB_FRAGS - 1) :
- MAX_SKB_FRAGS))
+ else if (i == (is_tls_tx(csk) ? (MAX_SKB_FRAGS - 1) :
+ MAX_SKB_FRAGS))
goto new_buf;
- if (page && off == pg_size) {
- put_page(page);
- TCP_PAGE(sk) = page = NULL;
- pg_size = PAGE_SIZE;
- }
-
- if (!page) {
- gfp_t gfp = sk->sk_allocation;
- int order = cdev->send_page_order;
-
- if (order) {
- page = alloc_pages(gfp | __GFP_COMP |
- __GFP_NOWARN |
- __GFP_NORETRY,
- order);
- if (page)
- pg_size <<= order;
- }
- if (!page) {
- page = alloc_page(gfp);
- pg_size = PAGE_SIZE;
- }
- if (!page)
- goto wait_for_memory;
- off = 0;
- }
-copy:
- if (copy > pg_size - off)
- copy = pg_size - off;
+ copy = min_t(int, copy, pfrag->size);
if (is_tls_tx(csk))
copy = min_t(int, copy, csk->tlshws.txleft);
- err = chtls_skb_copy_to_page_nocache(sk, &msg->msg_iter,
- skb, page,
- off, copy);
- if (unlikely(err)) {
- if (!TCP_PAGE(sk)) {
- TCP_PAGE(sk) = page;
- TCP_OFF(sk) = 0;
- }
+ err = chtls_skb_copy_to_va_nocache(sk, &msg->msg_iter,
+ skb, va, copy);
+ if (unlikely(err))
goto do_fault;
- }
+
/* Update the skb. */
if (merge) {
skb_frag_size_add(
&skb_shinfo(skb)->frags[i - 1],
copy);
+ page_frag_commit_noref(nc, pfrag, copy);
} else {
- skb_fill_page_desc(skb, i, page, off, copy);
- if (off + copy < pg_size) {
- /* space left keep page */
- get_page(page);
- TCP_PAGE(sk) = page;
- } else {
- TCP_PAGE(sk) = NULL;
- }
+ skb_fill_page_desc(skb, i, pfrag->page,
+ pfrag->offset, copy);
+ page_frag_commit(nc, pfrag, copy);
}
- TCP_OFF(sk) = off + copy;
}
if (unlikely(skb->len == mss))
tx_skb_finalize(skb);
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_main.c b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_main.c
index 455a54708be4..ba88b2fc7cd8 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_main.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_main.c
@@ -34,7 +34,6 @@ static DEFINE_MUTEX(notify_mutex);
static RAW_NOTIFIER_HEAD(listen_notify_list);
static struct proto chtls_cpl_prot, chtls_cpl_protv6;
struct request_sock_ops chtls_rsk_ops, chtls_rsk_opsv6;
-static uint send_page_order = (14 - PAGE_SHIFT < 0) ? 0 : 14 - PAGE_SHIFT;
static void register_listen_notifier(struct notifier_block *nb)
{
@@ -273,8 +272,6 @@ static void *chtls_uld_add(const struct cxgb4_lld_info *info)
INIT_WORK(&cdev->deferq_task, process_deferq);
spin_lock_init(&cdev->listen_lock);
spin_lock_init(&cdev->idr_lock);
- cdev->send_page_order = min_t(uint, get_order(32768),
- send_page_order);
cdev->max_host_sndbuf = 48 * 1024;
if (lldi->vr->key.size)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index d7a865ef370b..13113a1dca29 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1599,21 +1599,19 @@ static bool tun_can_build_skb(struct tun_struct *tun, struct tun_file *tfile,
}
static struct sk_buff *__tun_build_skb(struct tun_file *tfile,
- struct page_frag *alloc_frag, char *buf,
- int buflen, int len, int pad)
+ char *buf, int buflen, int len, int pad)
{
struct sk_buff *skb = build_skb(buf, buflen);
- if (!skb)
+ if (!skb) {
+ page_frag_free(buf);
return ERR_PTR(-ENOMEM);
+ }
skb_reserve(skb, pad);
skb_put(skb, len);
skb_set_owner_w(skb, tfile->socket.sk);
- get_page(alloc_frag->page);
- alloc_frag->offset += buflen;
-
return skb;
}
@@ -1661,8 +1659,8 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
struct virtio_net_hdr *hdr,
int len, int *skb_xdp)
{
- struct page_frag *alloc_frag = ¤t->task_frag;
struct bpf_net_context __bpf_net_ctx, *bpf_net_ctx;
+ struct page_frag_cache *nc = ¤t->task_frag;
struct bpf_prog *xdp_prog;
int buflen = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
char *buf;
@@ -1677,16 +1675,16 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
buflen += SKB_DATA_ALIGN(len + pad);
rcu_read_unlock();
- alloc_frag->offset = ALIGN((u64)alloc_frag->offset, SMP_CACHE_BYTES);
- if (unlikely(!skb_page_frag_refill(buflen, alloc_frag, GFP_KERNEL)))
+ buf = page_frag_alloc_align(nc, buflen, GFP_KERNEL,
+ SMP_CACHE_BYTES);
+ if (unlikely(!buf))
return ERR_PTR(-ENOMEM);
- buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
- copied = copy_page_from_iter(alloc_frag->page,
- alloc_frag->offset + pad,
- len, from);
- if (copied != len)
+ copied = copy_from_iter(buf + pad, len, from);
+ if (copied != len) {
+ page_frag_alloc_abort(nc, buflen);
return ERR_PTR(-EFAULT);
+ }
/* There's a small window that XDP may be set after the check
* of xdp_prog above, this should be rare and for simplicity
@@ -1694,8 +1692,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
*/
if (hdr->gso_type || !xdp_prog) {
*skb_xdp = 1;
- return __tun_build_skb(tfile, alloc_frag, buf, buflen, len,
- pad);
+ return __tun_build_skb(tfile, buf, buflen, len, pad);
}
*skb_xdp = 0;
@@ -1712,21 +1709,23 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
xdp_prepare_buff(&xdp, buf, pad, len, false);
act = bpf_prog_run_xdp(xdp_prog, &xdp);
- if (act == XDP_REDIRECT || act == XDP_TX) {
- get_page(alloc_frag->page);
- alloc_frag->offset += buflen;
- }
err = tun_xdp_act(tun, xdp_prog, &xdp, act);
if (err < 0) {
- if (act == XDP_REDIRECT || act == XDP_TX)
- put_page(alloc_frag->page);
+ if (act == XDP_REDIRECT || act == XDP_TX) {
+ page_frag_alloc_abort(nc, 0);
+ goto out;
+ }
+
+ page_frag_alloc_abort(nc, buflen);
goto out;
}
if (err == XDP_REDIRECT)
xdp_do_flush();
- if (err != XDP_PASS)
+ if (err != XDP_PASS) {
+ page_frag_alloc_abort(nc, buflen);
goto out;
+ }
pad = xdp.data - xdp.data_hard_start;
len = xdp.data_end - xdp.data;
@@ -1735,7 +1734,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
rcu_read_unlock();
local_bh_enable();
- return __tun_build_skb(tfile, alloc_frag, buf, buflen, len, pad);
+ return __tun_build_skb(tfile, buf, buflen, len, pad);
out:
bpf_net_ctx_clear(bpf_net_ctx);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index e6ee4258169a..d4a4273d015e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1379,7 +1379,7 @@ struct task_struct {
/* Cache last used pipe for splice(): */
struct pipe_inode_info *splice_pipe;
- struct page_frag task_frag;
+ struct page_frag_cache task_frag;
#ifdef CONFIG_TASK_DELAY_ACCT
struct task_delay_info *delays;
diff --git a/include/net/sock.h b/include/net/sock.h
index e0b4e2daca5d..23355f7d2040 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -303,7 +303,7 @@ struct sk_filter;
* @sk_stamp: time stamp of last packet received
* @sk_stamp_seq: lock for accessing sk_stamp on 32 bit architectures only
* @sk_tsflags: SO_TIMESTAMPING flags
- * @sk_use_task_frag: allow sk_page_frag() to use current->task_frag.
+ * @sk_use_task_frag: allow sk_page_frag_cache() to use current->task_frag.
* Sockets that can be used under memory reclaim should
* set this to false.
* @sk_bind_phc: SO_TIMESTAMPING bind PHC index of PTP virtual clock
@@ -462,7 +462,7 @@ struct sock {
struct sk_buff_head sk_write_queue;
u32 sk_dst_pending_confirm;
u32 sk_pacing_status; /* see enum sk_pacing */
- struct page_frag sk_frag;
+ struct page_frag_cache sk_frag;
struct timer_list sk_timer;
unsigned long sk_pacing_rate; /* bytes per second */
@@ -2478,22 +2478,22 @@ static inline void sk_stream_moderate_sndbuf(struct sock *sk)
}
/**
- * sk_page_frag - return an appropriate page_frag
+ * sk_page_frag_cache - return an appropriate page_frag_cache
* @sk: socket
*
- * Use the per task page_frag instead of the per socket one for
+ * Use the per task page_frag_cache instead of the per socket one for
* optimization when we know that we're in process context and own
* everything that's associated with %current.
*
* Both direct reclaim and page faults can nest inside other
- * socket operations and end up recursing into sk_page_frag()
- * while it's already in use: explicitly avoid task page_frag
+ * socket operations and end up recursing into sk_page_frag_cache()
+ * while it's already in use: explicitly avoid task page_frag_cache
* when users disable sk_use_task_frag.
*
* Return: a per task page_frag if context allows that,
* otherwise a per socket one.
*/
-static inline struct page_frag *sk_page_frag(struct sock *sk)
+static inline struct page_frag_cache *sk_page_frag_cache(struct sock *sk)
{
if (sk->sk_use_task_frag)
return ¤t->task_frag;
@@ -2501,7 +2501,12 @@ static inline struct page_frag *sk_page_frag(struct sock *sk)
return &sk->sk_frag;
}
-bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag);
+bool sk_page_frag_refill_prepare(struct sock *sk, struct page_frag_cache *nc,
+ struct page_frag *pfrag);
+
+void *sk_page_frag_alloc_refill_prepare(struct sock *sk,
+ struct page_frag_cache *nc,
+ struct page_frag *pfrag);
/*
* Default write policy as shown to user space via poll/select/SIGIO
diff --git a/kernel/exit.c b/kernel/exit.c
index 619f0014c33b..5f9b7f58098d 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -974,8 +974,7 @@ void __noreturn do_exit(long code)
if (tsk->splice_pipe)
free_pipe_info(tsk->splice_pipe);
- if (tsk->task_frag.page)
- put_page(tsk->task_frag.page);
+ page_frag_cache_drain(&tsk->task_frag);
exit_task_stack_account(tsk);
diff --git a/kernel/fork.c b/kernel/fork.c
index 60c0b4868fd4..a197ccfac468 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -81,6 +81,7 @@
#include <linux/tty.h>
#include <linux/fs_struct.h>
#include <linux/magic.h>
+#include <linux/page_frag_cache.h>
#include <linux/perf_event.h>
#include <linux/posix-timers.h>
#include <linux/user-return-notifier.h>
@@ -1159,10 +1160,10 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
tsk->btrace_seq = 0;
#endif
tsk->splice_pipe = NULL;
- tsk->task_frag.page = NULL;
tsk->wake_q.next = NULL;
tsk->worker_private = NULL;
+ page_frag_cache_init(&tsk->task_frag);
kcov_task_init(tsk);
kmsan_task_create(tsk);
kmap_local_fork(tsk);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index ca01880c7ad0..97e02892bed8 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3062,25 +3062,6 @@ static void sock_spd_release(struct splice_pipe_desc *spd, unsigned int i)
put_page(spd->pages[i]);
}
-static struct page *linear_to_page(struct page *page, unsigned int *len,
- unsigned int *offset,
- struct sock *sk)
-{
- struct page_frag *pfrag = sk_page_frag(sk);
-
- if (!sk_page_frag_refill(sk, pfrag))
- return NULL;
-
- *len = min_t(unsigned int, *len, pfrag->size - pfrag->offset);
-
- memcpy(page_address(pfrag->page) + pfrag->offset,
- page_address(page) + *offset, *len);
- *offset = pfrag->offset;
- pfrag->offset += *len;
-
- return pfrag->page;
-}
-
static bool spd_can_coalesce(const struct splice_pipe_desc *spd,
struct page *page,
unsigned int offset)
@@ -3091,6 +3072,37 @@ static bool spd_can_coalesce(const struct splice_pipe_desc *spd,
spd->partial[spd->nr_pages - 1].len == offset);
}
+static bool spd_fill_linear_page(struct splice_pipe_desc *spd,
+ struct page *page, unsigned int offset,
+ unsigned int *len, struct sock *sk)
+{
+ struct page_frag_cache *nc = sk_page_frag_cache(sk);
+ struct page_frag page_frag, *pfrag;
+ void *va;
+
+ pfrag = &page_frag;
+ va = sk_page_frag_alloc_refill_prepare(sk, nc, pfrag);
+ if (!va)
+ return true;
+
+ *len = min_t(unsigned int, *len, pfrag->size);
+ memcpy(va, page_address(page) + offset, *len);
+
+ if (spd_can_coalesce(spd, pfrag->page, pfrag->offset)) {
+ spd->partial[spd->nr_pages - 1].len += *len;
+ page_frag_commit_noref(nc, pfrag, *len);
+ return false;
+ }
+
+ page_frag_commit(nc, pfrag, *len);
+ spd->pages[spd->nr_pages] = pfrag->page;
+ spd->partial[spd->nr_pages].len = *len;
+ spd->partial[spd->nr_pages].offset = pfrag->offset;
+ spd->nr_pages++;
+
+ return false;
+}
+
/*
* Fill page/offset/length into spd, if it can hold more pages.
*/
@@ -3103,11 +3115,9 @@ static bool spd_fill_page(struct splice_pipe_desc *spd,
if (unlikely(spd->nr_pages == MAX_SKB_FRAGS))
return true;
- if (linear) {
- page = linear_to_page(page, len, &offset, sk);
- if (!page)
- return true;
- }
+ if (linear)
+ return spd_fill_linear_page(spd, page, offset, len, sk);
+
if (spd_can_coalesce(spd, page, offset)) {
spd->partial[spd->nr_pages - 1].len += *len;
return false;
diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index b1dcbd3be89e..b19eafeeb8ac 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -27,23 +27,25 @@ static bool sk_msg_try_coalesce_ok(struct sk_msg *msg, int elem_first_coalesce)
int sk_msg_alloc(struct sock *sk, struct sk_msg *msg, int len,
int elem_first_coalesce)
{
- struct page_frag *pfrag = sk_page_frag(sk);
+ struct page_frag_cache *nc = sk_page_frag_cache(sk);
u32 osize = msg->sg.size;
int ret = 0;
len -= msg->sg.size;
while (len > 0) {
+ struct page_frag page_frag, *pfrag;
struct scatterlist *sge;
u32 orig_offset;
int use, i;
- if (!sk_page_frag_refill(sk, pfrag)) {
+ pfrag = &page_frag;
+ if (!sk_page_frag_refill_prepare(sk, nc, pfrag)) {
ret = -ENOMEM;
goto msg_trim;
}
orig_offset = pfrag->offset;
- use = min_t(int, len, pfrag->size - orig_offset);
+ use = min_t(int, len, pfrag->size);
if (!sk_wmem_schedule(sk, use)) {
ret = -ENOMEM;
goto msg_trim;
@@ -57,6 +59,7 @@ int sk_msg_alloc(struct sock *sk, struct sk_msg *msg, int len,
sg_page(sge) == pfrag->page &&
sge->offset + sge->length == orig_offset) {
sge->length += use;
+ page_frag_commit_noref(nc, pfrag, use);
} else {
if (sk_msg_full(msg)) {
ret = -ENOSPC;
@@ -66,13 +69,12 @@ int sk_msg_alloc(struct sock *sk, struct sk_msg *msg, int len,
sge = &msg->sg.data[msg->sg.end];
sg_unmark_end(sge);
sg_set_page(sge, pfrag->page, use, orig_offset);
- get_page(pfrag->page);
+ page_frag_commit(nc, pfrag, use);
sk_msg_iter_next(msg, end);
}
sk_mem_charge(sk, use);
msg->sg.size += use;
- pfrag->offset += use;
len -= use;
}
diff --git a/net/core/sock.c b/net/core/sock.c
index 846f494a17cf..5afee5468130 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2275,10 +2275,7 @@ static void __sk_destruct(struct rcu_head *head)
pr_debug("%s: optmem leakage (%d bytes) detected\n",
__func__, atomic_read(&sk->sk_omem_alloc));
- if (sk->sk_frag.page) {
- put_page(sk->sk_frag.page);
- sk->sk_frag.page = NULL;
- }
+ page_frag_cache_drain(&sk->sk_frag);
/* We do not need to acquire sk->sk_peer_lock, we are the last user. */
put_cred(sk->sk_peer_cred);
@@ -3037,16 +3034,33 @@ bool skb_page_frag_refill(unsigned int sz, struct page_frag *pfrag, gfp_t gfp)
}
EXPORT_SYMBOL(skb_page_frag_refill);
-bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
+bool sk_page_frag_refill_prepare(struct sock *sk, struct page_frag_cache *nc,
+ struct page_frag *pfrag)
{
- if (likely(skb_page_frag_refill(32U, pfrag, sk->sk_allocation)))
+ if (likely(page_frag_refill_prepare(nc, 32U, pfrag, sk->sk_allocation)))
return true;
sk_enter_memory_pressure(sk);
sk_stream_moderate_sndbuf(sk);
return false;
}
-EXPORT_SYMBOL(sk_page_frag_refill);
+EXPORT_SYMBOL(sk_page_frag_refill_prepare);
+
+void *sk_page_frag_alloc_refill_prepare(struct sock *sk,
+ struct page_frag_cache *nc,
+ struct page_frag *pfrag)
+{
+ void *va;
+
+ va = page_frag_alloc_refill_prepare(nc, 32U, pfrag, sk->sk_allocation);
+ if (likely(va))
+ return va;
+
+ sk_enter_memory_pressure(sk);
+ sk_stream_moderate_sndbuf(sk);
+ return NULL;
+}
+EXPORT_SYMBOL(sk_page_frag_alloc_refill_prepare);
void __lock_sock(struct sock *sk)
__releases(&sk->sk_lock.slock)
@@ -3568,8 +3582,8 @@ void sock_init_data_uid(struct socket *sock, struct sock *sk, kuid_t uid)
sk->sk_error_report = sock_def_error_report;
sk->sk_destruct = sock_def_destruct;
- sk->sk_frag.page = NULL;
- sk->sk_frag.offset = 0;
+ page_frag_cache_init(&sk->sk_frag);
+
sk->sk_peek_off = -1;
sk->sk_peer_pid = NULL;
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index e5c55a95063d..5e5f4e7350ee 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -953,7 +953,7 @@ static int __ip_append_data(struct sock *sk,
struct flowi4 *fl4,
struct sk_buff_head *queue,
struct inet_cork *cork,
- struct page_frag *pfrag,
+ struct page_frag_cache *nc,
int getfrag(void *from, char *to, int offset,
int len, int odd, struct sk_buff *skb),
void *from, int length, int transhdrlen,
@@ -1233,13 +1233,19 @@ static int __ip_append_data(struct sock *sk,
copy = err;
wmem_alloc_delta += copy;
} else if (!zc) {
+ struct page_frag page_frag, *pfrag;
int i = skb_shinfo(skb)->nr_frags;
+ void *va;
err = -ENOMEM;
- if (!sk_page_frag_refill(sk, pfrag))
+ pfrag = &page_frag;
+ va = sk_page_frag_alloc_refill_prepare(sk, nc, pfrag);
+ if (!va)
goto error;
skb_zcopy_downgrade_managed(skb);
+ copy = min_t(int, copy, pfrag->size);
+
if (!skb_can_coalesce(skb, i, pfrag->page,
pfrag->offset)) {
err = -EMSGSIZE;
@@ -1247,18 +1253,18 @@ static int __ip_append_data(struct sock *sk,
goto error;
__skb_fill_page_desc(skb, i, pfrag->page,
- pfrag->offset, 0);
+ pfrag->offset, copy);
skb_shinfo(skb)->nr_frags = ++i;
- get_page(pfrag->page);
+ page_frag_commit(nc, pfrag, copy);
+ } else {
+ skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1],
+ copy);
+ page_frag_commit_noref(nc, pfrag, copy);
}
- copy = min_t(int, copy, pfrag->size - pfrag->offset);
- if (getfrag(from,
- page_address(pfrag->page) + pfrag->offset,
- offset, copy, skb->len, skb) < 0)
+
+ if (getfrag(from, va, offset, copy, skb->len, skb) < 0)
goto error_efault;
- pfrag->offset += copy;
- skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1], copy);
skb_len_add(skb, copy);
wmem_alloc_delta += copy;
} else {
@@ -1373,7 +1379,7 @@ int ip_append_data(struct sock *sk, struct flowi4 *fl4,
}
return __ip_append_data(sk, fl4, &sk->sk_write_queue, &inet->cork.base,
- sk_page_frag(sk), getfrag,
+ sk_page_frag_cache(sk), getfrag,
from, length, transhdrlen, flags);
}
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index bc36f02cac0d..2c66c016a54c 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1193,9 +1193,13 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
if (zc == 0) {
bool merge = true;
int i = skb_shinfo(skb)->nr_frags;
- struct page_frag *pfrag = sk_page_frag(sk);
+ struct page_frag_cache *nc = sk_page_frag_cache(sk);
+ struct page_frag page_frag, *pfrag;
+ void *va;
- if (!sk_page_frag_refill(sk, pfrag))
+ pfrag = &page_frag;
+ va = sk_page_frag_alloc_refill_prepare(sk, nc, pfrag);
+ if (!va)
goto wait_for_space;
if (!skb_can_coalesce(skb, i, pfrag->page,
@@ -1207,7 +1211,7 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
merge = false;
}
- copy = min_t(int, copy, pfrag->size - pfrag->offset);
+ copy = min_t(int, copy, pfrag->size);
if (unlikely(skb_zcopy_pure(skb) || skb_zcopy_managed(skb))) {
if (tcp_downgrade_zcopy_pure(sk, skb))
@@ -1220,20 +1224,19 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
goto wait_for_space;
err = skb_add_frag_nocache(sk, &msg->msg_iter, skb,
- page_address(pfrag->page) +
- pfrag->offset, copy);
+ va, copy);
if (err)
goto do_error;
/* Update the skb. */
if (merge) {
skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1], copy);
+ page_frag_commit_noref(nc, pfrag, copy);
} else {
skb_fill_page_desc(skb, i, pfrag->page,
pfrag->offset, copy);
- page_ref_inc(pfrag->page);
+ page_frag_commit(nc, pfrag, copy);
}
- pfrag->offset += copy;
} else if (zc == MSG_ZEROCOPY) {
/* First append to a fragless skb builds initial
* pure zerocopy skb
@@ -3393,11 +3396,7 @@ int tcp_disconnect(struct sock *sk, int flags)
WARN_ON(inet->inet_num && !icsk->icsk_bind_hash);
- if (sk->sk_frag.page) {
- put_page(sk->sk_frag.page);
- sk->sk_frag.page = NULL;
- sk->sk_frag.offset = 0;
- }
+ page_frag_cache_drain(&sk->sk_frag);
sk_error_report(sk);
return 0;
}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 08772395690d..d53d77500204 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3973,9 +3973,11 @@ static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn)
struct inet_connection_sock *icsk = inet_csk(sk);
struct tcp_sock *tp = tcp_sk(sk);
struct tcp_fastopen_request *fo = tp->fastopen_req;
- struct page_frag *pfrag = sk_page_frag(sk);
+ struct page_frag_cache *nc = sk_page_frag_cache(sk);
+ struct page_frag page_frag, *pfrag;
struct sk_buff *syn_data;
int space, err = 0;
+ void *va;
tp->rx_opt.mss_clamp = tp->advmss; /* If MSS is not cached */
if (!tcp_fastopen_cookie_check(sk, &tp->rx_opt.mss_clamp, &fo->cookie))
@@ -3994,21 +3996,25 @@ static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn)
space = min_t(size_t, space, fo->size);
- if (space &&
- !skb_page_frag_refill(min_t(size_t, space, PAGE_SIZE),
- pfrag, sk->sk_allocation))
- goto fallback;
+ if (space) {
+ pfrag = &page_frag;
+ va = page_frag_alloc_refill_prepare(nc,
+ min_t(size_t, space, PAGE_SIZE),
+ pfrag, sk->sk_allocation);
+ if (!va)
+ goto fallback;
+ }
+
syn_data = tcp_stream_alloc_skb(sk, sk->sk_allocation, false);
if (!syn_data)
goto fallback;
memcpy(syn_data->cb, syn->cb, sizeof(syn->cb));
if (space) {
- space = min_t(size_t, space, pfrag->size - pfrag->offset);
+ space = min_t(size_t, space, pfrag->size);
space = tcp_wmem_schedule(sk, space);
}
if (space) {
- space = copy_page_from_iter(pfrag->page, pfrag->offset,
- space, &fo->data->msg_iter);
+ space = _copy_from_iter(va, space, &fo->data->msg_iter);
if (unlikely(!space)) {
tcp_skb_tsorted_anchor_cleanup(syn_data);
kfree_skb(syn_data);
@@ -4016,8 +4022,7 @@ static int tcp_send_syn_data(struct sock *sk, struct sk_buff *syn)
}
skb_fill_page_desc(syn_data, 0, pfrag->page,
pfrag->offset, space);
- page_ref_inc(pfrag->page);
- pfrag->offset += space;
+ page_frag_commit(nc, pfrag, space);
skb_len_add(syn_data, space);
skb_zcopy_set(syn_data, fo->uarg, NULL);
}
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 205673179b3c..f48ee0096a31 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1416,7 +1416,7 @@ static int __ip6_append_data(struct sock *sk,
struct sk_buff_head *queue,
struct inet_cork_full *cork_full,
struct inet6_cork *v6_cork,
- struct page_frag *pfrag,
+ struct page_frag_cache *nc,
int getfrag(void *from, char *to, int offset,
int len, int odd, struct sk_buff *skb),
void *from, size_t length, int transhdrlen,
@@ -1762,13 +1762,19 @@ static int __ip6_append_data(struct sock *sk,
copy = err;
wmem_alloc_delta += copy;
} else if (!zc) {
+ struct page_frag page_frag, *pfrag;
int i = skb_shinfo(skb)->nr_frags;
+ void *va;
err = -ENOMEM;
- if (!sk_page_frag_refill(sk, pfrag))
+ pfrag = &page_frag;
+ va = sk_page_frag_alloc_refill_prepare(sk, nc, pfrag);
+ if (!va)
goto error;
skb_zcopy_downgrade_managed(skb);
+ copy = min_t(int, copy, pfrag->size);
+
if (!skb_can_coalesce(skb, i, pfrag->page,
pfrag->offset)) {
err = -EMSGSIZE;
@@ -1776,18 +1782,18 @@ static int __ip6_append_data(struct sock *sk,
goto error;
__skb_fill_page_desc(skb, i, pfrag->page,
- pfrag->offset, 0);
+ pfrag->offset, copy);
skb_shinfo(skb)->nr_frags = ++i;
- get_page(pfrag->page);
+ page_frag_commit(nc, pfrag, copy);
+ } else {
+ skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1],
+ copy);
+ page_frag_commit_noref(nc, pfrag, copy);
}
- copy = min_t(int, copy, pfrag->size - pfrag->offset);
- if (getfrag(from,
- page_address(pfrag->page) + pfrag->offset,
- offset, copy, skb->len, skb) < 0)
+
+ if (getfrag(from, va, offset, copy, skb->len, skb) < 0)
goto error_efault;
- pfrag->offset += copy;
- skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1], copy);
skb->len += copy;
skb->data_len += copy;
skb->truesize += copy;
@@ -1850,7 +1856,7 @@ int ip6_append_data(struct sock *sk,
}
return __ip6_append_data(sk, &sk->sk_write_queue, &inet->cork,
- &np->cork, sk_page_frag(sk), getfrag,
+ &np->cork, sk_page_frag_cache(sk), getfrag,
from, length, transhdrlen, flags, ipc6);
}
EXPORT_SYMBOL_GPL(ip6_append_data);
diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index e74ad63f0189..96243f74f499 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -804,9 +804,13 @@ static int kcm_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
while (msg_data_left(msg)) {
bool merge = true;
int i = skb_shinfo(skb)->nr_frags;
- struct page_frag *pfrag = sk_page_frag(sk);
+ struct page_frag_cache *nc = sk_page_frag_cache(sk);
+ struct page_frag page_frag, *pfrag;
+ void *va;
- if (!sk_page_frag_refill(sk, pfrag))
+ pfrag = &page_frag;
+ va = sk_page_frag_alloc_refill_prepare(sk, nc, pfrag);
+ if (!va)
goto wait_for_memory;
if (!skb_can_coalesce(skb, i, pfrag->page,
@@ -851,14 +855,12 @@ static int kcm_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
if (head != skb)
head->truesize += copy;
} else {
- copy = min_t(int, msg_data_left(msg),
- pfrag->size - pfrag->offset);
+ copy = min_t(int, msg_data_left(msg), pfrag->size);
if (!sk_wmem_schedule(sk, copy))
goto wait_for_memory;
err = skb_add_frag_nocache(sk, &msg->msg_iter, skb,
- page_address(pfrag->page) +
- pfrag->offset, copy);
+ va, copy);
if (err)
goto out_error;
@@ -866,13 +868,13 @@ static int kcm_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
if (merge) {
skb_frag_size_add(
&skb_shinfo(skb)->frags[i - 1], copy);
+ page_frag_commit_noref(nc, pfrag, copy);
} else {
skb_fill_page_desc(skb, i, pfrag->page,
pfrag->offset, copy);
- get_page(pfrag->page);
+ page_frag_commit(nc, pfrag, copy);
}
- pfrag->offset += copy;
}
copied += copy;
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index e85862352084..128b0670f2b8 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -960,7 +960,6 @@ static bool mptcp_skb_can_collapse_to(u64 write_seq,
}
/* we can append data to the given data frag if:
- * - there is space available in the backing page_frag
* - the data frag tail matches the current page_frag free offset
* - the data frag end sequence number matches the current write seq
*/
@@ -969,7 +968,6 @@ static bool mptcp_frag_can_collapse_to(const struct mptcp_sock *msk,
const struct mptcp_data_frag *df)
{
return df && pfrag->page == df->page &&
- pfrag->size - pfrag->offset > 0 &&
pfrag->offset == (df->offset + df->data_len) &&
df->data_seq + df->data_len == msk->write_seq;
}
@@ -1085,14 +1083,20 @@ static void mptcp_enter_memory_pressure(struct sock *sk)
/* ensure we get enough memory for the frag hdr, beyond some minimal amount of
* data
*/
-static bool mptcp_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
+static void *mptcp_page_frag_alloc_refill_prepare(struct sock *sk,
+ struct page_frag_cache *nc,
+ struct page_frag *pfrag)
{
- if (likely(skb_page_frag_refill(32U + sizeof(struct mptcp_data_frag),
- pfrag, sk->sk_allocation)))
- return true;
+ unsigned int fragsz = 32U + sizeof(struct mptcp_data_frag);
+ void *va;
+
+ va = page_frag_alloc_refill_prepare(nc, fragsz, pfrag,
+ sk->sk_allocation);
+ if (likely(va))
+ return va;
mptcp_enter_memory_pressure(sk);
- return false;
+ return NULL;
}
static struct mptcp_data_frag *
@@ -1795,7 +1799,7 @@ static u32 mptcp_send_limit(const struct sock *sk)
static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
{
struct mptcp_sock *msk = mptcp_sk(sk);
- struct page_frag *pfrag;
+ struct page_frag_cache *nc;
size_t copied = 0;
int ret = 0;
long timeo;
@@ -1829,14 +1833,16 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
if (unlikely(sk->sk_err || (sk->sk_shutdown & SEND_SHUTDOWN)))
goto do_error;
- pfrag = sk_page_frag(sk);
+ nc = sk_page_frag_cache(sk);
while (msg_data_left(msg)) {
+ struct page_frag page_frag, *pfrag;
int total_ts, frag_truesize = 0;
struct mptcp_data_frag *dfrag;
bool dfrag_collapsed;
- size_t psize, offset;
u32 copy_limit;
+ size_t psize;
+ void *va;
/* ensure fitting the notsent_lowat() constraint */
copy_limit = mptcp_send_limit(sk);
@@ -1847,21 +1853,26 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
* page allocator
*/
dfrag = mptcp_pending_tail(sk);
- dfrag_collapsed = mptcp_frag_can_collapse_to(msk, pfrag, dfrag);
+ pfrag = &page_frag;
+ va = page_frag_alloc_refill_probe(nc, 1, pfrag);
+ dfrag_collapsed = va && mptcp_frag_can_collapse_to(msk, pfrag,
+ dfrag);
if (!dfrag_collapsed) {
- if (!mptcp_page_frag_refill(sk, pfrag))
+ va = mptcp_page_frag_alloc_refill_prepare(sk, nc,
+ pfrag);
+ if (!va)
goto wait_for_memory;
dfrag = mptcp_carve_data_frag(msk, pfrag, pfrag->offset);
frag_truesize = dfrag->overhead;
+ va += dfrag->overhead;
}
/* we do not bound vs wspace, to allow a single packet.
* memory accounting will prevent execessive memory usage
* anyway
*/
- offset = dfrag->offset + dfrag->data_len;
- psize = pfrag->size - offset;
+ psize = pfrag->size - frag_truesize;
psize = min_t(size_t, psize, msg_data_left(msg));
psize = min_t(size_t, psize, copy_limit);
total_ts = psize + frag_truesize;
@@ -1869,8 +1880,7 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
if (!sk_wmem_schedule(sk, total_ts))
goto wait_for_memory;
- ret = do_copy_data_nocache(sk, psize, &msg->msg_iter,
- page_address(dfrag->page) + offset);
+ ret = do_copy_data_nocache(sk, psize, &msg->msg_iter, va);
if (ret)
goto do_error;
@@ -1879,7 +1889,6 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
copied += psize;
dfrag->data_len += psize;
frag_truesize += psize;
- pfrag->offset += frag_truesize;
WRITE_ONCE(msk->write_seq, msk->write_seq + psize);
/* charge data on mptcp pending queue to the msk socket
@@ -1887,10 +1896,12 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
*/
sk_wmem_queued_add(sk, frag_truesize);
if (!dfrag_collapsed) {
- get_page(dfrag->page);
+ page_frag_commit(nc, pfrag, frag_truesize);
list_add_tail(&dfrag->list, &msk->rtx_queue);
if (!msk->first_pending)
WRITE_ONCE(msk->first_pending, dfrag);
+ } else {
+ page_frag_commit_noref(nc, pfrag, frag_truesize);
}
pr_debug("msk=%p dfrag at seq=%llu len=%u sent=%u new=%d\n", msk,
dfrag->data_seq, dfrag->data_len, dfrag->already_sent,
diff --git a/net/sched/em_meta.c b/net/sched/em_meta.c
index 8996c73c9779..4da465af972f 100644
--- a/net/sched/em_meta.c
+++ b/net/sched/em_meta.c
@@ -590,7 +590,7 @@ META_COLLECTOR(int_sk_sendmsg_off)
*err = -1;
return;
}
- dst->value = sk->sk_frag.offset;
+ dst->value = page_frag_cache_page_offset(&sk->sk_frag);
}
META_COLLECTOR(int_sk_write_pend)
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index dc063c2c7950..0ea2cee5dffe 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -253,8 +253,8 @@ static void tls_device_resync_tx(struct sock *sk, struct tls_context *tls_ctx,
}
static void tls_append_frag(struct tls_record_info *record,
- struct page_frag *pfrag,
- int size)
+ struct page_frag_cache *nc,
+ struct page_frag *pfrag, int size)
{
skb_frag_t *frag;
@@ -262,15 +262,34 @@ static void tls_append_frag(struct tls_record_info *record,
if (skb_frag_page(frag) == pfrag->page &&
skb_frag_off(frag) + skb_frag_size(frag) == pfrag->offset) {
skb_frag_size_add(frag, size);
+ page_frag_commit_noref(nc, pfrag, size);
} else {
++frag;
skb_frag_fill_page_desc(frag, pfrag->page, pfrag->offset,
size);
++record->num_frags;
+ page_frag_commit(nc, pfrag, size);
+ }
+
+ record->len += size;
+}
+
+static void tls_append_dummy_frag(struct tls_record_info *record,
+ struct page_frag *pfrag, int size)
+{
+ skb_frag_t *frag;
+
+ frag = &record->frags[record->num_frags - 1];
+ if (skb_frag_page(frag) == pfrag->page &&
+ skb_frag_off(frag) + skb_frag_size(frag) == pfrag->offset) {
+ skb_frag_size_add(frag, size);
+ } else {
+ ++frag;
+ skb_frag_fill_page_desc(frag, pfrag->page, pfrag->offset, size);
+ ++record->num_frags;
get_page(pfrag->page);
}
- pfrag->offset += size;
record->len += size;
}
@@ -311,11 +330,11 @@ static int tls_push_record(struct sock *sk,
static void tls_device_record_close(struct sock *sk,
struct tls_context *ctx,
struct tls_record_info *record,
- struct page_frag *pfrag,
+ struct page_frag_cache *nc,
unsigned char record_type)
{
struct tls_prot_info *prot = &ctx->prot_info;
- struct page_frag dummy_tag_frag;
+ struct page_frag dummy_tag_frag, *pfrag;
/* append tag
* device will fill in the tag, we just need to append a placeholder
@@ -323,13 +342,16 @@ static void tls_device_record_close(struct sock *sk,
* increases frag count)
* if we can't allocate memory now use the dummy page
*/
- if (unlikely(pfrag->size - pfrag->offset < prot->tag_size) &&
- !skb_page_frag_refill(prot->tag_size, pfrag, sk->sk_allocation)) {
+ pfrag = &dummy_tag_frag;
+ if (unlikely(!page_frag_refill_probe(nc, prot->tag_size, pfrag) &&
+ !page_frag_refill_prepare(nc, prot->tag_size, pfrag,
+ sk->sk_allocation))) {
dummy_tag_frag.page = dummy_page;
dummy_tag_frag.offset = 0;
- pfrag = &dummy_tag_frag;
+ tls_append_dummy_frag(record, pfrag, prot->tag_size);
+ } else {
+ tls_append_frag(record, nc, pfrag, prot->tag_size);
}
- tls_append_frag(record, pfrag, prot->tag_size);
/* fill prepend */
tls_fill_prepend(ctx, skb_frag_address(&record->frags[0]),
@@ -338,6 +360,7 @@ static void tls_device_record_close(struct sock *sk,
}
static int tls_create_new_record(struct tls_offload_context_tx *offload_ctx,
+ struct page_frag_cache *nc,
struct page_frag *pfrag,
size_t prepend_size)
{
@@ -352,8 +375,7 @@ static int tls_create_new_record(struct tls_offload_context_tx *offload_ctx,
skb_frag_fill_page_desc(frag, pfrag->page, pfrag->offset,
prepend_size);
- get_page(pfrag->page);
- pfrag->offset += prepend_size;
+ page_frag_commit(nc, pfrag, prepend_size);
record->num_frags = 1;
record->len = prepend_size;
@@ -361,33 +383,34 @@ static int tls_create_new_record(struct tls_offload_context_tx *offload_ctx,
return 0;
}
-static int tls_do_allocation(struct sock *sk,
- struct tls_offload_context_tx *offload_ctx,
- struct page_frag *pfrag,
- size_t prepend_size)
+static void *tls_do_allocation(struct sock *sk,
+ struct tls_offload_context_tx *offload_ctx,
+ struct page_frag_cache *nc,
+ size_t prepend_size, struct page_frag *pfrag)
{
int ret;
if (!offload_ctx->open_record) {
- if (unlikely(!skb_page_frag_refill(prepend_size, pfrag,
- sk->sk_allocation))) {
+ void *va;
+
+ if (unlikely(!page_frag_refill_prepare(nc, prepend_size, pfrag,
+ sk->sk_allocation))) {
READ_ONCE(sk->sk_prot)->enter_memory_pressure(sk);
sk_stream_moderate_sndbuf(sk);
- return -ENOMEM;
+ return NULL;
}
- ret = tls_create_new_record(offload_ctx, pfrag, prepend_size);
+ ret = tls_create_new_record(offload_ctx, nc, pfrag,
+ prepend_size);
if (ret)
- return ret;
+ return NULL;
- if (pfrag->size > pfrag->offset)
- return 0;
+ va = page_frag_alloc_refill_probe(nc, 1, pfrag);
+ if (va)
+ return va;
}
- if (!sk_page_frag_refill(sk, pfrag))
- return -ENOMEM;
-
- return 0;
+ return sk_page_frag_alloc_refill_prepare(sk, nc, pfrag);
}
static int tls_device_copy_data(void *addr, size_t bytes, struct iov_iter *i)
@@ -424,8 +447,8 @@ static int tls_push_data(struct sock *sk,
struct tls_prot_info *prot = &tls_ctx->prot_info;
struct tls_offload_context_tx *ctx = tls_offload_ctx_tx(tls_ctx);
struct tls_record_info *record;
+ struct page_frag_cache *nc;
int tls_push_record_flags;
- struct page_frag *pfrag;
size_t orig_size = size;
u32 max_open_record_len;
bool more = false;
@@ -454,7 +477,7 @@ static int tls_push_data(struct sock *sk,
return rc;
}
- pfrag = sk_page_frag(sk);
+ nc = sk_page_frag_cache(sk);
/* TLS_HEADER_SIZE is not counted as part of the TLS record, and
* we need to leave room for an authentication tag.
@@ -462,8 +485,12 @@ static int tls_push_data(struct sock *sk,
max_open_record_len = TLS_MAX_PAYLOAD_SIZE +
prot->prepend_size;
do {
- rc = tls_do_allocation(sk, ctx, pfrag, prot->prepend_size);
- if (unlikely(rc)) {
+ struct page_frag page_frag, *pfrag;
+ void *va;
+
+ pfrag = &page_frag;
+ va = tls_do_allocation(sk, ctx, nc, prot->prepend_size, pfrag);
+ if (unlikely(!va)) {
rc = sk_stream_wait_memory(sk, &timeo);
if (!rc)
continue;
@@ -512,16 +539,15 @@ static int tls_push_data(struct sock *sk,
zc_pfrag.offset = off;
zc_pfrag.size = copy;
- tls_append_frag(record, &zc_pfrag, copy);
+ tls_append_dummy_frag(record, &zc_pfrag, copy);
} else if (copy) {
- copy = min_t(size_t, copy, pfrag->size - pfrag->offset);
+ copy = min_t(size_t, copy, pfrag->size);
- rc = tls_device_copy_data(page_address(pfrag->page) +
- pfrag->offset, copy,
- iter);
+ rc = tls_device_copy_data(va, copy, iter);
if (rc)
goto handle_error;
- tls_append_frag(record, pfrag, copy);
+
+ tls_append_frag(record, nc, pfrag, copy);
}
size -= copy;
@@ -539,7 +565,7 @@ static int tls_push_data(struct sock *sk,
if (done || record->len >= max_open_record_len ||
(record->num_frags >= MAX_SKB_FRAGS - 1)) {
tls_device_record_close(sk, tls_ctx, record,
- pfrag, record_type);
+ nc, record_type);
rc = tls_push_record(sk,
tls_ctx,
--
2.33.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next v20 13/14] mm: page_frag: update documentation for page_frag
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
` (11 preceding siblings ...)
2024-10-08 11:20 ` [PATCH net-next v20 12/14] net: replace page_frag with page_frag_cache Yunsheng Lin
@ 2024-10-08 11:20 ` Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 14/14] mm: page_frag: add an entry in MAINTAINERS " Yunsheng Lin
13 siblings, 0 replies; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni
Cc: netdev, linux-kernel, Yunsheng Lin, Alexander Duyck,
Jonathan Corbet, Andrew Morton, linux-mm, linux-doc
Update documentation about design, implementation and API usages
for page_frag.
CC: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
Documentation/mm/page_frags.rst | 177 +++++++++++++++++++++-
include/linux/page_frag_cache.h | 259 +++++++++++++++++++++++++++++++-
mm/page_frag_cache.c | 26 +++-
3 files changed, 451 insertions(+), 11 deletions(-)
diff --git a/Documentation/mm/page_frags.rst b/Documentation/mm/page_frags.rst
index 503ca6cdb804..5eec04a3fe90 100644
--- a/Documentation/mm/page_frags.rst
+++ b/Documentation/mm/page_frags.rst
@@ -1,3 +1,5 @@
+.. SPDX-License-Identifier: GPL-2.0
+
==============
Page fragments
==============
@@ -40,4 +42,177 @@ page via a single call. The advantage to doing this is that it allows for
cleaning up the multiple references that were added to a page in order to
avoid calling get_page per allocation.
-Alexander Duyck, Nov 29, 2016.
+
+Architecture overview
+=====================
+
+.. code-block:: none
+
+ +----------------------+
+ | page_frag API caller |
+ +----------------------+
+ |
+ |
+ v
+ +------------------------------------------------------------------+
+ | request page fragment |
+ +------------------------------------------------------------------+
+ | | |
+ | | |
+ | Cache not enough |
+ | | |
+ | +-----------------+ |
+ | | reuse old cache |--Usable-->|
+ | +-----------------+ |
+ | | |
+ | Not usable |
+ | | |
+ | v |
+ Cache empty +-----------------+ |
+ | | drain old cache | |
+ | +-----------------+ |
+ | | |
+ v_________________________________v |
+ | |
+ | |
+ _________________v_______________ |
+ | | Cache is enough
+ | | |
+ PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE | |
+ | | |
+ | PAGE_SIZE >= PAGE_FRAG_CACHE_MAX_SIZE |
+ v | |
+ +----------------------------------+ | |
+ | refill cache with order > 0 page | | |
+ +----------------------------------+ | |
+ | | | |
+ | | | |
+ | Refill failed | |
+ | | | |
+ | v v |
+ | +------------------------------------+ |
+ | | refill cache with order 0 page | |
+ | +----------------------------------=-+ |
+ | | |
+ Refill succeed | |
+ | Refill succeed |
+ | | |
+ v v v
+ +------------------------------------------------------------------+
+ | allocate fragment from cache |
+ +------------------------------------------------------------------+
+
+API interface
+=============
+As the design and implementation of page_frag API implies, the allocation side
+does not allow concurrent calling. Instead it is assumed that the caller must
+ensure there is not concurrent alloc calling to the same page_frag_cache
+instance by using its own lock or rely on some lockless guarantee like NAPI
+softirq.
+
+Depending on different aligning requirement, the page_frag API caller may call
+page_frag_*_align*() to ensure the returned virtual address or offset of the
+page is aligned according to the 'align/alignment' parameter. Note the size of
+the allocated fragment is not aligned, the caller needs to provide an aligned
+fragsz if there is an alignment requirement for the size of the fragment.
+
+Depending on different use cases, callers expecting to deal with va, page or
+both va and page for them may call page_frag_alloc, page_frag_refill, or
+page_frag_alloc_refill API accordingly.
+
+There is also a use case that needs minimum memory in order for forward progress,
+but more performant if more memory is available. Using page_frag_*_prepare() and
+page_frag_commit*() related API, the caller requests the minimum memory it needs
+and the prepare API will return the maximum size of the fragment returned. The
+caller needs to either call the commit API to report how much memory it actually
+uses, or not do so if deciding to not use any memory.
+
+.. kernel-doc:: include/linux/page_frag_cache.h
+ :identifiers: page_frag_cache_init page_frag_cache_is_pfmemalloc
+ page_frag_cache_page_offset __page_frag_alloc_align
+ page_frag_alloc_align page_frag_alloc
+ __page_frag_refill_align page_frag_refill_align
+ page_frag_refill __page_frag_refill_prepare_align
+ page_frag_refill_prepare_align page_frag_refill_prepare
+ __page_frag_alloc_refill_prepare_align
+ page_frag_alloc_refill_prepare_align
+ page_frag_alloc_refill_prepare page_frag_alloc_refill_probe
+ page_frag_refill_probe page_frag_commit
+ page_frag_commit_noref page_frag_alloc_abort
+
+.. kernel-doc:: mm/page_frag_cache.c
+ :identifiers: page_frag_cache_drain page_frag_free
+ __page_frag_alloc_refill_probe_align
+
+Coding examples
+===============
+
+Init & Drain API
+----------------
+
+.. code-block:: c
+
+ page_frag_cache_init(nc);
+ ...
+ page_frag_cache_drain(nc);
+
+
+Alloc & Free API
+----------------
+
+.. code-block:: c
+
+ void *va;
+
+ va = page_frag_alloc_align(nc, size, gfp, align);
+ if (!va)
+ goto do_error;
+
+ err = do_something(va, size);
+ if (err) {
+ page_frag_abort(nc, size);
+ goto do_error;
+ }
+
+ ...
+
+ page_frag_free(va);
+
+
+Prepare & Commit API
+--------------------
+
+.. code-block:: c
+
+ struct page_frag page_frag, *pfrag;
+ bool merge = true;
+ void *va;
+
+ pfrag = &page_frag;
+ va = page_frag_alloc_refill_prepare(nc, 32U, pfrag, GFP_KERNEL);
+ if (!va)
+ goto wait_for_space;
+
+ copy = min_t(unsigned int, copy, pfrag->size);
+ if (!skb_can_coalesce(skb, i, pfrag->page, pfrag->offset)) {
+ if (i >= max_skb_frags)
+ goto new_segment;
+
+ merge = false;
+ }
+
+ copy = mem_schedule(copy);
+ if (!copy)
+ goto wait_for_space;
+
+ err = copy_from_iter_full_nocache(va, copy, iter);
+ if (err)
+ goto do_error;
+
+ if (merge) {
+ skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1], copy);
+ page_frag_commit_noref(nc, pfrag, copy);
+ } else {
+ skb_fill_page_desc(skb, i, pfrag->page, pfrag->offset, copy);
+ page_frag_commit(nc, pfrag, copy);
+ }
diff --git a/include/linux/page_frag_cache.h b/include/linux/page_frag_cache.h
index d91ad53f25d3..922d412469c7 100644
--- a/include/linux/page_frag_cache.h
+++ b/include/linux/page_frag_cache.h
@@ -28,16 +28,43 @@ static inline bool page_frag_encoded_page_pfmemalloc(unsigned long encoded_page)
return !!(encoded_page & PAGE_FRAG_CACHE_PFMEMALLOC_BIT);
}
+/**
+ * page_frag_cache_init() - Init page_frag cache.
+ * @nc: page_frag cache from which to init
+ *
+ * Inline helper to init the page_frag cache.
+ */
static inline void page_frag_cache_init(struct page_frag_cache *nc)
{
nc->encoded_page = 0;
}
+/**
+ * page_frag_cache_is_pfmemalloc() - Check for pfmemalloc.
+ * @nc: page_frag cache from which to check
+ *
+ * Used to check if the current page in page_frag cache is pfmemalloc'ed.
+ * It has the same calling context expectation as the alloc API.
+ *
+ * Return:
+ * true if the current page in page_frag cache is pfmemalloc'ed, otherwise
+ * return false.
+ */
static inline bool page_frag_cache_is_pfmemalloc(struct page_frag_cache *nc)
{
return page_frag_encoded_page_pfmemalloc(nc->encoded_page);
}
+/**
+ * page_frag_cache_page_offset() - Return the current page fragment's offset.
+ * @nc: page_frag cache from which to check
+ *
+ * The API is only used in net/sched/em_meta.c for historical reason, do not use
+ * it for new caller unless there is a strong reason.
+ *
+ * Return:
+ * the offset of the current page fragment in the page_frag cache.
+ */
static inline unsigned int page_frag_cache_page_offset(const struct page_frag_cache *nc)
{
return nc->offset;
@@ -66,6 +93,19 @@ static inline unsigned int __page_frag_cache_commit(struct page_frag_cache *nc,
return __page_frag_cache_commit_noref(nc, pfrag, used_sz);
}
+/**
+ * __page_frag_alloc_align() - Alloc a page fragment with aligning
+ * requirement.
+ * @nc: page_frag cache from which to allocate
+ * @fragsz: the requested fragment size
+ * @gfp_mask: the allocation gfp to use when cache need to be refilled
+ * @align_mask: the requested aligning requirement for the 'va'
+ *
+ * Alloc a page fragment from page_frag cache with aligning requirement.
+ *
+ * Return:
+ * Virtual address of the page fragment, otherwise return NULL.
+ */
static inline void *__page_frag_alloc_align(struct page_frag_cache *nc,
unsigned int fragsz, gfp_t gfp_mask,
unsigned int align_mask)
@@ -83,6 +123,19 @@ static inline void *__page_frag_alloc_align(struct page_frag_cache *nc,
return va;
}
+/**
+ * page_frag_alloc_align() - Alloc a page fragment with aligning requirement.
+ * @nc: page_frag cache from which to allocate
+ * @fragsz: the requested fragment size
+ * @gfp_mask: the allocation gfp to use when cache needs to be refilled
+ * @align: the requested aligning requirement for the fragment
+ *
+ * WARN_ON_ONCE() checking for @align before allocing a page fragment from
+ * page_frag cache with aligning requirement.
+ *
+ * Return:
+ * virtual address of the page fragment, otherwise return NULL.
+ */
static inline void *page_frag_alloc_align(struct page_frag_cache *nc,
unsigned int fragsz, gfp_t gfp_mask,
unsigned int align)
@@ -91,12 +144,36 @@ static inline void *page_frag_alloc_align(struct page_frag_cache *nc,
return __page_frag_alloc_align(nc, fragsz, gfp_mask, -align);
}
+/**
+ * page_frag_alloc() - Alloc a page fragment.
+ * @nc: page_frag cache from which to allocate
+ * @fragsz: the requested fragment size
+ * @gfp_mask: the allocation gfp to use when cache need to be refilled
+ *
+ * Alloc a page fragment from page_frag cache.
+ *
+ * Return:
+ * virtual address of the page fragment, otherwise return NULL.
+ */
static inline void *page_frag_alloc(struct page_frag_cache *nc,
unsigned int fragsz, gfp_t gfp_mask)
{
return __page_frag_alloc_align(nc, fragsz, gfp_mask, ~0u);
}
+/**
+ * __page_frag_refill_align() - Refill a page_frag with aligning requirement.
+ * @nc: page_frag cache from which to refill
+ * @fragsz: the requested fragment size
+ * @pfrag: the page_frag to be refilled.
+ * @gfp_mask: the allocation gfp to use when cache need to be refilled
+ * @align_mask: the requested aligning requirement for the fragment
+ *
+ * Refill a page_frag from page_frag cache with aligning requirement.
+ *
+ * Return:
+ * True if refill succeeds, otherwise return false.
+ */
static inline bool __page_frag_refill_align(struct page_frag_cache *nc,
unsigned int fragsz,
struct page_frag *pfrag,
@@ -111,6 +188,20 @@ static inline bool __page_frag_refill_align(struct page_frag_cache *nc,
return true;
}
+/**
+ * page_frag_refill_align() - Refill a page_frag with aligning requirement.
+ * @nc: page_frag cache from which to refill
+ * @fragsz: the requested fragment size
+ * @pfrag: the page_frag to be refilled.
+ * @gfp_mask: the allocation gfp to use when cache needs to be refilled
+ * @align: the requested aligning requirement for the fragment
+ *
+ * WARN_ON_ONCE() checking for @align before refilling a page_frag from
+ * page_frag cache with aligning requirement.
+ *
+ * Return:
+ * True if refill succeeds, otherwise return false.
+ */
static inline bool page_frag_refill_align(struct page_frag_cache *nc,
unsigned int fragsz,
struct page_frag *pfrag,
@@ -120,6 +211,18 @@ static inline bool page_frag_refill_align(struct page_frag_cache *nc,
return __page_frag_refill_align(nc, fragsz, pfrag, gfp_mask, -align);
}
+/**
+ * page_frag_refill() - Refill a page_frag.
+ * @nc: page_frag cache from which to refill
+ * @fragsz: the requested fragment size
+ * @pfrag: the page_frag to be refilled.
+ * @gfp_mask: the allocation gfp to use when cache need to be refilled
+ *
+ * Refill a page_frag from page_frag cache.
+ *
+ * Return:
+ * True if refill succeeds, otherwise return false.
+ */
static inline bool page_frag_refill(struct page_frag_cache *nc,
unsigned int fragsz,
struct page_frag *pfrag, gfp_t gfp_mask)
@@ -127,6 +230,20 @@ static inline bool page_frag_refill(struct page_frag_cache *nc,
return __page_frag_refill_align(nc, fragsz, pfrag, gfp_mask, ~0u);
}
+/**
+ * __page_frag_refill_prepare_align() - Prepare refilling a page_frag with
+ * aligning requirement.
+ * @nc: page_frag cache from which to refill
+ * @fragsz: the requested fragment size
+ * @pfrag: the page_frag to be refilled.
+ * @gfp_mask: the allocation gfp to use when cache need to be refilled
+ * @align_mask: the requested aligning requirement for the fragment
+ *
+ * Prepare refill a page_frag from page_frag cache with aligning requirement.
+ *
+ * Return:
+ * True if prepare refilling succeeds, otherwise return false.
+ */
static inline bool __page_frag_refill_prepare_align(struct page_frag_cache *nc,
unsigned int fragsz,
struct page_frag *pfrag,
@@ -137,6 +254,21 @@ static inline bool __page_frag_refill_prepare_align(struct page_frag_cache *nc,
align_mask);
}
+/**
+ * page_frag_refill_prepare_align() - Prepare refilling a page_frag with
+ * aligning requirement.
+ * @nc: page_frag cache from which to refill
+ * @fragsz: the requested fragment size
+ * @pfrag: the page_frag to be refilled.
+ * @gfp_mask: the allocation gfp to use when cache needs to be refilled
+ * @align: the requested aligning requirement for the fragment
+ *
+ * WARN_ON_ONCE() checking for @align before prepare refilling a page_frag from
+ * page_frag cache with aligning requirement.
+ *
+ * Return:
+ * True if prepare refilling succeeds, otherwise return false.
+ */
static inline bool page_frag_refill_prepare_align(struct page_frag_cache *nc,
unsigned int fragsz,
struct page_frag *pfrag,
@@ -148,6 +280,18 @@ static inline bool page_frag_refill_prepare_align(struct page_frag_cache *nc,
-align);
}
+/**
+ * page_frag_refill_prepare() - Prepare refilling a page_frag.
+ * @nc: page_frag cache from which to refill
+ * @fragsz: the requested fragment size
+ * @pfrag: the page_frag to be refilled.
+ * @gfp_mask: the allocation gfp to use when cache need to be refilled
+ *
+ * Prepare refilling a page_frag from page_frag cache.
+ *
+ * Return:
+ * True if refill succeeds, otherwise return false.
+ */
static inline bool page_frag_refill_prepare(struct page_frag_cache *nc,
unsigned int fragsz,
struct page_frag *pfrag,
@@ -157,6 +301,20 @@ static inline bool page_frag_refill_prepare(struct page_frag_cache *nc,
~0u);
}
+/**
+ * __page_frag_alloc_refill_prepare_align() - Prepare allocing a fragment and
+ * refilling a page_frag with aligning requirement.
+ * @nc: page_frag cache from which to allocate and refill
+ * @fragsz: the requested fragment size
+ * @pfrag: the page_frag to be refilled.
+ * @gfp_mask: the allocation gfp to use when cache need to be refilled
+ * @align_mask: the requested aligning requirement for the fragment.
+ *
+ * Prepare allocing a fragment and refilling a page_frag from page_frag cache.
+ *
+ * Return:
+ * virtual address of the page fragment, otherwise return NULL.
+ */
static inline void *__page_frag_alloc_refill_prepare_align(struct page_frag_cache *nc,
unsigned int fragsz,
struct page_frag *pfrag,
@@ -166,6 +324,21 @@ static inline void *__page_frag_alloc_refill_prepare_align(struct page_frag_cach
return __page_frag_cache_prepare(nc, fragsz, pfrag, gfp_mask, align_mask);
}
+/**
+ * page_frag_alloc_refill_prepare_align() - Prepare allocing a fragment and
+ * refilling a page_frag with aligning requirement.
+ * @nc: page_frag cache from which to allocate and refill
+ * @fragsz: the requested fragment size
+ * @pfrag: the page_frag to be refilled.
+ * @gfp_mask: the allocation gfp to use when cache need to be refilled
+ * @align: the requested aligning requirement for the fragment.
+ *
+ * WARN_ON_ONCE() checking for @align before prepare allocing a fragment and
+ * refilling a page_frag from page_frag cache.
+ *
+ * Return:
+ * virtual address of the page fragment, otherwise return NULL.
+ */
static inline void *page_frag_alloc_refill_prepare_align(struct page_frag_cache *nc,
unsigned int fragsz,
struct page_frag *pfrag,
@@ -177,6 +350,19 @@ static inline void *page_frag_alloc_refill_prepare_align(struct page_frag_cache
gfp_mask, -align);
}
+/**
+ * page_frag_alloc_refill_prepare() - Prepare allocing a fragment and refilling
+ * a page_frag.
+ * @nc: page_frag cache from which to allocate and refill
+ * @fragsz: the requested fragment size
+ * @pfrag: the page_frag to be refilled.
+ * @gfp_mask: the allocation gfp to use when cache need to be refilled
+ *
+ * Prepare allocing a fragment and refilling a page_frag from page_frag cache.
+ *
+ * Return:
+ * virtual address of the page fragment, otherwise return NULL.
+ */
static inline void *page_frag_alloc_refill_prepare(struct page_frag_cache *nc,
unsigned int fragsz,
struct page_frag *pfrag,
@@ -186,6 +372,18 @@ static inline void *page_frag_alloc_refill_prepare(struct page_frag_cache *nc,
gfp_mask, ~0u);
}
+/**
+ * page_frag_alloc_refill_probe() - Probe allocing a fragment and refilling
+ * a page_frag.
+ * @nc: page_frag cache from which to allocate and refill
+ * @fragsz: the requested fragment size
+ * @pfrag: the page_frag to be refilled
+ *
+ * Probe allocing a fragment and refilling a page_frag from page_frag cache.
+ *
+ * Return:
+ * virtual address of the page fragment, otherwise return NULL.
+ */
static inline void *page_frag_alloc_refill_probe(struct page_frag_cache *nc,
unsigned int fragsz,
struct page_frag *pfrag)
@@ -193,6 +391,17 @@ static inline void *page_frag_alloc_refill_probe(struct page_frag_cache *nc,
return __page_frag_alloc_refill_probe_align(nc, fragsz, pfrag, ~0u);
}
+/**
+ * page_frag_refill_probe() - Probe refilling a page_frag.
+ * @nc: page_frag cache from which to refill
+ * @fragsz: the requested fragment size
+ * @pfrag: the page_frag to be refilled
+ *
+ * Probe refilling a page_frag from page_frag cache.
+ *
+ * Return:
+ * True if refill succeeds, otherwise return false.
+ */
static inline bool page_frag_refill_probe(struct page_frag_cache *nc,
unsigned int fragsz,
struct page_frag *pfrag)
@@ -200,20 +409,54 @@ static inline bool page_frag_refill_probe(struct page_frag_cache *nc,
return !!page_frag_alloc_refill_probe(nc, fragsz, pfrag);
}
-static inline void page_frag_commit(struct page_frag_cache *nc,
- struct page_frag *pfrag,
- unsigned int used_sz)
+/**
+ * page_frag_commit - Commit allocing a page fragment.
+ * @nc: page_frag cache from which to commit
+ * @pfrag: the page_frag to be committed
+ * @used_sz: size of the page fragment has been used
+ *
+ * Commit the actual used size for the allocation that was either prepared
+ * or probed.
+ *
+ * Return:
+ * The true size of the fragment considering the offset alignment.
+ */
+static inline unsigned int page_frag_commit(struct page_frag_cache *nc,
+ struct page_frag *pfrag,
+ unsigned int used_sz)
{
- __page_frag_cache_commit(nc, pfrag, used_sz);
+ return __page_frag_cache_commit(nc, pfrag, used_sz);
}
-static inline void page_frag_commit_noref(struct page_frag_cache *nc,
- struct page_frag *pfrag,
- unsigned int used_sz)
+/**
+ * page_frag_commit_noref - Commit allocing a page fragment without taking
+ * page refcount.
+ * @nc: page_frag cache from which to commit
+ * @pfrag: the page_frag to be committed
+ * @used_sz: size of the page fragment has been used
+ *
+ * Commit the alloc preparing or probing by passing the actual used size, but
+ * not taking refcount. Mostly used for fragmemt coalescing case when the
+ * current fragment can share the same refcount with previous fragment.
+ *
+ * Return:
+ * The true size of the fragment considering the offset alignment.
+ */
+static inline unsigned int page_frag_commit_noref(struct page_frag_cache *nc,
+ struct page_frag *pfrag,
+ unsigned int used_sz)
{
- __page_frag_cache_commit_noref(nc, pfrag, used_sz);
+ return __page_frag_cache_commit_noref(nc, pfrag, used_sz);
}
+/**
+ * page_frag_alloc_abort - Abort the page fragment allocation.
+ * @nc: page_frag cache to which the page fragment is aborted back
+ * @fragsz: size of the page fragment to be aborted
+ *
+ * It is expected to be called from the same context as the alloc API.
+ * Mostly used for error handling cases where the fragment is no longer needed.
+ */
static inline void page_frag_alloc_abort(struct page_frag_cache *nc,
unsigned int fragsz)
{
diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
index 1e7757a433d0..7b801856fd98 100644
--- a/mm/page_frag_cache.c
+++ b/mm/page_frag_cache.c
@@ -75,6 +75,10 @@ static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
return page;
}
+/**
+ * page_frag_cache_drain - Drain the current page from page_frag cache.
+ * @nc: page_frag cache from which to drain
+ */
void page_frag_cache_drain(struct page_frag_cache *nc)
{
if (!nc->encoded_page)
@@ -117,6 +121,20 @@ unsigned int __page_frag_cache_commit_noref(struct page_frag_cache *nc,
}
EXPORT_SYMBOL(__page_frag_cache_commit_noref);
+/**
+ * __page_frag_alloc_refill_probe_align() - Probe allocing a fragment and
+ * refilling a page_frag with aligning requirement.
+ * @nc: page_frag cache from which to allocate and refill
+ * @fragsz: the requested fragment size
+ * @pfrag: the page_frag to be refilled.
+ * @align_mask: the requested aligning requirement for the fragment.
+ *
+ * Probe allocing a fragment and refilling a page_frag from page_frag cache with
+ * aligning requirement.
+ *
+ * Return:
+ * virtual address of the page fragment, otherwise return NULL.
+ */
void *__page_frag_alloc_refill_probe_align(struct page_frag_cache *nc,
unsigned int fragsz,
struct page_frag *pfrag,
@@ -208,8 +226,12 @@ void *__page_frag_cache_prepare(struct page_frag_cache *nc, unsigned int fragsz,
}
EXPORT_SYMBOL(__page_frag_cache_prepare);
-/*
- * Frees a page fragment allocated out of either a compound or order 0 page.
+/**
+ * page_frag_free - Free a page fragment.
+ * @addr: va of page fragment to be freed
+ *
+ * Free a page fragment allocated out of either a compound or order 0 page by
+ * virtual address.
*/
void page_frag_free(void *addr)
{
--
2.33.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH net-next v20 14/14] mm: page_frag: add an entry in MAINTAINERS for page_frag
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
` (12 preceding siblings ...)
2024-10-08 11:20 ` [PATCH net-next v20 13/14] mm: page_frag: update documentation for page_frag Yunsheng Lin
@ 2024-10-08 11:20 ` Yunsheng Lin
2024-10-09 0:43 ` Jakub Kicinski
13 siblings, 1 reply; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-08 11:20 UTC (permalink / raw)
To: davem, kuba, pabeni; +Cc: netdev, linux-kernel, Yunsheng Lin, Alexander Duyck
After this patchset, page_frag is a small subsystem/library
on its own, so add an entry in MAINTAINERS to indicate the
new subsystem/library's maintainer, maillist, status and file
lists of page_frag.
Alexander is the original author of page_frag, add him in the
MAINTAINERS too.
CC: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
MAINTAINERS | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index af635dc60cfe..05b45231454d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17496,6 +17496,18 @@ F: mm/page-writeback.c
F: mm/readahead.c
F: mm/truncate.c
+PAGE FRAG
+M: Alexander Duyck <alexander.duyck@gmail.com>
+M: Yunsheng Lin <linyunsheng@huawei.com>
+L: linux-mm@kvack.org
+L: netdev@vger.kernel.org
+S: Supported
+F: Documentation/mm/page_frags.rst
+F: include/linux/page_frag_cache.h
+F: mm/page_frag_cache.c
+F: tools/testing/selftests/mm/page_frag/
+F: tools/testing/selftests/mm/test_page_frag.sh
+
PAGE POOL
M: Jesper Dangaard Brouer <hawk@kernel.org>
M: Ilias Apalodimas <ilias.apalodimas@linaro.org>
--
2.33.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 01/14] mm: page_frag: add a test module for page_frag
2024-10-08 11:20 ` [PATCH net-next v20 01/14] mm: page_frag: add a test module for page_frag Yunsheng Lin
@ 2024-10-08 19:56 ` Shuah Khan
2024-10-09 3:59 ` Yunsheng Lin
0 siblings, 1 reply; 30+ messages in thread
From: Shuah Khan @ 2024-10-08 19:56 UTC (permalink / raw)
To: Yunsheng Lin, davem, kuba, pabeni
Cc: netdev, linux-kernel, Alexander Duyck, Alexander Duyck,
Andrew Morton, Shuah Khan, linux-mm, linux-kselftest, Shuah Khan
On 10/8/24 05:20, Yunsheng Lin wrote:
> The testing is done by ensuring that the fragment allocated
> from a frag_frag_cache instance is pushed into a ptr_ring
> instance in a kthread binded to a specified cpu, and a kthread
> binded to a specified cpu will pop the fragment from the
> ptr_ring and free the fragment.
>
> CC: Alexander Duyck <alexander.duyck@gmail.com>
> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
> Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
Signed-off-by should be last. Same comment on all the other
patches in this series. When you have 4 patches, it is a good
practice to add cover-letter.
> ---
> tools/testing/selftests/mm/Makefile | 3 +
> tools/testing/selftests/mm/page_frag/Makefile | 18 ++
> .../selftests/mm/page_frag/page_frag_test.c | 173 ++++++++++++++++++
> tools/testing/selftests/mm/run_vmtests.sh | 8 +
> tools/testing/selftests/mm/test_page_frag.sh | 171 +++++++++++++++++
> 5 files changed, 373 insertions(+)
> create mode 100644 tools/testing/selftests/mm/page_frag/Makefile
> create mode 100644 tools/testing/selftests/mm/page_frag/page_frag_test.c
> create mode 100755 tools/testing/selftests/mm/test_page_frag.sh
>
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index 02e1204971b0..acec529baaca 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -36,6 +36,8 @@ MAKEFLAGS += --no-builtin-rules
> CFLAGS = -Wall -I $(top_srcdir) $(EXTRA_CFLAGS) $(KHDR_INCLUDES) $(TOOLS_INCLUDES)
> LDLIBS = -lrt -lpthread -lm
>
> +TEST_GEN_MODS_DIR := page_frag
> +
> TEST_GEN_FILES = cow
> TEST_GEN_FILES += compaction_test
> TEST_GEN_FILES += gup_longterm
> @@ -126,6 +128,7 @@ TEST_FILES += test_hmm.sh
> TEST_FILES += va_high_addr_switch.sh
> TEST_FILES += charge_reserved_hugetlb.sh
> TEST_FILES += hugetlb_reparenting_test.sh
> +TEST_FILES += test_page_frag.sh
>
> # required by charge_reserved_hugetlb.sh
> TEST_FILES += write_hugetlb_memory.sh
> diff --git a/tools/testing/selftests/mm/page_frag/Makefile b/tools/testing/selftests/mm/page_frag/Makefile
> new file mode 100644
> index 000000000000..58dda74d50a3
> --- /dev/null
> +++ b/tools/testing/selftests/mm/page_frag/Makefile
> @@ -0,0 +1,18 @@
> +PAGE_FRAG_TEST_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
> +KDIR ?= $(abspath $(PAGE_FRAG_TEST_DIR)/../../../../..)
> +
> +ifeq ($(V),1)
> +Q =
> +else
> +Q = @
> +endif
> +
> +MODULES = page_frag_test.ko
> +
> +obj-m += page_frag_test.o
> +
> +all:
> + +$(Q)make -C $(KDIR) M=$(PAGE_FRAG_TEST_DIR) modules
> +
> +clean:
> + +$(Q)make -C $(KDIR) M=$(PAGE_FRAG_TEST_DIR) clean
> diff --git a/tools/testing/selftests/mm/page_frag/page_frag_test.c b/tools/testing/selftests/mm/page_frag/page_frag_test.c
> new file mode 100644
> index 000000000000..eeb2b6bc681a
> --- /dev/null
> +++ b/tools/testing/selftests/mm/page_frag/page_frag_test.c
> @@ -0,0 +1,173 @@
> +// SPDX-License-Identifier: GPL-2.0
I think this would throw a checkpatch warning about
comment should be "/*" and not "//"
> +
> +/*
> + * Test module for page_frag cache
> + *
> + * Copyright (C) 2024 Yunsheng Lin <linyunsheng@huawei.com>
> + */
> +
> +#include <linux/mm.h>
> +#include <linux/module.h>
> +#include <linux/cpumask.h>
> +#include <linux/completion.h>
> +#include <linux/ptr_ring.h>
> +#include <linux/kthread.h>
> +
> +static struct ptr_ring ptr_ring;
> +static int nr_objs = 512;
> +static atomic_t nthreads;
> +static struct completion wait;
> +static struct page_frag_cache test_nc;
> +static int test_popped;
> +static int test_pushed;
> +
> +static int nr_test = 2000000;
> +module_param(nr_test, int, 0);
> +MODULE_PARM_DESC(nr_test, "number of iterations to test");
> +
> +static bool test_align;
> +module_param(test_align, bool, 0);
> +MODULE_PARM_DESC(test_align, "use align API for testing");
> +
> +static int test_alloc_len = 2048;
> +module_param(test_alloc_len, int, 0);
> +MODULE_PARM_DESC(test_alloc_len, "alloc len for testing");
> +
> +static int test_push_cpu;
> +module_param(test_push_cpu, int, 0);
> +MODULE_PARM_DESC(test_push_cpu, "test cpu for pushing fragment");
> +
> +static int test_pop_cpu;
> +module_param(test_pop_cpu, int, 0);
> +MODULE_PARM_DESC(test_pop_cpu, "test cpu for popping fragment");
> +
> +static int page_frag_pop_thread(void *arg)
> +{
> + struct ptr_ring *ring = arg;
> +
> + pr_info("page_frag pop test thread begins on cpu %d\n",
> + smp_processor_id());
> +
> + while (test_popped < nr_test) {
> + void *obj = __ptr_ring_consume(ring);
> +
> + if (obj) {
> + test_popped++;
> + page_frag_free(obj);
> + } else {
> + cond_resched();
> + }
> + }
> +
> + if (atomic_dec_and_test(&nthreads))
> + complete(&wait);
> +
> + pr_info("page_frag pop test thread exits on cpu %d\n",
> + smp_processor_id());
> +
> + return 0;
> +}
> +
> +static int page_frag_push_thread(void *arg)
> +{
> + struct ptr_ring *ring = arg;
> +
> + pr_info("page_frag push test thread begins on cpu %d\n",
> + smp_processor_id());
> +
> + while (test_pushed < nr_test) {
> + void *va;
> + int ret;
> +
> + if (test_align) {
> + va = page_frag_alloc_align(&test_nc, test_alloc_len,
> + GFP_KERNEL, SMP_CACHE_BYTES);
> +
> + WARN_ONCE((unsigned long)va & (SMP_CACHE_BYTES - 1),
> + "unaligned va returned\n");
> + } else {
> + va = page_frag_alloc(&test_nc, test_alloc_len, GFP_KERNEL);
> + }
> +
> + if (!va)
> + continue;
> +
> + ret = __ptr_ring_produce(ring, va);
> + if (ret) {
> + page_frag_free(va);
> + cond_resched();
> + } else {
> + test_pushed++;
> + }
> + }
> +
> + pr_info("page_frag push test thread exits on cpu %d\n",
> + smp_processor_id());
> +
> + if (atomic_dec_and_test(&nthreads))
> + complete(&wait);
> +
> + return 0;
> +}
> +
> +static int __init page_frag_test_init(void)
> +{
> + struct task_struct *tsk_push, *tsk_pop;
> + ktime_t start;
> + u64 duration;
> + int ret;
> +
> + test_nc.va = NULL;
> + atomic_set(&nthreads, 2);
> + init_completion(&wait);
> +
> + if (test_alloc_len > PAGE_SIZE || test_alloc_len <= 0 ||
> + !cpu_active(test_push_cpu) || !cpu_active(test_pop_cpu))
> + return -EINVAL;
> +
> + ret = ptr_ring_init(&ptr_ring, nr_objs, GFP_KERNEL);
> + if (ret)
> + return ret;
> +
> + tsk_push = kthread_create_on_cpu(page_frag_push_thread, &ptr_ring,
> + test_push_cpu, "page_frag_push");
> + if (IS_ERR(tsk_push))
> + return PTR_ERR(tsk_push);
> +
> + tsk_pop = kthread_create_on_cpu(page_frag_pop_thread, &ptr_ring,
> + test_pop_cpu, "page_frag_pop");
> + if (IS_ERR(tsk_pop)) {
> + kthread_stop(tsk_push);
> + return PTR_ERR(tsk_pop);
> + }
> +
> + start = ktime_get();
> + wake_up_process(tsk_push);
> + wake_up_process(tsk_pop);
> +
> + pr_info("waiting for test to complete\n");
> +
> + while (!wait_for_completion_timeout(&wait, msecs_to_jiffies(10000)))
> + pr_info("page_frag_test progress: pushed = %d, popped = %d\n",
> + test_pushed, test_popped);
> +
> + duration = (u64)ktime_us_delta(ktime_get(), start);
> + pr_info("%d of iterations for %s testing took: %lluus\n", nr_test,
> + test_align ? "aligned" : "non-aligned", duration);
> +
> + ptr_ring_cleanup(&ptr_ring, NULL);
> + page_frag_cache_drain(&test_nc);
> +
> + return -EAGAIN;
> +}
> +
> +static void __exit page_frag_test_exit(void)
> +{
> +}
> +
> +module_init(page_frag_test_init);
> +module_exit(page_frag_test_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Yunsheng Lin <linyunsheng@huawei.com>");
> +MODULE_DESCRIPTION("Test module for page_frag");
> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
> index c5797ad1d37b..2c5394584af4 100755
> --- a/tools/testing/selftests/mm/run_vmtests.sh
> +++ b/tools/testing/selftests/mm/run_vmtests.sh
> @@ -75,6 +75,8 @@ separated by spaces:
> read-only VMAs
> - mdwe
> test prctl(PR_SET_MDWE, ...)
> +- page_frag
> + test handling of page fragment allocation and freeing
>
> example: ./run_vmtests.sh -t "hmm mmap ksm"
> EOF
> @@ -456,6 +458,12 @@ CATEGORY="mkdirty" run_test ./mkdirty
>
> CATEGORY="mdwe" run_test ./mdwe_test
>
> +CATEGORY="page_frag" run_test ./test_page_frag.sh smoke
> +
> +CATEGORY="page_frag" run_test ./test_page_frag.sh aligned
> +
> +CATEGORY="page_frag" run_test ./test_page_frag.sh nonaligned
> +
> echo "SUMMARY: PASS=${count_pass} SKIP=${count_skip} FAIL=${count_fail}" | tap_prefix
> echo "1..${count_total}" | tap_output
>
> diff --git a/tools/testing/selftests/mm/test_page_frag.sh b/tools/testing/selftests/mm/test_page_frag.sh
> new file mode 100755
> index 000000000000..d750d910c899
> --- /dev/null
> +++ b/tools/testing/selftests/mm/test_page_frag.sh
> @@ -0,0 +1,171 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Copyright (C) 2024 Yunsheng Lin <linyunsheng@huawei.com>
> +# Copyright (C) 2018 Uladzislau Rezki (Sony) <urezki@gmail.com>
> +#
> +# This is a test script for the kernel test driver to test the
> +# correctness and performance of page_frag's implementation.
> +# Therefore it is just a kernel module loader. You can specify
> +# and pass different parameters in order to:
> +# a) analyse performance of page fragment allocations;
> +# b) stressing and stability check of page_frag subsystem.
> +
> +DRIVER="./page_frag/page_frag_test.ko"
> +CPU_LIST=$(grep -m 2 processor /proc/cpuinfo | cut -d ' ' -f 2)
> +TEST_CPU_0=$(echo $CPU_LIST | awk '{print $1}')
> +
> +if [ $(echo $CPU_LIST | wc -w) -gt 1 ]; then
> + TEST_CPU_1=$(echo $CPU_LIST | awk '{print $2}')
> + NR_TEST=100000000
> +else
> + TEST_CPU_1=$TEST_CPU_0
> + NR_TEST=1000000
> +fi
> +
> +# 1 if fails
> +exitcode=1
> +
> +# Kselftest framework requirement - SKIP code is 4.
> +ksft_skip=4
> +
> +#
> +# Static templates for testing of page_frag APIs.
> +# Also it is possible to pass any supported parameters manually.
> +#
> +SMOKE_PARAM="test_push_cpu=$TEST_CPU_0 test_pop_cpu=$TEST_CPU_1"
> +NONALIGNED_PARAM="$SMOKE_PARAM test_alloc_len=75 nr_test=$NR_TEST"
> +ALIGNED_PARAM="$NONALIGNED_PARAM test_align=1"
> +
> +check_test_requirements()
> +{
> + uid=$(id -u)
> + if [ $uid -ne 0 ]; then
> + echo "$0: Must be run as root"
> + exit $ksft_skip
> + fi
> +
> + if ! which insmod > /dev/null 2>&1; then
> + echo "$0: You need insmod installed"
> + exit $ksft_skip
> + fi
> +
> + if [ ! -f $DRIVER ]; then
> + echo "$0: You need to compile page_frag_test module"
> + exit $ksft_skip
> + fi
> +}
> +
> +run_nonaligned_check()
> +{
> + echo "Run performance tests to evaluate how fast nonaligned alloc API is."
> +
> + insmod $DRIVER $NONALIGNED_PARAM > /dev/null 2>&1
> + echo "Done."
> + echo "Check the kernel ring buffer to see the summary."
> +}
> +
> +run_aligned_check()
> +{
> + echo "Run performance tests to evaluate how fast aligned alloc API is."
> +
> + insmod $DRIVER $ALIGNED_PARAM > /dev/null 2>&1
> + echo "Done."
> + echo "Check the kernel ring buffer to see the summary."
> +}
> +
> +run_smoke_check()
> +{
> + echo "Run smoke test."
> +
> + insmod $DRIVER $SMOKE_PARAM > /dev/null 2>&1
> + echo "Done."
> + echo "Check the kernel ring buffer to see the summary."
> +}
> +
> +usage()
> +{
> + echo -n "Usage: $0 [ aligned ] | [ nonaligned ] | | [ smoke ] | "
> + echo "manual parameters"
> + echo
> + echo "Valid tests and parameters:"
> + echo
> + modinfo $DRIVER
> + echo
> + echo "Example usage:"
> + echo
> + echo "# Shows help message"
> + echo "$0"
> + echo
> + echo "# Smoke testing"
> + echo "$0 smoke"
> + echo
> + echo "# Performance testing for nonaligned alloc API"
> + echo "$0 nonaligned"
> + echo
> + echo "# Performance testing for aligned alloc API"
> + echo "$0 aligned"
> + echo
> + exit 0
> +}
> +
> +function validate_passed_args()
> +{
> + VALID_ARGS=`modinfo $DRIVER | awk '/parm:/ {print $2}' | sed 's/:.*//'`
> +
> + #
> + # Something has been passed, check it.
> + #
> + for passed_arg in $@; do
> + key=${passed_arg//=*/}
> + valid=0
> +
> + for valid_arg in $VALID_ARGS; do
> + if [[ $key = $valid_arg ]]; then
> + valid=1
> + break
> + fi
> + done
> +
> + if [[ $valid -ne 1 ]]; then
> + echo "Error: key is not correct: ${key}"
> + exit $exitcode
> + fi
> + done
> +}
> +
> +function run_manual_check()
> +{
> + #
> + # Validate passed parameters. If there is wrong one,
> + # the script exists and does not execute further.
> + #
> + validate_passed_args $@
> +
> + echo "Run the test with following parameters: $@"
Is this marker good enough to isolate the test results in the
dmesg? Include the test name in the message.
> + insmod $DRIVER $@ > /dev/null 2>&1
> + echo "Done."
Is this marker good enough to isolate the test results in the
dmesg? Include the test name in the message.
> + echo "Check the kernel ring buffer to see the summary."
Usually the test would run dmesg and filter out the test results
from the dmesg and include them in the test script output.
You can refer to other tests that do that: powerpc/scripts/hmi.sh
is one example.
> +}
> +
> +function run_test()
> +{
> + if [ $# -eq 0 ]; then
> + usage
> + else
> + if [[ "$1" = "smoke" ]]; then
> + run_smoke_check
> + elif [[ "$1" = "nonaligned" ]]; then
> + run_nonaligned_check
> + elif [[ "$1" = "aligned" ]]; then
> + run_aligned_check
> + else
> + run_manual_check $@
> + fi
> + fi
> +}
> +
> +check_test_requirements
> +run_test $@
> +
> +exit 0
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 14/14] mm: page_frag: add an entry in MAINTAINERS for page_frag
2024-10-08 11:20 ` [PATCH net-next v20 14/14] mm: page_frag: add an entry in MAINTAINERS " Yunsheng Lin
@ 2024-10-09 0:43 ` Jakub Kicinski
2024-10-09 4:01 ` Yunsheng Lin
0 siblings, 1 reply; 30+ messages in thread
From: Jakub Kicinski @ 2024-10-09 0:43 UTC (permalink / raw)
To: Yunsheng Lin; +Cc: davem, pabeni, netdev, linux-kernel, Alexander Duyck
On Tue, 8 Oct 2024 19:20:48 +0800 Yunsheng Lin wrote:
> +M: Yunsheng Lin <linyunsheng@huawei.com>
The bar for maintaining core code is very high, if you'd
like to be a maintainer please start small.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 01/14] mm: page_frag: add a test module for page_frag
2024-10-08 19:56 ` Shuah Khan
@ 2024-10-09 3:59 ` Yunsheng Lin
2024-10-10 21:18 ` Shuah Khan
0 siblings, 1 reply; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-09 3:59 UTC (permalink / raw)
To: Shuah Khan, davem, kuba, pabeni
Cc: netdev, linux-kernel, Alexander Duyck, Alexander Duyck,
Andrew Morton, Shuah Khan, linux-mm, linux-kselftest
On 2024/10/9 3:56, Shuah Khan wrote:
> On 10/8/24 05:20, Yunsheng Lin wrote:
>> The testing is done by ensuring that the fragment allocated
>> from a frag_frag_cache instance is pushed into a ptr_ring
>> instance in a kthread binded to a specified cpu, and a kthread
>> binded to a specified cpu will pop the fragment from the
>> ptr_ring and free the fragment.
>>
>> CC: Alexander Duyck <alexander.duyck@gmail.com>
>> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
>> Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
>
> Signed-off-by should be last. Same comment on all the other
Hi, Shuah
I used 'git am' to collect those tag, it seems that is the order
the tool applied, and I checking other applied commit, it seems
only Signed-off-by from the committer is the last, like the below
recent mm commit:
6901cf55de22
ff7f5ad7bce4
> patches in this series. When you have 4 patches, it is a good
> practice to add cover-letter.
I guess the cover-letter meant below?
https://lore.kernel.org/all/20241008112049.2279307-1-linyunsheng@huawei.com/
>
>> ---
>> tools/testing/selftests/mm/Makefile | 3 +
>> tools/testing/selftests/mm/page_frag/Makefile | 18 ++
>> .../selftests/mm/page_frag/page_frag_test.c | 173 ++++++++++++++++++
>> tools/testing/selftests/mm/run_vmtests.sh | 8 +
>> tools/testing/selftests/mm/test_page_frag.sh | 171 +++++++++++++++++
>> 5 files changed, 373 insertions(+)
>> create mode 100644 tools/testing/selftests/mm/page_frag/Makefile
>> create mode 100644 tools/testing/selftests/mm/page_frag/page_frag_test.c
>> create mode 100755 tools/testing/selftests/mm/test_page_frag.sh
>>
>> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
>> index 02e1204971b0..acec529baaca 100644
>> --- a/tools/testing/selftests/mm/Makefile
>> +++ b/tools/testing/selftests/mm/Makefile
>> @@ -36,6 +36,8 @@ MAKEFLAGS += --no-builtin-rules
>> CFLAGS = -Wall -I $(top_srcdir) $(EXTRA_CFLAGS) $(KHDR_INCLUDES) $(TOOLS_INCLUDES)
>> LDLIBS = -lrt -lpthread -lm
>> +TEST_GEN_MODS_DIR := page_frag
>> +
>> TEST_GEN_FILES = cow
>> TEST_GEN_FILES += compaction_test
>> TEST_GEN_FILES += gup_longterm
>> @@ -126,6 +128,7 @@ TEST_FILES += test_hmm.sh
>> TEST_FILES += va_high_addr_switch.sh
>> TEST_FILES += charge_reserved_hugetlb.sh
>> TEST_FILES += hugetlb_reparenting_test.sh
>> +TEST_FILES += test_page_frag.sh
>> # required by charge_reserved_hugetlb.sh
>> TEST_FILES += write_hugetlb_memory.sh
>> diff --git a/tools/testing/selftests/mm/page_frag/Makefile b/tools/testing/selftests/mm/page_frag/Makefile
>> new file mode 100644
>> index 000000000000..58dda74d50a3
>> --- /dev/null
>> +++ b/tools/testing/selftests/mm/page_frag/Makefile
>> @@ -0,0 +1,18 @@
>> +PAGE_FRAG_TEST_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
>> +KDIR ?= $(abspath $(PAGE_FRAG_TEST_DIR)/../../../../..)
>> +
>> +ifeq ($(V),1)
>> +Q =
>> +else
>> +Q = @
>> +endif
>> +
>> +MODULES = page_frag_test.ko
>> +
>> +obj-m += page_frag_test.o
>> +
>> +all:
>> + +$(Q)make -C $(KDIR) M=$(PAGE_FRAG_TEST_DIR) modules
>> +
>> +clean:
>> + +$(Q)make -C $(KDIR) M=$(PAGE_FRAG_TEST_DIR) clean
>> diff --git a/tools/testing/selftests/mm/page_frag/page_frag_test.c b/tools/testing/selftests/mm/page_frag/page_frag_test.c
>> new file mode 100644
>> index 000000000000..eeb2b6bc681a
>> --- /dev/null
>> +++ b/tools/testing/selftests/mm/page_frag/page_frag_test.c
>> @@ -0,0 +1,173 @@
>> +// SPDX-License-Identifier: GPL-2.0
>
> I think this would throw a checkpatch warning about
> comment should be "/*" and not "//"
using "git grep 'SPDX-License' mm", "//" seems like a more common
case.
And I did using './scripts/checkpatch.pl --strict --codespell', and
it does not throw a checkpatch warning.
>> +
>> +/*
>> + * Test module for page_frag cache
>> + *
...
>> +function run_manual_check()
>> +{
>> + #
>> + # Validate passed parameters. If there is wrong one,
>> + # the script exists and does not execute further.
>> + #
>> + validate_passed_args $@
>> +
>> + echo "Run the test with following parameters: $@"
>
> Is this marker good enough to isolate the test results in the
> dmesg? Include the test name in the message.
>
>
>> + insmod $DRIVER $@ > /dev/null 2>&1
>> + echo "Done."
>
> Is this marker good enough to isolate the test results in the
> dmesg? Include the test name in the message.
>
>> + echo "Check the kernel ring buffer to see the summary."
>
> Usually the test would run dmesg and filter out the test results
> from the dmesg and include them in the test script output.
>
> You can refer to other tests that do that: powerpc/scripts/hmi.sh
> is one example.
Thanks, will check that.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 14/14] mm: page_frag: add an entry in MAINTAINERS for page_frag
2024-10-09 0:43 ` Jakub Kicinski
@ 2024-10-09 4:01 ` Yunsheng Lin
2024-10-09 16:32 ` Paolo Abeni
0 siblings, 1 reply; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-09 4:01 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, pabeni, netdev, linux-kernel, Alexander Duyck
On 2024/10/9 8:43, Jakub Kicinski wrote:
> On Tue, 8 Oct 2024 19:20:48 +0800 Yunsheng Lin wrote:
>> +M: Yunsheng Lin <linyunsheng@huawei.com>
>
> The bar for maintaining core code is very high, if you'd
> like to be a maintainer please start small.
I did start small with the page_pool case, as mentioned in
[1] of a similar comment, and the page_frag is a small
subsystem/library as mentioned in commit log.
I think I still might need a second opinion here.
1. https://lore.kernel.org/linux-kernel/dea82ac3-65fc-c941-685f-9d4655aa4a52@huawei.com/
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 14/14] mm: page_frag: add an entry in MAINTAINERS for page_frag
2024-10-09 4:01 ` Yunsheng Lin
@ 2024-10-09 16:32 ` Paolo Abeni
2024-10-10 11:37 ` Yunsheng Lin
0 siblings, 1 reply; 30+ messages in thread
From: Paolo Abeni @ 2024-10-09 16:32 UTC (permalink / raw)
To: Yunsheng Lin, Jakub Kicinski; +Cc: davem, netdev, linux-kernel, Alexander Duyck
Hi,
On 10/9/24 06:01, Yunsheng Lin wrote:
> On 2024/10/9 8:43, Jakub Kicinski wrote:
>> On Tue, 8 Oct 2024 19:20:48 +0800 Yunsheng Lin wrote:
>>> +M: Yunsheng Lin <linyunsheng@huawei.com>
>>
>> The bar for maintaining core code is very high, if you'd
>> like to be a maintainer please start small.
>
> I did start small with the page_pool case, as mentioned in
> [1] of a similar comment, and the page_frag is a small
> subsystem/library as mentioned in commit log.
>
> I think I still might need a second opinion here.
>
> 1. https://lore.kernel.org/linux-kernel/dea82ac3-65fc-c941-685f-9d4655aa4a52@huawei.com/
Please note that the 'small' part here does not refer strictly to code
size. Any core networking code has the bar significantly higher than
i.e. NIC drivers - even if the latter could count order of magnitude
more LoC.
AFAICS there is an unwritten convention that people are called to
maintain core code, as opposed to people appointing themself to maintain
driver code.
Cheers,
Paolo
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 09/14] net: rename skb_copy_to_page_nocache() helper
2024-10-08 11:20 ` [PATCH net-next v20 09/14] net: rename skb_copy_to_page_nocache() helper Yunsheng Lin
@ 2024-10-09 23:40 ` Alexander Duyck
2024-10-10 11:32 ` Yunsheng Lin
0 siblings, 1 reply; 30+ messages in thread
From: Alexander Duyck @ 2024-10-09 23:40 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, kuba, pabeni, netdev, linux-kernel, Eric Dumazet,
David Ahern
On Tue, Oct 8, 2024 at 4:27 AM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>
> Rename skb_copy_to_page_nocache() to skb_add_frag_nocache()
> to avoid calling virt_to_page() as we are about to pass virtual
> address directly.
>
> CC: Alexander Duyck <alexander.duyck@gmail.com>
> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
> ---
> include/net/sock.h | 9 +++------
> net/ipv4/tcp.c | 7 +++----
> net/kcm/kcmsock.c | 7 +++----
> 3 files changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index e282127092ab..e0b4e2daca5d 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -2192,15 +2192,12 @@ static inline int skb_add_data_nocache(struct sock *sk, struct sk_buff *skb,
> return err;
> }
>
> -static inline int skb_copy_to_page_nocache(struct sock *sk, struct iov_iter *from,
> - struct sk_buff *skb,
> - struct page *page,
> - int off, int copy)
> +static inline int skb_add_frag_nocache(struct sock *sk, struct iov_iter *from,
> + struct sk_buff *skb, char *va, int copy)
This is not adding a frag. It is copying to a frag. This naming is a
hard no as there are functions that actually add frags to the skb and
this is not what this is doing. It sounds like it should be some
variant on skb_add_rx_frag and it isn't.
Instead of "_add_" I would suggest you stick with "_copy_to_" as the
action as the alternative would be confusing as it implies you are
going to be adding this to frags yourself.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 06/14] mm: page_frag: reuse existing space for 'size' and 'pfmemalloc'
2024-10-08 11:20 ` [PATCH net-next v20 06/14] mm: page_frag: reuse existing space for 'size' and 'pfmemalloc' Yunsheng Lin
@ 2024-10-09 23:50 ` Alexander Duyck
2024-10-10 11:32 ` Yunsheng Lin
0 siblings, 1 reply; 30+ messages in thread
From: Alexander Duyck @ 2024-10-09 23:50 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, kuba, pabeni, netdev, linux-kernel, Andrew Morton,
linux-mm
On Tue, Oct 8, 2024 at 4:27 AM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>
> Currently there is one 'struct page_frag' for every 'struct
> sock' and 'struct task_struct', we are about to replace the
> 'struct page_frag' with 'struct page_frag_cache' for them.
> Before begin the replacing, we need to ensure the size of
> 'struct page_frag_cache' is not bigger than the size of
> 'struct page_frag', as there may be tens of thousands of
> 'struct sock' and 'struct task_struct' instances in the
> system.
>
> By or'ing the page order & pfmemalloc with lower bits of
> 'va' instead of using 'u16' or 'u32' for page size and 'u8'
> for pfmemalloc, we are able to avoid 3 or 5 bytes space waste.
> And page address & pfmemalloc & order is unchanged for the
> same page in the same 'page_frag_cache' instance, it makes
> sense to fit them together.
>
> After this patch, the size of 'struct page_frag_cache' should be
> the same as the size of 'struct page_frag'.
>
> CC: Alexander Duyck <alexander.duyck@gmail.com>
> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
> ---
> include/linux/mm_types_task.h | 19 +++++----
> include/linux/page_frag_cache.h | 24 ++++++++++-
> mm/page_frag_cache.c | 75 +++++++++++++++++++++++----------
> 3 files changed, 86 insertions(+), 32 deletions(-)
>
> diff --git a/include/linux/mm_types_task.h b/include/linux/mm_types_task.h
> index 0ac6daebdd5c..a82aa80c0ba4 100644
> --- a/include/linux/mm_types_task.h
> +++ b/include/linux/mm_types_task.h
> @@ -47,18 +47,21 @@ struct page_frag {
> #define PAGE_FRAG_CACHE_MAX_SIZE __ALIGN_MASK(32768, ~PAGE_MASK)
> #define PAGE_FRAG_CACHE_MAX_ORDER get_order(PAGE_FRAG_CACHE_MAX_SIZE)
> struct page_frag_cache {
> - void *va;
> -#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
> + /* encoded_page consists of the virtual address, pfmemalloc bit and
> + * order of a page.
> + */
> + unsigned long encoded_page;
> +
> + /* we maintain a pagecount bias, so that we dont dirty cache line
> + * containing page->_refcount every time we allocate a fragment.
> + */
> +#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) && (BITS_PER_LONG <= 32)
> __u16 offset;
> - __u16 size;
> + __u16 pagecnt_bias;
> #else
> __u32 offset;
> + __u32 pagecnt_bias;
> #endif
> - /* we maintain a pagecount bias, so that we dont dirty cache line
> - * containing page->_refcount every time we allocate a fragment.
> - */
> - unsigned int pagecnt_bias;
> - bool pfmemalloc;
> };
>
> /* Track pages that require TLB flushes */
> diff --git a/include/linux/page_frag_cache.h b/include/linux/page_frag_cache.h
> index 0a52f7a179c8..dba2268e451a 100644
> --- a/include/linux/page_frag_cache.h
> +++ b/include/linux/page_frag_cache.h
> @@ -3,18 +3,38 @@
> #ifndef _LINUX_PAGE_FRAG_CACHE_H
> #define _LINUX_PAGE_FRAG_CACHE_H
>
> +#include <linux/bits.h>
> #include <linux/log2.h>
> #include <linux/mm_types_task.h>
> #include <linux/types.h>
>
> +#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
> +/* Use a full byte here to enable assembler optimization as the shift
> + * operation is usually expecting a byte.
> + */
> +#define PAGE_FRAG_CACHE_ORDER_MASK GENMASK(7, 0)
> +#else
> +/* Compiler should be able to figure out we don't read things as any value
> + * ANDed with 0 is 0.
> + */
> +#define PAGE_FRAG_CACHE_ORDER_MASK 0
> +#endif
> +
> +#define PAGE_FRAG_CACHE_PFMEMALLOC_BIT (PAGE_FRAG_CACHE_ORDER_MASK + 1)
> +
> +static inline bool page_frag_encoded_page_pfmemalloc(unsigned long encoded_page)
> +{
> + return !!(encoded_page & PAGE_FRAG_CACHE_PFMEMALLOC_BIT);
> +}
> +
Rather than calling this encoded_page_pfmemalloc you might just go
with decode_pfmemalloc. Also rather than passing the unsigned long we
might just want to pass the page_frag_cache pointer.
> static inline void page_frag_cache_init(struct page_frag_cache *nc)
> {
> - nc->va = NULL;
> + nc->encoded_page = 0;
> }
>
> static inline bool page_frag_cache_is_pfmemalloc(struct page_frag_cache *nc)
> {
> - return !!nc->pfmemalloc;
> + return page_frag_encoded_page_pfmemalloc(nc->encoded_page);
> }
>
> void page_frag_cache_drain(struct page_frag_cache *nc);
> diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
> index 4c8e04379cb3..4bff4de58808 100644
> --- a/mm/page_frag_cache.c
> +++ b/mm/page_frag_cache.c
> @@ -12,6 +12,7 @@
> * be used in the "frags" portion of skb_shared_info.
> */
>
> +#include <linux/build_bug.h>
> #include <linux/export.h>
> #include <linux/gfp_types.h>
> #include <linux/init.h>
> @@ -19,9 +20,41 @@
> #include <linux/page_frag_cache.h>
> #include "internal.h"
>
> +static unsigned long page_frag_encode_page(struct page *page, unsigned int order,
> + bool pfmemalloc)
> +{
> + BUILD_BUG_ON(PAGE_FRAG_CACHE_MAX_ORDER > PAGE_FRAG_CACHE_ORDER_MASK);
> + BUILD_BUG_ON(PAGE_FRAG_CACHE_PFMEMALLOC_BIT >= PAGE_SIZE);
> +
> + return (unsigned long)page_address(page) |
> + (order & PAGE_FRAG_CACHE_ORDER_MASK) |
> + ((unsigned long)pfmemalloc * PAGE_FRAG_CACHE_PFMEMALLOC_BIT);
> +}
> +
> +static unsigned long page_frag_encoded_page_order(unsigned long encoded_page)
> +{
> + return encoded_page & PAGE_FRAG_CACHE_ORDER_MASK;
> +}
> +
> +static void *page_frag_encoded_page_address(unsigned long encoded_page)
> +{
> + return (void *)(encoded_page & PAGE_MASK);
> +}
> +
> +static struct page *page_frag_encoded_page_ptr(unsigned long encoded_page)
> +{
> + return virt_to_page((void *)encoded_page);
> +}
> +
Same with these. Instead of calling it encoded_page_XXX we could
probably just go with decode_page, decode_order, and decode_address.
Also instead of passing an unsigned long it would make more sense to
be passing the page_frag_cache pointer, especially once you start
pulling these out of this block.
If you are wanting to just work with the raw unsigned long value in
the file it might make more sense to drop the "page_frag_" prefix from
it and just have functions for handling your "encoded_page_" value. In
that case you might rename page_frag_encode_page to
"encoded_page_encode" or something like that.
> +static unsigned int page_frag_cache_page_size(unsigned long encoded_page)
> +{
> + return PAGE_SIZE << page_frag_encoded_page_order(encoded_page);
> +}
> +
> static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
> gfp_t gfp_mask)
> {
> + unsigned long order = PAGE_FRAG_CACHE_MAX_ORDER;
> struct page *page = NULL;
> gfp_t gfp = gfp_mask;
>
> @@ -30,23 +63,26 @@ static struct page *__page_frag_cache_refill(struct page_frag_cache *nc,
> __GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC;
> page = alloc_pages_node(NUMA_NO_NODE, gfp_mask,
> PAGE_FRAG_CACHE_MAX_ORDER);
> - nc->size = page ? PAGE_FRAG_CACHE_MAX_SIZE : PAGE_SIZE;
> #endif
> - if (unlikely(!page))
> + if (unlikely(!page)) {
> page = alloc_pages_node(NUMA_NO_NODE, gfp, 0);
> + order = 0;
> + }
>
> - nc->va = page ? page_address(page) : NULL;
> + nc->encoded_page = page ?
> + page_frag_encode_page(page, order, page_is_pfmemalloc(page)) : 0;
>
> return page;
> }
>
> void page_frag_cache_drain(struct page_frag_cache *nc)
> {
> - if (!nc->va)
> + if (!nc->encoded_page)
> return;
>
> - __page_frag_cache_drain(virt_to_head_page(nc->va), nc->pagecnt_bias);
> - nc->va = NULL;
> + __page_frag_cache_drain(page_frag_encoded_page_ptr(nc->encoded_page),
> + nc->pagecnt_bias);
> + nc->encoded_page = 0;
> }
> EXPORT_SYMBOL(page_frag_cache_drain);
>
> @@ -63,35 +99,29 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
> unsigned int fragsz, gfp_t gfp_mask,
> unsigned int align_mask)
> {
> -#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
> - unsigned int size = nc->size;
> -#else
> - unsigned int size = PAGE_SIZE;
> -#endif
> - unsigned int offset;
> + unsigned long encoded_page = nc->encoded_page;
> + unsigned int size, offset;
> struct page *page;
>
> - if (unlikely(!nc->va)) {
> + if (unlikely(!encoded_page)) {
> refill:
> page = __page_frag_cache_refill(nc, gfp_mask);
> if (!page)
> return NULL;
>
> -#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
> - /* if size can vary use size else just use PAGE_SIZE */
> - size = nc->size;
> -#endif
> + encoded_page = nc->encoded_page;
> +
> /* Even if we own the page, we do not use atomic_set().
> * This would break get_page_unless_zero() users.
> */
> page_ref_add(page, PAGE_FRAG_CACHE_MAX_SIZE);
>
> /* reset page count bias and offset to start of new frag */
> - nc->pfmemalloc = page_is_pfmemalloc(page);
> nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
> nc->offset = 0;
> }
>
> + size = page_frag_cache_page_size(encoded_page);
> offset = __ALIGN_KERNEL_MASK(nc->offset, ~align_mask);
> if (unlikely(offset + fragsz > size)) {
> if (unlikely(fragsz > PAGE_SIZE)) {
> @@ -107,13 +137,14 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
> return NULL;
> }
>
> - page = virt_to_page(nc->va);
> + page = page_frag_encoded_page_ptr(encoded_page);
>
> if (!page_ref_sub_and_test(page, nc->pagecnt_bias))
> goto refill;
>
> - if (unlikely(nc->pfmemalloc)) {
> - free_unref_page(page, compound_order(page));
> + if (unlikely(page_frag_encoded_page_pfmemalloc(encoded_page))) {
> + free_unref_page(page,
> + page_frag_encoded_page_order(encoded_page));
> goto refill;
> }
>
> @@ -128,7 +159,7 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
> nc->pagecnt_bias--;
> nc->offset = offset + fragsz;
>
> - return nc->va + offset;
> + return page_frag_encoded_page_address(encoded_page) + offset;
> }
> EXPORT_SYMBOL(__page_frag_alloc_align);
>
> --
> 2.33.0
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 06/14] mm: page_frag: reuse existing space for 'size' and 'pfmemalloc'
2024-10-09 23:50 ` Alexander Duyck
@ 2024-10-10 11:32 ` Yunsheng Lin
2024-10-10 14:33 ` Alexander Duyck
0 siblings, 1 reply; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-10 11:32 UTC (permalink / raw)
To: Alexander Duyck
Cc: davem, kuba, pabeni, netdev, linux-kernel, Andrew Morton,
linux-mm
On 2024/10/10 7:50, Alexander Duyck wrote:
...
>> +
>> +#define PAGE_FRAG_CACHE_PFMEMALLOC_BIT (PAGE_FRAG_CACHE_ORDER_MASK + 1)
>> +
>> +static inline bool page_frag_encoded_page_pfmemalloc(unsigned long encoded_page)
>> +{
>> + return !!(encoded_page & PAGE_FRAG_CACHE_PFMEMALLOC_BIT);
>> +}
>> +
>
> Rather than calling this encoded_page_pfmemalloc you might just go
> with decode_pfmemalloc. Also rather than passing the unsigned long we
> might just want to pass the page_frag_cache pointer.
As the page_frag_encoded_page_pfmemalloc() is also called in
__page_frag_alloc_align(), and __page_frag_alloc_align() uses a
local variable for 'nc->encoded_page' to avoid fetching from
page_frag_cache pointer multi-times, so passing an 'unsigned long'
is perferred here?
I am not sure if decode_pfmemalloc() is simple enough that it
might be conflicted with naming from other subsystem in the
future. I thought about adding a '__' prefix to it, but the naming
seems long enough that some inline helper' naming is over 80 characters.
>
>> static inline void page_frag_cache_init(struct page_frag_cache *nc)
>> {
>> - nc->va = NULL;
>> + nc->encoded_page = 0;
>> }
>>
>> static inline bool page_frag_cache_is_pfmemalloc(struct page_frag_cache *nc)
>> {
>> - return !!nc->pfmemalloc;
>> + return page_frag_encoded_page_pfmemalloc(nc->encoded_page);
>> }
>>
>> void page_frag_cache_drain(struct page_frag_cache *nc);
>> diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
>> index 4c8e04379cb3..4bff4de58808 100644
>> --- a/mm/page_frag_cache.c
>> +++ b/mm/page_frag_cache.c
>> @@ -12,6 +12,7 @@
>> * be used in the "frags" portion of skb_shared_info.
>> */
>>
>> +#include <linux/build_bug.h>
>> #include <linux/export.h>
>> #include <linux/gfp_types.h>
>> #include <linux/init.h>
>> @@ -19,9 +20,41 @@
>> #include <linux/page_frag_cache.h>
>> #include "internal.h"
>>
>> +static unsigned long page_frag_encode_page(struct page *page, unsigned int order,
>> + bool pfmemalloc)
>> +{
>> + BUILD_BUG_ON(PAGE_FRAG_CACHE_MAX_ORDER > PAGE_FRAG_CACHE_ORDER_MASK);
>> + BUILD_BUG_ON(PAGE_FRAG_CACHE_PFMEMALLOC_BIT >= PAGE_SIZE);
>> +
>> + return (unsigned long)page_address(page) |
>> + (order & PAGE_FRAG_CACHE_ORDER_MASK) |
>> + ((unsigned long)pfmemalloc * PAGE_FRAG_CACHE_PFMEMALLOC_BIT);
>> +}
>> +
>> +static unsigned long page_frag_encoded_page_order(unsigned long encoded_page)
>> +{
>> + return encoded_page & PAGE_FRAG_CACHE_ORDER_MASK;
>> +}
>> +
>> +static void *page_frag_encoded_page_address(unsigned long encoded_page)
>> +{
>> + return (void *)(encoded_page & PAGE_MASK);
>> +}
>> +
>> +static struct page *page_frag_encoded_page_ptr(unsigned long encoded_page)
>> +{
>> + return virt_to_page((void *)encoded_page);
>> +}
>> +
>
> Same with these. Instead of calling it encoded_page_XXX we could
> probably just go with decode_page, decode_order, and decode_address.
> Also instead of passing an unsigned long it would make more sense to
> be passing the page_frag_cache pointer, especially once you start
> pulling these out of this block.
For the not passing the page_frag_cache pointer part, it is the same
as above, it is mainly to avoid fetching from pointer multi-times.
>
> If you are wanting to just work with the raw unsigned long value in
> the file it might make more sense to drop the "page_frag_" prefix from
> it and just have functions for handling your "encoded_page_" value. In
> that case you might rename page_frag_encode_page to
> "encoded_page_encode" or something like that.
It am supposing you meant 'encoded_page_decode' here instead of
"encoded_page_encode"?
Something like below?
encoded_page_decode_pfmemalloc()
encoded_page_decode_order()
encoded_page_decode_page()
encoded_page_decode_virt()
>
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 09/14] net: rename skb_copy_to_page_nocache() helper
2024-10-09 23:40 ` Alexander Duyck
@ 2024-10-10 11:32 ` Yunsheng Lin
2024-10-10 14:39 ` Alexander Duyck
0 siblings, 1 reply; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-10 11:32 UTC (permalink / raw)
To: Alexander Duyck
Cc: davem, kuba, pabeni, netdev, linux-kernel, Eric Dumazet,
David Ahern
On 2024/10/10 7:40, Alexander Duyck wrote:
> On Tue, Oct 8, 2024 at 4:27 AM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>>
>> Rename skb_copy_to_page_nocache() to skb_add_frag_nocache()
>> to avoid calling virt_to_page() as we are about to pass virtual
>> address directly.
>>
>> CC: Alexander Duyck <alexander.duyck@gmail.com>
>> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
>> ---
>> include/net/sock.h | 9 +++------
>> net/ipv4/tcp.c | 7 +++----
>> net/kcm/kcmsock.c | 7 +++----
>> 3 files changed, 9 insertions(+), 14 deletions(-)
>>
>> diff --git a/include/net/sock.h b/include/net/sock.h
>> index e282127092ab..e0b4e2daca5d 100644
>> --- a/include/net/sock.h
>> +++ b/include/net/sock.h
>> @@ -2192,15 +2192,12 @@ static inline int skb_add_data_nocache(struct sock *sk, struct sk_buff *skb,
>> return err;
>> }
>>
>> -static inline int skb_copy_to_page_nocache(struct sock *sk, struct iov_iter *from,
>> - struct sk_buff *skb,
>> - struct page *page,
>> - int off, int copy)
>> +static inline int skb_add_frag_nocache(struct sock *sk, struct iov_iter *from,
>> + struct sk_buff *skb, char *va, int copy)
>
> This is not adding a frag. It is copying to a frag. This naming is a
> hard no as there are functions that actually add frags to the skb and
> this is not what this is doing. It sounds like it should be some
> variant on skb_add_rx_frag and it isn't.
>
> Instead of "_add_" I would suggest you stick with "_copy_to_" as the
> action as the alternative would be confusing as it implies you are
> going to be adding this to frags yourself.
I though we had reached a agreement in [1]? I guessed the 'That works
for me' only refer to the 'sk_' prefix?
The argumemt is that "skb_add_data_nocache() does memcpy'ing to skb->data
and update skb->len only by calling skb_put()" without calling something as
pskb_expand_head() to add more tailroom, so skb_add_frag_nocache is mirroring
that.
Does it mean skb_add_data_nocache() may be renamed to skb_copy_to_data_nocache()
in the future?
1. https://lore.kernel.org/all/CAKgT0Ue=tX+hKWiXQaM-6ypZ8fGvcUagGKfVrNGtRHVuhMX80g@mail.gmail.com/
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 14/14] mm: page_frag: add an entry in MAINTAINERS for page_frag
2024-10-09 16:32 ` Paolo Abeni
@ 2024-10-10 11:37 ` Yunsheng Lin
0 siblings, 0 replies; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-10 11:37 UTC (permalink / raw)
To: Paolo Abeni, Jakub Kicinski; +Cc: davem, netdev, linux-kernel, Alexander Duyck
On 2024/10/10 0:32, Paolo Abeni wrote:
> Hi,
Hi,
>
> On 10/9/24 06:01, Yunsheng Lin wrote:
>> On 2024/10/9 8:43, Jakub Kicinski wrote:
>>> On Tue, 8 Oct 2024 19:20:48 +0800 Yunsheng Lin wrote:
>>>> +M: Yunsheng Lin <linyunsheng@huawei.com>
>>>
>>> The bar for maintaining core code is very high, if you'd
>>> like to be a maintainer please start small.
>>
>> I did start small with the page_pool case, as mentioned in
>> [1] of a similar comment, and the page_frag is a small
>> subsystem/library as mentioned in commit log.
>>
>> I think I still might need a second opinion here.
>>
>> 1. https://lore.kernel.org/linux-kernel/dea82ac3-65fc-c941-685f-9d4655aa4a52@huawei.com/
>
> Please note that the 'small' part here does not refer strictly to code size. Any core networking code has the bar significantly higher than i.e. NIC drivers - even if the latter could count order of magnitude more LoC.
> AFAICS there is an unwritten convention that people are called to maintain core code, as opposed to people appointing themself to maintain driver code.
Is there any discussion that is referring to above 'unwritten convention'?
As my pool community experience tells me the above 'unwritten
convention' is mainly referring to well-established subsystem that is
already in the MAINTAINERS, and page_frag is not really a subsystem or
library before this patchset, it seems common to me that someone being
willing and able to turn it into a subsystem or library might become the
co-maintainer if she/he is also willing to co-maintain it.
>
> Cheers,
>
> Paolo
>
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 06/14] mm: page_frag: reuse existing space for 'size' and 'pfmemalloc'
2024-10-10 11:32 ` Yunsheng Lin
@ 2024-10-10 14:33 ` Alexander Duyck
2024-10-11 11:40 ` Yunsheng Lin
0 siblings, 1 reply; 30+ messages in thread
From: Alexander Duyck @ 2024-10-10 14:33 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, kuba, pabeni, netdev, linux-kernel, Andrew Morton,
linux-mm
On Thu, Oct 10, 2024 at 4:32 AM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>
> On 2024/10/10 7:50, Alexander Duyck wrote:
>
> ...
>
> >> +
> >> +#define PAGE_FRAG_CACHE_PFMEMALLOC_BIT (PAGE_FRAG_CACHE_ORDER_MASK + 1)
> >> +
> >> +static inline bool page_frag_encoded_page_pfmemalloc(unsigned long encoded_page)
> >> +{
> >> + return !!(encoded_page & PAGE_FRAG_CACHE_PFMEMALLOC_BIT);
> >> +}
> >> +
> >
> > Rather than calling this encoded_page_pfmemalloc you might just go
> > with decode_pfmemalloc. Also rather than passing the unsigned long we
> > might just want to pass the page_frag_cache pointer.
> As the page_frag_encoded_page_pfmemalloc() is also called in
> __page_frag_alloc_align(), and __page_frag_alloc_align() uses a
> local variable for 'nc->encoded_page' to avoid fetching from
> page_frag_cache pointer multi-times, so passing an 'unsigned long'
> is perferred here?
>
> I am not sure if decode_pfmemalloc() is simple enough that it
> might be conflicted with naming from other subsystem in the
> future. I thought about adding a '__' prefix to it, but the naming
> seems long enough that some inline helper' naming is over 80 characters.
What you might do is look at having a page_frag version of the
function and a encoded_page version as I called out below with the
naming. It would make sense to call the two out separately as this is
operating on an encoded page, not a page frag. With that we can avoid
any sort of naming confusion.
> >
> >> static inline void page_frag_cache_init(struct page_frag_cache *nc)
> >> {
> >> - nc->va = NULL;
> >> + nc->encoded_page = 0;
> >> }
> >>
> >> static inline bool page_frag_cache_is_pfmemalloc(struct page_frag_cache *nc)
> >> {
> >> - return !!nc->pfmemalloc;
> >> + return page_frag_encoded_page_pfmemalloc(nc->encoded_page);
> >> }
> >>
> >> void page_frag_cache_drain(struct page_frag_cache *nc);
> >> diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
> >> index 4c8e04379cb3..4bff4de58808 100644
> >> --- a/mm/page_frag_cache.c
> >> +++ b/mm/page_frag_cache.c
> >> @@ -12,6 +12,7 @@
> >> * be used in the "frags" portion of skb_shared_info.
> >> */
> >>
> >> +#include <linux/build_bug.h>
> >> #include <linux/export.h>
> >> #include <linux/gfp_types.h>
> >> #include <linux/init.h>
> >> @@ -19,9 +20,41 @@
> >> #include <linux/page_frag_cache.h>
> >> #include "internal.h"
> >>
> >> +static unsigned long page_frag_encode_page(struct page *page, unsigned int order,
> >> + bool pfmemalloc)
> >> +{
> >> + BUILD_BUG_ON(PAGE_FRAG_CACHE_MAX_ORDER > PAGE_FRAG_CACHE_ORDER_MASK);
> >> + BUILD_BUG_ON(PAGE_FRAG_CACHE_PFMEMALLOC_BIT >= PAGE_SIZE);
> >> +
> >> + return (unsigned long)page_address(page) |
> >> + (order & PAGE_FRAG_CACHE_ORDER_MASK) |
> >> + ((unsigned long)pfmemalloc * PAGE_FRAG_CACHE_PFMEMALLOC_BIT);
> >> +}
> >> +
> >> +static unsigned long page_frag_encoded_page_order(unsigned long encoded_page)
> >> +{
> >> + return encoded_page & PAGE_FRAG_CACHE_ORDER_MASK;
> >> +}
> >> +
> >> +static void *page_frag_encoded_page_address(unsigned long encoded_page)
> >> +{
> >> + return (void *)(encoded_page & PAGE_MASK);
> >> +}
> >> +
> >> +static struct page *page_frag_encoded_page_ptr(unsigned long encoded_page)
> >> +{
> >> + return virt_to_page((void *)encoded_page);
> >> +}
> >> +
> >
> > Same with these. Instead of calling it encoded_page_XXX we could
> > probably just go with decode_page, decode_order, and decode_address.
> > Also instead of passing an unsigned long it would make more sense to
> > be passing the page_frag_cache pointer, especially once you start
> > pulling these out of this block.
>
> For the not passing the page_frag_cache pointer part, it is the same
> as above, it is mainly to avoid fetching from pointer multi-times.
>
> >
> > If you are wanting to just work with the raw unsigned long value in
> > the file it might make more sense to drop the "page_frag_" prefix from
> > it and just have functions for handling your "encoded_page_" value. In
> > that case you might rename page_frag_encode_page to
> > "encoded_page_encode" or something like that.
>
> It am supposing you meant 'encoded_page_decode' here instead of
> "encoded_page_encode"?
> Something like below?
> encoded_page_decode_pfmemalloc()
> encoded_page_decode_order()
> encoded_page_decode_page()
> encoded_page_decode_virt()
For the decodes yes. I was referring to page_frag_encode_page.
Basically the output from that isn't anything page frag, it is your
encoded page type so you could probably just call it
encoded_page_encode, or encoded_page_create or something like that.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 09/14] net: rename skb_copy_to_page_nocache() helper
2024-10-10 11:32 ` Yunsheng Lin
@ 2024-10-10 14:39 ` Alexander Duyck
0 siblings, 0 replies; 30+ messages in thread
From: Alexander Duyck @ 2024-10-10 14:39 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, kuba, pabeni, netdev, linux-kernel, Eric Dumazet,
David Ahern
On Thu, Oct 10, 2024 at 4:32 AM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>
> On 2024/10/10 7:40, Alexander Duyck wrote:
> > On Tue, Oct 8, 2024 at 4:27 AM Yunsheng Lin <linyunsheng@huawei.com> wrote:
> >>
> >> Rename skb_copy_to_page_nocache() to skb_add_frag_nocache()
> >> to avoid calling virt_to_page() as we are about to pass virtual
> >> address directly.
> >>
> >> CC: Alexander Duyck <alexander.duyck@gmail.com>
> >> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
> >> ---
> >> include/net/sock.h | 9 +++------
> >> net/ipv4/tcp.c | 7 +++----
> >> net/kcm/kcmsock.c | 7 +++----
> >> 3 files changed, 9 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/include/net/sock.h b/include/net/sock.h
> >> index e282127092ab..e0b4e2daca5d 100644
> >> --- a/include/net/sock.h
> >> +++ b/include/net/sock.h
> >> @@ -2192,15 +2192,12 @@ static inline int skb_add_data_nocache(struct sock *sk, struct sk_buff *skb,
> >> return err;
> >> }
> >>
> >> -static inline int skb_copy_to_page_nocache(struct sock *sk, struct iov_iter *from,
> >> - struct sk_buff *skb,
> >> - struct page *page,
> >> - int off, int copy)
> >> +static inline int skb_add_frag_nocache(struct sock *sk, struct iov_iter *from,
> >> + struct sk_buff *skb, char *va, int copy)
> >
> > This is not adding a frag. It is copying to a frag. This naming is a
> > hard no as there are functions that actually add frags to the skb and
> > this is not what this is doing. It sounds like it should be some
> > variant on skb_add_rx_frag and it isn't.
> >
> > Instead of "_add_" I would suggest you stick with "_copy_to_" as the
> > action as the alternative would be confusing as it implies you are
> > going to be adding this to frags yourself.
>
> I though we had reached a agreement in [1]? I guessed the 'That works
> for me' only refer to the 'sk_' prefix?
>
> The argumemt is that "skb_add_data_nocache() does memcpy'ing to skb->data
> and update skb->len only by calling skb_put()" without calling something as
> pskb_expand_head() to add more tailroom, so skb_add_frag_nocache is mirroring
> that.
>
> Does it mean skb_add_data_nocache() may be renamed to skb_copy_to_data_nocache()
> in the future?
>
> 1. https://lore.kernel.org/all/CAKgT0Ue=tX+hKWiXQaM-6ypZ8fGvcUagGKfVrNGtRHVuhMX80g@mail.gmail.com/
Sorry, I overlooked the part where you mentioned skb_add_frag_nocache.
For some reason I was thinking you were going with the
skb_copy_to_data_nocache. The issue is that adding a frag has a
meaning and sounds similar to other existing functions. By sticking
with the data_nocache suffix it stays closer to the other similar
functions.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 01/14] mm: page_frag: add a test module for page_frag
2024-10-09 3:59 ` Yunsheng Lin
@ 2024-10-10 21:18 ` Shuah Khan
0 siblings, 0 replies; 30+ messages in thread
From: Shuah Khan @ 2024-10-10 21:18 UTC (permalink / raw)
To: Yunsheng Lin, davem, kuba, pabeni
Cc: netdev, linux-kernel, Alexander Duyck, Alexander Duyck,
Andrew Morton, Shuah Khan, linux-mm, linux-kselftest, Shuah Khan
On 10/8/24 21:59, Yunsheng Lin wrote:
> On 2024/10/9 3:56, Shuah Khan wrote:
>> On 10/8/24 05:20, Yunsheng Lin wrote:
>>> The testing is done by ensuring that the fragment allocated
>>> from a frag_frag_cache instance is pushed into a ptr_ring
>>> instance in a kthread binded to a specified cpu, and a kthread
>>> binded to a specified cpu will pop the fragment from the
>>> ptr_ring and free the fragment.
>>>
>>> CC: Alexander Duyck <alexander.duyck@gmail.com>
>>> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
>>> Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
>>
>> Signed-off-by should be last. Same comment on all the other
>
> Hi, Shuah
>
> I used 'git am' to collect those tag, it seems that is the order
> the tool applied, and I checking other applied commit, it seems
> only Signed-off-by from the committer is the last, like the below
> recent mm commit:
> 6901cf55de22
> ff7f5ad7bce4
>
okay.
>> patches in this series. When you have 4 patches, it is a good
>> practice to add cover-letter.
>
> I guess the cover-letter meant below?
> https://lore.kernel.org/all/20241008112049.2279307-1-linyunsheng@huawei.com/
Somehow this isn't in my Inbox.
>
>>
[snip]
> ...
>
>>> +function run_manual_check()
>>> +{
>>> + #
>>> + # Validate passed parameters. If there is wrong one,
>>> + # the script exists and does not execute further.
>>> + #
>>> + validate_passed_args $@
>>> +
>>> + echo "Run the test with following parameters: $@"
>>
>> Is this marker good enough to isolate the test results in the
>> dmesg? Include the test name in the message.
>>
>>
>>> + insmod $DRIVER $@ > /dev/null 2>&1
>>> + echo "Done."
>>
>> Is this marker good enough to isolate the test results in the
>> dmesg? Include the test name in the message.
>>
>>> + echo "Check the kernel ring buffer to see the summary."
>>
>> Usually the test would run dmesg and filter out the test results
>> from the dmesg and include them in the test script output.
>>
>> You can refer to other tests that do that: powerpc/scripts/hmi.sh
>> is one example.
>
> Thanks, will check that.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 06/14] mm: page_frag: reuse existing space for 'size' and 'pfmemalloc'
2024-10-10 14:33 ` Alexander Duyck
@ 2024-10-11 11:40 ` Yunsheng Lin
2024-10-11 15:31 ` Alexander Duyck
0 siblings, 1 reply; 30+ messages in thread
From: Yunsheng Lin @ 2024-10-11 11:40 UTC (permalink / raw)
To: Alexander Duyck
Cc: davem, kuba, pabeni, netdev, linux-kernel, Andrew Morton,
linux-mm
On 2024/10/10 22:33, Alexander Duyck wrote:
...
>
> For the decodes yes. I was referring to page_frag_encode_page.
> Basically the output from that isn't anything page frag, it is your
> encoded page type so you could probably just call it
> encoded_page_encode, or encoded_page_create or something like that.
It is kind of confusing as there is some mix of encode/encoded/decode
here, but let's be more specific if it is something like below:
static unsigned long encoded_page_create(struct page *page, unsigned int order,
bool pfmemalloc)
{
BUILD_BUG_ON(PAGE_FRAG_CACHE_MAX_ORDER > PAGE_FRAG_CACHE_ORDER_MASK);
BUILD_BUG_ON(PAGE_FRAG_CACHE_PFMEMALLOC_BIT >= PAGE_SIZE);
return (unsigned long)page_address(page) |
(order & PAGE_FRAG_CACHE_ORDER_MASK) |
((unsigned long)pfmemalloc * PAGE_FRAG_CACHE_PFMEMALLOC_BIT);
}
static inline bool encoded_page_decode_pfmemalloc(unsigned long encoded_page)
{
return !!(encoded_page & PAGE_FRAG_CACHE_PFMEMALLOC_BIT);
}
static unsigned long encoded_page_decode_order(unsigned long encoded_page)
{
return encoded_page & PAGE_FRAG_CACHE_ORDER_MASK;
}
static void *encoded_page_decode_virt(unsigned long encoded_page)
{
return (void *)(encoded_page & PAGE_MASK);
}
static struct page *encoded_page_decode_page(unsigned long encoded_page)
{
return virt_to_page((void *)encoded_page);
}
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH net-next v20 06/14] mm: page_frag: reuse existing space for 'size' and 'pfmemalloc'
2024-10-11 11:40 ` Yunsheng Lin
@ 2024-10-11 15:31 ` Alexander Duyck
0 siblings, 0 replies; 30+ messages in thread
From: Alexander Duyck @ 2024-10-11 15:31 UTC (permalink / raw)
To: Yunsheng Lin
Cc: davem, kuba, pabeni, netdev, linux-kernel, Andrew Morton,
linux-mm
On Fri, Oct 11, 2024 at 4:40 AM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>
> On 2024/10/10 22:33, Alexander Duyck wrote:
>
> ...
>
> >
> > For the decodes yes. I was referring to page_frag_encode_page.
> > Basically the output from that isn't anything page frag, it is your
> > encoded page type so you could probably just call it
> > encoded_page_encode, or encoded_page_create or something like that.
>
> It is kind of confusing as there is some mix of encode/encoded/decode
> here, but let's be more specific if it is something like below:
>
> static unsigned long encoded_page_create(struct page *page, unsigned int order,
> bool pfmemalloc)
> {
> BUILD_BUG_ON(PAGE_FRAG_CACHE_MAX_ORDER > PAGE_FRAG_CACHE_ORDER_MASK);
> BUILD_BUG_ON(PAGE_FRAG_CACHE_PFMEMALLOC_BIT >= PAGE_SIZE);
>
> return (unsigned long)page_address(page) |
> (order & PAGE_FRAG_CACHE_ORDER_MASK) |
> ((unsigned long)pfmemalloc * PAGE_FRAG_CACHE_PFMEMALLOC_BIT);
> }
>
> static inline bool encoded_page_decode_pfmemalloc(unsigned long encoded_page)
> {
> return !!(encoded_page & PAGE_FRAG_CACHE_PFMEMALLOC_BIT);
> }
>
> static unsigned long encoded_page_decode_order(unsigned long encoded_page)
> {
> return encoded_page & PAGE_FRAG_CACHE_ORDER_MASK;
> }
>
> static void *encoded_page_decode_virt(unsigned long encoded_page)
> {
> return (void *)(encoded_page & PAGE_MASK);
> }
>
> static struct page *encoded_page_decode_page(unsigned long encoded_page)
> {
> return virt_to_page((void *)encoded_page);
> }
Yes, this is what I had in mind. Basically the encoded_page is the
object you are working on so it becomes the prefix for the function
name and the action is the suffix, so you are either doing a "create"
to put together the object, or performing a "decode" to get the
individual components.
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2024-10-11 15:32 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-08 11:20 [PATCH net-next v20 00/14] Replace page_frag with page_frag_cache for sk_page_frag() Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 01/14] mm: page_frag: add a test module for page_frag Yunsheng Lin
2024-10-08 19:56 ` Shuah Khan
2024-10-09 3:59 ` Yunsheng Lin
2024-10-10 21:18 ` Shuah Khan
2024-10-08 11:20 ` [PATCH net-next v20 02/14] mm: move the page fragment allocator from page_alloc into its own file Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 03/14] mm: page_frag: use initial zero offset for page_frag_alloc_align() Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 04/14] mm: page_frag: avoid caller accessing 'page_frag_cache' directly Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 05/14] xtensa: remove the get_order() implementation Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 06/14] mm: page_frag: reuse existing space for 'size' and 'pfmemalloc' Yunsheng Lin
2024-10-09 23:50 ` Alexander Duyck
2024-10-10 11:32 ` Yunsheng Lin
2024-10-10 14:33 ` Alexander Duyck
2024-10-11 11:40 ` Yunsheng Lin
2024-10-11 15:31 ` Alexander Duyck
2024-10-08 11:20 ` [PATCH net-next v20 07/14] mm: page_frag: some minor refactoring before adding new API Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 08/14] mm: page_frag: use __alloc_pages() to replace alloc_pages_node() Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 09/14] net: rename skb_copy_to_page_nocache() helper Yunsheng Lin
2024-10-09 23:40 ` Alexander Duyck
2024-10-10 11:32 ` Yunsheng Lin
2024-10-10 14:39 ` Alexander Duyck
2024-10-08 11:20 ` [PATCH net-next v20 10/14] mm: page_frag: introduce prepare/probe/commit API Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 11/14] mm: page_frag: add testing for the newly added prepare API Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 12/14] net: replace page_frag with page_frag_cache Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 13/14] mm: page_frag: update documentation for page_frag Yunsheng Lin
2024-10-08 11:20 ` [PATCH net-next v20 14/14] mm: page_frag: add an entry in MAINTAINERS " Yunsheng Lin
2024-10-09 0:43 ` Jakub Kicinski
2024-10-09 4:01 ` Yunsheng Lin
2024-10-09 16:32 ` Paolo Abeni
2024-10-10 11:37 ` Yunsheng Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).