[PATCH net-next v22 00/14] Replace page_frag with page_frag_cache for sk_page_frag()

Yunsheng Lin posted 14 patches 1 month, 1 week ago
Documentation/mm/page_frags.rst               | 176 ++++++-
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               | 457 ++++++++++++++++++
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                          | 238 +++++++++
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/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   | 264 ++++++++++
tools/testing/selftests/mm/run_vmtests.sh     |  12 +
tools/testing/selftests/mm/test_page_frag.sh  | 202 ++++++++
38 files changed, 1711 insertions(+), 479 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
[PATCH net-next v22 00/14] Replace page_frag with page_frag_cache for sk_page_frag()
Posted by Yunsheng Lin 1 month, 1 week ago
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>
CC: Shuah Khan <skhan@linuxfoundation.org>

1. https://lore.kernel.org/all/20240228093013.8263-1-linyunsheng@huawei.com/

Change log:
V22:
   1. Fix some typo as noted by Bagas.
   2. Remove page_frag_cache_page_offset() as it is not really related to
      this patchset.

V21:
   1. Do renaming as suggested by Alexander.
   2. Filter out the test results of dmesg in script as suggested by
      Shuah.

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               | 176 ++++++-
 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               | 457 ++++++++++++++++++
 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                          | 238 +++++++++
 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/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   | 264 ++++++++++
 tools/testing/selftests/mm/run_vmtests.sh     |  12 +
 tools/testing/selftests/mm/test_page_frag.sh  | 202 ++++++++
 38 files changed, 1711 insertions(+), 479 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
Re: [PATCH net-next v22 00/14] Replace page_frag with page_frag_cache for sk_page_frag()
Posted by Alexander Duyck 1 month, 1 week ago
On Fri, Oct 18, 2024 at 4:00 AM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>
> 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:
>

So I still think this set should be split in half in order to make
this easier to review. The ones I have provided a review-by for so far
seem fine to me. I really think if you just submitted that batch first
we can get that landed and let them stew in the kernel for a bit to
make sure we didn't miss anything there.

As far as the others there is a bunch there for me to try and chew
through. A bunch of that is stuff not related necessarily to my
version of the page frag stuff that I did so merging the two is a bit
less obvious to me. The one thing I am wondering about is the behavior
for why we are pulling apart the logic with this "commit" approach
that is deferring the offset update which seems to have to happen
unless we need to abort.

My review time is going to be limited for the next several weeks. As
such I will likely not be able to get to a review until Friday or
Saturday each week so sending out updates faster than that will not
get you any additional reviews from me.

Thanks,

- Alex
Re: [PATCH net-next v22 00/14] Replace page_frag with page_frag_cache for sk_page_frag()
Posted by Yunsheng Lin 1 month, 1 week ago
On 10/19/2024 1:39 AM, Alexander Duyck wrote:
>>
> 
> So I still think this set should be split in half in order to make
> this easier to review. The ones I have provided a review-by for so far
> seem fine to me. I really think if you just submitted that batch first
> we can get that landed and let them stew in the kernel for a bit to
> make sure we didn't miss anything there.

It makes sense to me too that it might be better to get those submitted
to get more testing if there is no more comment about it.

I am guessing they should be targetting net-next tree to get more
testing as all the callers of page_frag API seem to be in the
networking, right?

Hi, David, Jakub & Paolo
It would be good if those patches are just cherry-picked from this
patchset as those patches with 'Reviewed-by' tag seem to be applying
cleanly. Or any better suggestion here?

> 
> As far as the others there is a bunch there for me to try and chew
> through. A bunch of that is stuff not related necessarily to my
> version of the page frag stuff that I did so merging the two is a bit
> less obvious to me. The one thing I am wondering about is the behavior
> for why we are pulling apart the logic with this "commit" approach
> that is deferring the offset update which seems to have to happen
> unless we need to abort.

Let's discuss that in patch 7.

> 
> My review time is going to be limited for the next several weeks. As
> such I will likely not be able to get to a review until Friday or
> Saturday each week so sending out updates faster than that will not
> get you any additional reviews from me.

Thanks for the time reviewing and reminding about the above.
It makes sense to have more time to have more reviewing as long as we
are making productive progress.

> 
> Thanks,
> 
> - Alex
>
Re: [PATCH net-next v22 00/14] Replace page_frag with page_frag_cache for sk_page_frag()
Posted by Paolo Abeni 1 month ago
Hi,

I just noted MM maintainer and ML was not CC on the cover-letter (but
they were on the relevant patches), adding them now.

