On 2024/5/10 0:22, Mat Martineau wrote: > On Wed, 8 May 2024, Yunsheng Lin wrote: > >> 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@xxxxxxxxx> >> Signed-off-by: Yunsheng Lin <linyunsheng@xxxxxxxxxx> >> --- >> .../chelsio/inline_crypto/chtls/chtls.h | 3 - >> .../chelsio/inline_crypto/chtls/chtls_io.c | 100 ++++--------- >> .../chelsio/inline_crypto/chtls/chtls_main.c | 3 - >> drivers/net/tun.c | 28 ++-- >> include/linux/sched.h | 4 +- >> include/net/sock.h | 14 +- >> kernel/exit.c | 3 +- >> kernel/fork.c | 3 +- >> net/core/skbuff.c | 32 ++-- >> net/core/skmsg.c | 22 +-- >> net/core/sock.c | 46 ++++-- >> net/ipv4/ip_output.c | 33 +++-- >> net/ipv4/tcp.c | 35 ++--- >> net/ipv4/tcp_output.c | 28 ++-- >> net/ipv6/ip6_output.c | 33 +++-- >> net/kcm/kcmsock.c | 30 ++-- >> net/mptcp/protocol.c | 70 +++++---- >> net/sched/em_meta.c | 2 +- >> net/tls/tls_device.c | 139 ++++++++++-------- >> 19 files changed, 331 insertions(+), 297 deletions(-) >> > > <snip> > >> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c >> index bb8f96f2b86f..ab844011d442 100644 >> --- a/net/mptcp/protocol.c >> +++ b/net/mptcp/protocol.c >> @@ -960,17 +960,18 @@ 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 >> + * - there is space available for the current page >> + * - the data frag tail matches the current page and offset >> * - the data frag end sequence number matches the current write seq >> */ >> static bool mptcp_frag_can_collapse_to(const struct mptcp_sock *msk, >> - const struct page_frag *pfrag, >> + const struct page *page, >> + const unsigned int offset, >> + const unsigned int size, > > Hi Yunsheng - > > Why add the 'size' parameter here? It's checked to be a nonzero value, but it can only be 0 if page is also NULL. In this case "page == df->page" will be false, so the function will return false even without checking 'size'. Is it possible that the pfrag->page is also NULL, which may cause mptcp_frag_can_collapse_to() to return true? I just found out that the 'size' is not set to zero when return NULL for the implementation of probe API for the current version. Perhaps it makes more sense to expect the API caller to make sure the the returned 'page' not being NULL before using the 'offset', 'size' and 'va', like below: df && page && page == df->page
