From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A3F3CC433ED for ; Mon, 19 Apr 2021 18:41:12 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id E77096127C for ; Mon, 19 Apr 2021 18:41:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E77096127C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 62E106B0036; Mon, 19 Apr 2021 14:41:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5DDAE6B006E; Mon, 19 Apr 2021 14:41:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3E22F6B0070; Mon, 19 Apr 2021 14:41:11 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0167.hostedemail.com [216.40.44.167]) by kanga.kvack.org (Postfix) with ESMTP id 1C9F16B0036 for ; Mon, 19 Apr 2021 14:41:11 -0400 (EDT) Received: from smtpin12.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id C25D8180AD830 for ; Mon, 19 Apr 2021 18:41:10 +0000 (UTC) X-FDA: 78049983900.12.858FF04 Received: from mail-ej1-f53.google.com (mail-ej1-f53.google.com [209.85.218.53]) by imf22.hostedemail.com (Postfix) with ESMTP id 44CE0C0007CB for ; Mon, 19 Apr 2021 18:41:05 +0000 (UTC) Received: by mail-ej1-f53.google.com with SMTP id r12so54506854ejr.5 for ; Mon, 19 Apr 2021 11:41:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=3TXXauaxjaFJkryZmKe4pKXsqt2c2W4efv7xXXt6kDg=; b=K9U3Wsn/ZMp1qcVYmH8K49eGtKmxmMFJSG/7TwpdB++O51Bvwx+tGQzCMd2HWtCCCj XkCNqHebmllrUDYH703DvaCBFIWgSQv6LE4Cbcf2HP99jSESzpqew5uPUVaK2069tqYj tZYbGjtxJ2VsQp+eM/Xqdg32vwrRUqYHp41rtvTo2tv49YUNJF97ZV4gpkqaEDa1JKZL Z6YVtkJSH7bYpmUle6fdm8FVq4JLc6chNiBFVgFpOGNZ+D9JgQikxIFy4njTKnuu92mH pS/DzlQqFeg0jYG6YLidrAmH1WKQgCm7dvMcSnuxGhhjUn59EuYG3vW+H+LmrqEFQqyA 83cA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=3TXXauaxjaFJkryZmKe4pKXsqt2c2W4efv7xXXt6kDg=; b=O/gNdWnGeMcUnbIZluhIRMkWR3FJ5V8TbPyTGNxVXbxmDn7w76fcmsGOo/P8Nt1oIq wuhbUWxe9/hGPE4IBDHNkXf5JmpUV0Y7mYP9d86ZeO0BclY5rf+TmDUOxevD5oAy0gnk OBxqT+ghCH/wE4ydsqpL/FyAX7OzbbNYV2nKxa2xwrutDi8xGxOZNXgtCUCyER9c2FDi T0+u+9qNgh4wMhXMsvkGfnJunUtMn7LpVBOHhP2Da1Lep7EG9DxZlQWy5x0ZPOIZobmh w3VDf5yrPWi1whyML43uZug/4oq6mtZL5rOVviZ2f7+x3cQjg6H4ZCO1guE9iuRKIJPh Pdvg== X-Gm-Message-State: AOAM532UU25ygnQpKPGdG87T0Hm0kvhyXDTOiZY94gabdy0fvAwFM1Pt OVUxGkWc8pUE0OKUdYjPn62Gmm4wMoCXS7Jy X-Google-Smtp-Source: ABdhPJycOdUR+CwuNkAlnUJFFot68MXH4HjBcF7ZhZklGQLGYR6WexKascVTMUkGkR7LHMZEQK+/Ig== X-Received: by 2002:a17:906:6896:: with SMTP id n22mr23676314ejr.316.1618857669049; Mon, 19 Apr 2021 11:41:09 -0700 (PDT) Received: from apalos.home (ppp-94-65-92-88.home.otenet.gr. [94.65.92.88]) by smtp.gmail.com with ESMTPSA id l6sm5233811ejc.92.2021.04.19.11.41.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Apr 2021 11:41:08 -0700 (PDT) Date: Mon, 19 Apr 2021 21:41:02 +0300 From: Ilias Apalodimas To: Shakeel Butt Cc: Jesper Dangaard Brouer , Matthew Wilcox , Matteo Croce , netdev , Linux MM , Ayush Sawal , Vinay Kumar Yadav , Rohit Maheshwari , "David S. Miller" , Jakub Kicinski , Thomas Petazzoni , Marcin Wojtas , Russell King , Mirko Lindner , Stephen Hemminger , Tariq Toukan , Jesper Dangaard Brouer , Alexei Starovoitov , Daniel Borkmann , John Fastabend , Boris Pismenny , Arnd Bergmann , Andrew Morton , "Peter Zijlstra (Intel)" , Vlastimil Babka , Yu Zhao , Will Deacon , Michel Lespinasse , Fenghua Yu , Roman Gushchin , Hugh Dickins , Peter Xu , Jason Gunthorpe , Guoqing Jiang , Jonathan Lemon , Alexander Lobakin , Cong Wang , wenxu , Kevin Hao , Aleksandr Nogikh , Jakub Sitnicki , Marco Elver , Willem de Bruijn , Miaohe Lin , Yunsheng Lin , Guillaume Nault , LKML , linux-rdma@vger.kernel.org, bpf , Eric Dumazet , David Ahern , Lorenzo Bianconi , Saeed Mahameed , Andrew Lunn , Paolo Abeni Subject: Re: [PATCH net-next v3 2/5] mm: add a signature in struct page Message-ID: References: <20210410154824.GZ2531743@casper.infradead.org> <20210414214132.74f721dd@carbon> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Stat-Signature: kx3e1piq3jaqypad8fi1i1prmjke9fd9 X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 44CE0C0007CB Received-SPF: none (linaro.org>: No applicable sender policy available) receiver=imf22; identity=mailfrom; envelope-from=""; helo=mail-ej1-f53.google.com; client-ip=209.85.218.53 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1618857665-217765 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Apr 19, 2021 at 09:21:55AM -0700, Shakeel Butt wrote: > On Mon, Apr 19, 2021 at 8:43 AM Ilias Apalodimas > wrote: > > > [...] > > > Pages mapped into the userspace have their refcnt elevated, so the > > > page_ref_count() check by the drivers indicates to not reuse such > > > pages. > > > > > > > When tcp_zerocopy_receive() is invoked it will call tcp_zerocopy_vm_insert_batch() > > which will end up doing a get_page(). > > What you are saying is that once the zerocopy is done though, skb_release_data() > > won't be called, but instead put_page() will be? If that's the case then we are > > indeed leaking DMA mappings and memory. That sounds weird though, since the > > refcnt will be one in that case (zerocopy will do +1/-1 once it's done), so who > > eventually frees the page? > > If kfree_skb() (or any wrapper that calls skb_release_data()) is called > > eventually, we'll end up properly recycling the page into our pool. > > > > From what I understand (Eric, please correct me if I'm wrong) for > simple cases there are 3 page references taken. One by the driver, > second by skb and third by page table. > > In tcp_zerocopy_receive(), tcp_zerocopy_vm_insert_batch() gets one > page ref through insert_page_into_pte_locked(). However before > returning from tcp_zerocopy_receive(), the skb references are dropped > through tcp_recv_skb(). So, whenever the user unmaps the page and > drops the page ref only then that page can be reused by the driver. > > In my understanding, for zerocopy rx the skb_release_data() is called > on the pages while they are still mapped into the userspace. So, > skb_release_data() might not be the right place to recycle the page > for zerocopy. The email chain at [1] has some discussion on how to > bundle the recycling of pages with their lifetime. > > [1] https://lore.kernel.org/linux-mm/20210316013003.25271-1-arjunroy.kdev@gmail.com/ Ah right, you mentioned the same email before and I completely forgot about it! In the past we had thoughts of 'stealing' the page on put_page instead of skb_release_data(). We were afraid that this would cause a measurable performance hit, so we tried to limit it within the skb lifecycle. However I don't think this will be a problem. Assuming we are right here and skb_release_data() is called while the userspace holds an extra reference from the mapping here's what will happen: skb_release_data() -> skb_free_head() -> page_pool_return_skb_page() -> set_page_private() -> xdp_return_skb_frame() -> __xdp_return() -> page_pool_put_full_page() -> page_pool_put_page() -> __page_pool_put_page() When we call __page_pool_put_page(), the refcnt will be != 1 (because a user mapping is still active), so we won't try to recycle it. Instead we'll remove the DMA mappings and decrease the refcnt. So although the recycling won't 'work', nothing bad will happen (famous last words). In any case, I'll double check with the test you pointed out before v4. Thanks! /Ilias