On 10/19/24 10:27, Yunsheng Lin wrote:
> On 10/19/2024 1:39 AM, Alexander Duyck wrote:
>> So I still think this set should be split in half in order to make
>> this easier to review. The ones I have provided a review-by for so far
>> seem fine to me. I really think if you just submitted that batch first
>> we can get that landed and let them stew in the kernel for a bit to
>> make sure we didn't miss anything there.
> 
> It makes sense to me too that it might be better to get those submitted
> to get more testing if there is no more comment about it.
> 
> I am guessing they should be targetting net-next tree to get more
> testing as all the callers of page_frag API seem to be in the
> networking, right?
> 
> Hi, David, Jakub & Paolo
> It would be good if those patches are just cherry-picked from this
> patchset as those patches with 'Reviewed-by' tag seem to be applying
> cleanly. Or any better suggestion here?

We can cherry pick the patches from the posted series, applying the
review tags as needed, but we need an explicit ack from the mm
maintainer, given the mentioned patches touch mostly such code.

I would like to avoid repeating a recent incident of unintentionally
stepping on other subsystem toes.

@Andrew: are you ok with the above plan?

Thank you,

Paolo
Re: [PATCH net-next v22 00/14] Replace page_frag with page_frag_cache for sk_page_frag()
Posted by Yunsheng Lin 1 month ago
Hi, Andrew

On 10/24/2024 5:05 PM, Paolo Abeni wrote:
> Hi,
> 
> I just noted MM maintainer and ML was not CC on the cover-letter (but
> they were on the relevant patches), adding them now.
> 
> On 10/19/24 10:27, Yunsheng Lin wrote:
>> On 10/19/2024 1:39 AM, Alexander Duyck wrote:
>>> So I still think this set should be split in half in order to make
>>> this easier to review. The ones I have provided a review-by for so far
>>> seem fine to me. I really think if you just submitted that batch first
>>> we can get that landed and let them stew in the kernel for a bit to
>>> make sure we didn't miss anything there.
>>
>> It makes sense to me too that it might be better to get those submitted
>> to get more testing if there is no more comment about it.
>>
>> I am guessing they should be targetting net-next tree to get more
>> testing as all the callers of page_frag API seem to be in the
>> networking, right?
>>
>> Hi, David, Jakub & Paolo
>> It would be good if those patches are just cherry-picked from this
>> patchset as those patches with 'Reviewed-by' tag seem to be applying
>> cleanly. Or any better suggestion here?
> 
> We can cherry pick the patches from the posted series, applying the
> review tags as needed, but we need an explicit ack from the mm
> maintainer, given the mentioned patches touch mostly such code.
> 
> I would like to avoid repeating a recent incident of unintentionally
> stepping on other subsystem toes.
> 
> @Andrew: are you ok with the above plan?

Are the above patches cherry-picked to net-next tree ok with you?
More specifically, they are patch 1, 2, 3, 4, 5, 6, 8 with at least
one 'Acked-by' or 'Reviewed-by' tag.

Or any better suggestion about the plan?
Re: [PATCH net-next v22 00/14] Replace page_frag with page_frag_cache for sk_page_frag()
Posted by Yunsheng Lin 1 month ago
On 2024/10/24 17:05, Paolo Abeni wrote:
> Hi,
> 
> I just noted MM maintainer and ML was not CC on the cover-letter (but
> they were on the relevant patches), adding them now.
> 
> On 10/19/24 10:27, Yunsheng Lin wrote:
>> On 10/19/2024 1:39 AM, Alexander Duyck wrote:
>>> So I still think this set should be split in half in order to make
>>> this easier to review. The ones I have provided a review-by for so far
>>> seem fine to me. I really think if you just submitted that batch first
>>> we can get that landed and let them stew in the kernel for a bit to
>>> make sure we didn't miss anything there.
>>
>> It makes sense to me too that it might be better to get those submitted
>> to get more testing if there is no more comment about it.
>>
>> I am guessing they should be targetting net-next tree to get more
>> testing as all the callers of page_frag API seem to be in the
>> networking, right?
>>
>> Hi, David, Jakub & Paolo
>> It would be good if those patches are just cherry-picked from this
>> patchset as those patches with 'Reviewed-by' tag seem to be applying
>> cleanly. Or any better suggestion here?
> 
> We can cherry pick the patches from the posted series, applying the
> review tags as needed, but we need an explicit ack from the mm

Thanks.
I would be good to cherry pick the below one too, as it has also a
'Reviewed-by' tag. I mentioned that it might be easier to miss that
one because it sits after one without 'Reviewed-by' and it seems to
be also applied cleanly:

[net-next,v22,08/14] mm: page_frag: use __alloc_pages() to replace alloc_pages_node()

https://patchwork.kernel.org/project/netdevbpf/patch/20241018105351.1960345-9-linyunsheng@huawei.com/

> maintainer, given the mentioned patches touch mostly such code.

Sorry for missing to cc Andrew and MM ML.
Maybe I should have mentioned that Andrew provided an 'Acked-by' in
patch 2, but it is always safer to double check it.

> 
> I would like to avoid repeating a recent incident of unintentionally
> stepping on other subsystem toes.
> 
> @Andrew: are you ok with the above plan?
> 
> Thank you,
> 
> Paolo
> 
>