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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id DEFD1C10DCE for ; Sat, 9 Dec 2023 01:38:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 632326B0072; Fri, 8 Dec 2023 20:38:22 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5E25E6B0075; Fri, 8 Dec 2023 20:38:22 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4D0F46B007D; Fri, 8 Dec 2023 20:38:22 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 3F02F6B0072 for ; Fri, 8 Dec 2023 20:38:22 -0500 (EST) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 1FD80A0132 for ; Sat, 9 Dec 2023 01:38:22 +0000 (UTC) X-FDA: 81545569644.06.30AAC32 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf19.hostedemail.com (Postfix) with ESMTP id 4D7BA1A0013 for ; Sat, 9 Dec 2023 01:38:20 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=QTBMK6I2; spf=pass (imf19.hostedemail.com: domain of kuba@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=kuba@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702085900; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=GNbG837A+/EFmfV2yLi5mug4xPqMqIRIA0UXl8YVfcw=; b=55PSsKpGH0sLS37DyAOjQYxSZhBcC+OiwW30d7m0tUzrxFijApgI1a3w1/HI15XxEUIWo3 D515uAFNLu9o5RcTfFBkZFzxrdJ7fUMaMjBJrSCUA9lFJcg2OGg4y475Z4AQ1gaAcFTh3W nLM0vO/dudsUOuqDR8guiZopjpctEm8= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=QTBMK6I2; spf=pass (imf19.hostedemail.com: domain of kuba@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=kuba@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702085900; a=rsa-sha256; cv=none; b=f+iHA4heERxPX0ykYMKIkPiIai/v1z6J26dcRsTtubKgKlq+C9wPS5hg6EDtOxS574fI31 E6eEpvVkMatNaCm6G6q/hbcj7YsdO38Zd7dNoPLv2cLvHvaX9Vefd4EQ5MotabROShesLE eVgeQmGjU74CjtA6MoGz8F0HOMAK9Qs= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by ams.source.kernel.org (Postfix) with ESMTP id 95A38B830C7; Sat, 9 Dec 2023 01:38:18 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 76566C433C8; Sat, 9 Dec 2023 01:38:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1702085897; bh=Zb4W5U81zr9iBTuDWuPtvrYzyuDkADvK3X/33n9Wz2U=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=QTBMK6I2Qo22ZCH4loWVlOpI6nj2/x0LsQ/pcbnevM1+AQvfqXACDdbvX4osdUCzx JKRJMKgSWFI7z5CPzgFiTcEB+Um7vJIzzQMwMpyzj+pzDT9jw/wY8a3rcvbBz0Dpk0 NCDc6IiBbqCEwlO+FdCsdj/ZaqOK5WlWXU8RkDel7GR/G4+8qbtMXHTJlyzaEzfrwS Uzqo9uilwoD9cE1aQUn7gvSkO2aWgTocbuo+Jf7C7DwchMqJ+0Nei/dpq+cps4rRk7 RilhL24T1iSm38pYbdRtRq6sg4pIVu3uHTZQiDwNngI1a3nhHE2vwUrDVX9WtbgZxH qf6GK95BckxvQ== Date: Fri, 8 Dec 2023 17:38:16 -0800 From: Jakub Kicinski To: Liang Chen Cc: davem@davemloft.net, edumazet@google.com, pabeni@redhat.com, hawk@kernel.org, ilias.apalodimas@linaro.org, linyunsheng@huawei.com, netdev@vger.kernel.org, linux-mm@kvack.org, jasowang@redhat.com Subject: Re: [PATCH net-next v7 1/4] page_pool: transition to reference count management after page draining Message-ID: <20231208173816.2f32ad0f@kernel.org> In-Reply-To: <20231206105419.27952-2-liangchen.linux@gmail.com> References: <20231206105419.27952-1-liangchen.linux@gmail.com> <20231206105419.27952-2-liangchen.linux@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 4D7BA1A0013 X-Rspam-User: X-Stat-Signature: 3g4hw3kwfq8q5et984q11zzjj4epke59 X-Rspamd-Server: rspam01 X-HE-Tag: 1702085900-619638 X-HE-Meta: U2FsdGVkX1+tLU1FtV4seCMAqxK6rRGSc+I2zPlNjRBNbGPv6nj764RxRSmNHyGSqGmpFD/M60iOlbHx4kPVp8XQODzWfnTg/e6dfnx2dCg3X/yIODBIgS7c0nYRviY1m6lZ9l/nfWxfg5z3hAQ6jZHQwlM1ideWR8LmvoFtZa1Ho3+c5VmZEb0XnhGpxFM1g3ry2neLCOPVW3UZQlADuF7PtNt/Nfpze5H/qrCdsEVGRphGGlFMeXvFvmmd/dJ4ohLHpRoqwMhEU7l41Hq2l2ue7WrCZhHUTOGkygc23yPCgK2o2hGvxJM7B0z+ouzeEEaacZNM9hg8X5lwtiQ/oo8rIvgCAIwRkTlVSAkWVrnlT/D/Du0qjoYad1/+AzFw8ZNv9t1LrW3vSnc1x+pfyS4C30FaVAHWlbDNZLjokWrBn/rZdH9fz4KgSzJ9bZdPcMpOqXO9vAqTS9r/sSssbNJCGttLXxRPshMbIcHuhhR8hojc+vamIiRcQo9a0sWYbREGQphm5BWIri2JIQNWR6i6idChwLGwxH/nhyvYTDRiEpJKKT1wSPKvFze6USIeXO38k/NAZQWgjQldpAbCdWtph+s38CzYQP6huzCfw2FPoP2foFI563UZjBl/EYRnmc1vjYol5GXOxNr5xxWBMdaNjxXFzTjSKu60s6c2YgW1EVLBjrCxq1NvGuZuT2/ifGc+G1xVqWxKSVH9eZKr9Tap5jAVoEKSdzHCXDmjHXovC0cS0J1Yk6XzBBT+vzMugb1BS4NsGSaHLR+f7ogwdAE48xpf7UheybGhQH9t7tVy9sv94sFkHJ+ZRt+UhpyRIyohQbIlK+qdfVFKhMjWCpMsP5MczcKmXkbsxZtQOpH5ws7EUlOdhevKWHT4xFujUyx9TmXj7yoFfiJjMR+I3q8iz/LhpwVK4SqbF9wJ2AnYPjmxcnNkSC+bLTy9jEKwd9DOZhMMEg1eG3Oc2M8 luh6L0rs tGdB34Y4Ysr7CehEDvOY7KATGbnn0kCLweIRlws/0JO4pB1yHZ7HEeiD3gmArRJ4IRpzCJ7a6evfbF0aqZs++8CsH0+OFQIJwWK4kehGTr2p1e7AnHIED5J96EGm67MD5n+c10+HESRU+3c+UvFjImyK/q2AzFRYf+vhk1i9vRLC2HnTob1XhoNX0Vwoc6vbr23L5H5TDC+ung5QCE9JbQzJWAeVK+B06MAHy+YjLJ2JO0BL2K1Tw4hBsc1R/UxaYOdP84E3QDbHyUICmSe6XXEBzBTjg0mOqZjdInp/37DIn9B1OoaZazmkN3g== 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: List-Subscribe: List-Unsubscribe: On Wed, 6 Dec 2023 18:54:16 +0800 Liang Chen wrote: > -/* pp_frag_count represents the number of writers who can update the page > +/* pp_ref_count represents the number of writers who can update the page > * either by updating skb->data or via DMA mappings for the device. > * We can't rely on the page refcnt for that as we don't know who might be > * holding page references and we can't reliably destroy or sync DMA mappings > * of the fragments. > * > - * When pp_frag_count reaches 0 we can either recycle the page if the page > + * pp_ref_count initially corresponds to the number of fragments. However, > + * when multiple users start to reference a single fragment, for example in > + * skb_try_coalesce, the pp_ref_count will become greater than the number of > + * fragments. > + * > + * When pp_ref_count reaches 0 we can either recycle the page if the page > * refcnt is 1 or return it back to the memory allocator and destroy any > * mappings we have. > */ Sorry to nit pick but I think this whole doc has to be rewritten completely. It does state the most important thing which is that the caller must have just allocated the page. How about: /** * page_pool_fragment_page() - split a fresh page into fragments * @.. fill these in * * pp_ref_count represents the number of outstanding references * to the page, which will be freed using page_pool APIs (rather * than page allocator APIs like put_page()). Such references are * usually held by page_pool-aware objects like skbs marked for * page pool recycling. * * This helper allows the caller to take (set) multiple references * to a freshly allocated page. The page must be freshly allocated * (have a pp_ref_count of 1). This is commonly done by drivers * and "fragment allocators" to save atomic operations - either * when they know upfront how many references they will need; or * to take MAX references and return the unused ones with a single * atomic dec(), instead of performing multiple atomic inc() * operations. */ I think that's more informative at this stage of evolution of the page pool API, when most users aren't experts on internals. But feel free to disagree.. > static inline void page_pool_fragment_page(struct page *page, long nr) > { > - atomic_long_set(&page->pp_frag_count, nr); > + atomic_long_set(&page->pp_ref_count, nr); > } The code itself and rest of the patches LGTM, although it would be great to get ACKs from pp maintainers..