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 AA112C28B20 for ; Wed, 2 Apr 2025 11:10:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4E22B280006; Wed, 2 Apr 2025 07:10:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 468A9280001; Wed, 2 Apr 2025 07:10:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 30ABB280006; Wed, 2 Apr 2025 07:10:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 0F33E280001 for ; Wed, 2 Apr 2025 07:10:32 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 064AEC1764 for ; Wed, 2 Apr 2025 11:10:33 +0000 (UTC) X-FDA: 83288835546.01.89B5ECC Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf08.hostedemail.com (Postfix) with ESMTP id 7CCAD160009 for ; Wed, 2 Apr 2025 11:10:30 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=LQK710F0; spf=pass (imf08.hostedemail.com: domain of toke@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=toke@redhat.com; dmarc=pass (policy=quarantine) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1743592230; 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=lNhhyFMaBe+ZjtDnKN1fww+rY/V3YA2QsQwiTfsJL0w=; b=JAH+BebiJRyivQpcGcmMbaa0VmIEToEpDU1fHoVaUkA4e5SixDp1JBWNineMK5BygCyhaf t8n4BeK0HaFYHZNzzE3ypMspf2yRwk6NAMixreicfVaZQXLWUaeuC1U11wXfr8MjESr/x3 HeBRUYkniz1nhw2KcWi7T1a05ov56IQ= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=LQK710F0; spf=pass (imf08.hostedemail.com: domain of toke@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=toke@redhat.com; dmarc=pass (policy=quarantine) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1743592230; a=rsa-sha256; cv=none; b=s8ljUIixMD04blm3UIx7wJDYhDUUVpVlFzbzkIrtCgFh/BbIcOdRSoReK1sBlBNevrkTZ/ eF3+s7LQtfNxn08LQncNeuoo3F4PiqaWBjuAGhcKHWRZLlBYaxjmYqTBrWVH/Wl3ksveyg DTOGqzTjessmd+WPjj3e8PcrNaBsRzY= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1743592229; h=from:from: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; bh=lNhhyFMaBe+ZjtDnKN1fww+rY/V3YA2QsQwiTfsJL0w=; b=LQK710F0sOaCHNmPMhlLnS8Nf9293GKJgOL9gcWXvd2c1xaZkOpI87RjDJhRkOeJda9XkF Nrlv1qp+N6O9SId4ZXhn5dH4gqxedlGTn8Rfx8CvmqbIn8vVKap0+zDL+VmKHm+Ly81u86 roAZPiqHVlpJ43sHSPgtTG3kafTJ2bY= Received: from mail-ej1-f70.google.com (mail-ej1-f70.google.com [209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-594-Wjny_drHOr2pGqYWgs-REw-1; Wed, 02 Apr 2025 07:10:28 -0400 X-MC-Unique: Wjny_drHOr2pGqYWgs-REw-1 X-Mimecast-MFC-AGG-ID: Wjny_drHOr2pGqYWgs-REw_1743592227 Received: by mail-ej1-f70.google.com with SMTP id a640c23a62f3a-ac737973d06so390683766b.2 for ; Wed, 02 Apr 2025 04:10:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1743592227; x=1744197027; h=content-transfer-encoding:mime-version:message-id:date:references :in-reply-to:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=4vWh3ZSkArty4GE704JEhJE0l6rYapz0ZdylM6ZVJFg=; b=otjwG3eVIPGWXqba6lSeBhv5tioLtD2uCQVoPw+zM78AivrlATDHsUPkWvZASYMxv0 8unynDIoJVj0URwCtPhUOV1x7FxIFwtEv9fLWIYn+GXXs4232eIgNvszCnXw6blT4fBT meIynPfPeo9h0zj/1hopeIfrhoEB/QHRX/Ud3aeWqWtO9hUHFmNuvF4UNmNhhXWDrm/C qnEWZRDgTkce+R1kTZtyzLPtZXNjmNjYmtf3ErZv2lVCyxm4GoXU9Q0ZVbZBO4ppylLR JJp8s+C/TzHD7kIa+1Pilp/l/7aDK21JSRjWw3kd79wQnuDmTJukYU5lEggPRADys+a0 PpPQ== X-Forwarded-Encrypted: i=1; AJvYcCXnLRam6ufZpahUrSph0VaPTYb6/Um+uNVATNetm1LAmSQtXmadugsdU9OcOU53gQaqpEIocu35bg==@kvack.org X-Gm-Message-State: AOJu0YxVJO5oEiQXhvsHViy7nIdBLaNFeIXdLI1gyuqR2ugqjecj1luW Ca6SFL9dFEAoQx/C16YKh81QAF1tCsMRNM/inaXFDENm/cdWso1XiIQgdog71OqYCEhtxzN57WM ojwTKjNViDxdt6snJukAtF1GSPdF4nZxvJPbYRN0bSwVTzgwf X-Gm-Gg: ASbGnctIY/VKoCdHCrdlLI5ul2RUYL2VzB/dfpPS+VrIAXs9jS7tyFbQLEln1aEutc7 yTFQ7dmJ/OA+c0bbez/atf9b881kACijCylTfTt4OuGE4VO4KzGkZ8XXFe8IiyNaNXMsUTQmTRi z4PZ65djDhRZfVsb5XNdeuQHkHpMH98Q3WeiirhH4zxz5PmCgYsq0H+TFi9Oh1s377xarBklLGE lW+jg8el+xec94/phCN41MsOv8TAaTGHTIOjEMjlneDzpF9VytT5QZozbwYS1nr4P5kdY8BssZi BtxCpEVdzg9y X-Received: by 2002:a17:907:3f26:b0:ac3:154e:1391 with SMTP id a640c23a62f3a-ac73897af75mr1468889366b.10.1743592227084; Wed, 02 Apr 2025 04:10:27 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFgMJBQJGMVcHMqaXRV768YtbruiOfUMHIbIN54f/Fgpq3xvdwdRNsKIiLjNa/scM2YmTFJ4Q== X-Received: by 2002:a17:907:3f26:b0:ac3:154e:1391 with SMTP id a640c23a62f3a-ac73897af75mr1468885666b.10.1743592226596; Wed, 02 Apr 2025 04:10:26 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ac71962177fsm891693366b.115.2025.04.02.04.10.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 02 Apr 2025 04:10:25 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id C8AE018FD3A9; Wed, 02 Apr 2025 13:10:24 +0200 (CEST) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= To: Pavel Begunkov , "David S. Miller" , Jakub Kicinski , Jesper Dangaard Brouer , Saeed Mahameed , Leon Romanovsky , Tariq Toukan , Andrew Lunn , Eric Dumazet , Paolo Abeni , Ilias Apalodimas , Simon Horman , Andrew Morton , Mina Almasry , Yonglong Liu , Yunsheng Lin , Matthew Wilcox Cc: netdev@vger.kernel.org, bpf@vger.kernel.org, linux-rdma@vger.kernel.org, linux-mm@kvack.org, Qiuling Ren , Yuying Ma Subject: Re: [PATCH net-next v6 2/2] page_pool: Track DMA-mapped pages and unmap them when destroying the pool In-Reply-To: <3e0eb1fa-b501-4573-be9f-3d8e52593f75@gmail.com> References: <20250401-page-pool-track-dma-v6-0-8b83474870d4@redhat.com> <20250401-page-pool-track-dma-v6-2-8b83474870d4@redhat.com> <3e0eb1fa-b501-4573-be9f-3d8e52593f75@gmail.com> X-Clacks-Overhead: GNU Terry Pratchett Date: Wed, 02 Apr 2025 13:10:24 +0200 Message-ID: <87jz82n7j3.fsf@toke.dk> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: qMQLixM35y9XmZhAQZRKcExQLE-ZYxaf3ikiNpBU5v0_1743592227 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 7CCAD160009 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: nwsg1odizibggca33ogy4r47f5hkwj69 X-HE-Tag: 1743592230-672780 X-HE-Meta: U2FsdGVkX18mV8m/gzmpRIt5eMj6C/4VxRVsctavi1SR+3pGFABfWDDReYpRhQByWPLn47ziDlYCA2kzMRO6KRPKdeASnoBjtWVXwz73NmT/ozAA0J2zSjRojbJj06EqYN0K8QiF6xOW7klcPlBm49rrsKVWjIA5Q6Pg7eLiIKs4nhACBa4aUQKSBlawuEvuJc+um4AP3sr+PuCYSGpISYhm+bWtVOcRYQwlhLdioqScetckJwVl0Dl/OMIktGddZLrKIK3frzC7I5pqwCRIPasW9Mpbn8dXZjIlk6uTTjP1cHnBRhPZsNew/Ls+14AUbb4lyb0v6Vw3eQ5lUW03VagxV3BfOO0dFLQL8AlDi34PocY3tV2ledZP6jZ1y0dBq38tfhG1r/1b5ou3XoXx6uA8Vm8EWe9pJApR8vTQry5Q31mg+OdtZub0qCGcr0W0S6NzqE76ZVwHui16SnKpC1lNzekjaZJP4HSVisYhRvpkmYYWoN5qTZv3GYgQ0XSeqcmGMVNj9oRG+sJEgSaBkj09YP8I2EaMGui4sOAvm5hjWvaYzXCY8OiwxE3Az6inFmU1X2AfFxke3Nb4EpjJgz1lug2Qul1V3kyiFZr2CDY/l5y9BErGLY9GczfE3LaM5ohZ3K7kuRxrnSmwOHLqvOdgSfZXbAP7rTPq8X8bMZxGTIgU2X6qYTWizZcwrWewpc7zcC/vZBvkbYwFVhcHqvA02yzLRQpglW7B9ESgfmSbH1sZ4EpjVNFc0UtDxvYnYU+sf9EOGz09GojEEnskt7PoL83TM4JdJZq1y96FQofZTJUmO3YP92T1q+wGtY/RbHQc/3Qomxq4sc84zdqQi0IV5zz/E67JmmxXLWyyr+CxJk4DsB4FUtPNo55F1qoZqd6V4CY1lcf7m4nffxhYnJNK4FiEW5W1AWUzbdYArO2eYCs/u6tCg+beSzaHYwdG6GbeNrjwvcM0GQycCMm MoFOcPDP kp7Xqq4omFI313DcmWZmDoF9BiIAtxNMuz9ec/Qq/9eVmnYnZ5iTxej5lY/d/U/JZjo6DxkdQTY4tGNWHqpqvE/bgU4qO69W1gMMNMVPCu/+486QkB88fOnqrxJGQUZFSFRISjliXNcPNmGX/U0hO3bOQ8tQQ9iGYahc37VNGKil1uHnlk6BIF0N8JM/mzVhJWtTn1tcbShuqbXqE3amIRC6q6s3n818MN0HUlFeDEHibW0WkFKyOxaV8S2eBJIDSEhdnTs0a/LO6HhiVPNPX5DEcTix/pTanTf681qT9J7vrK2Vy7vMDXwruWfwnwk7ZGcXPFQf3DeNyvKnZ26n/s9zGvVIJp+ipGCivLhJtqPqE+F8H67yD+GRZLK+8qD+nfaGoIqorHSiBlhRyJP7bKMP+mTWRFDo8p1ucrUa3ntvR2YUxyE0pLgZ6OgbL7afz+2GhRnIjaXZ7j/5zc/DXwJfDbsyDtnzPM1afeWeGdL7ck3oQfhDoDHYdRXom3Aw/QBKjKTDMjZ9BPs/rwuhLUll0/gTacLgdXl3bEPAvrflcm8fZWRB3XwXnBCGP1/0pwxSux3eIQDd5acqpWC6JY2TFxlQyvrfF3+PRqfjXgxTTmGHgjmvVoaEBwazqhiCljIjOPWQvFvmO/+mj6xjidj0+tw== 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: Pavel Begunkov writes: > On 4/1/25 10:27, Toke H=C3=B8iland-J=C3=B8rgensen wrote: > ... >> Reported-by: Yonglong Liu >> Closes: https://lore.kernel.org/r/8743264a-9700-4227-a556-5f931c720211@h= uawei.com >> Fixes: ff7d6b27f894 ("page_pool: refurbish version of page_pool code") >> Suggested-by: Mina Almasry >> Reviewed-by: Mina Almasry >> Reviewed-by: Jesper Dangaard Brouer >> Tested-by: Jesper Dangaard Brouer >> Tested-by: Qiuling Ren >> Tested-by: Yuying Ma >> Tested-by: Yonglong Liu >> Acked-by: Jesper Dangaard Brouer >> Signed-off-by: Toke H=C3=B8iland-J=C3=B8rgensen > > I haven't looked into the bit carving, but the rest looks > good to me. A few nits below, > > ... >> diff --git a/net/core/page_pool.c b/net/core/page_pool.c >> index 7745ad924ae2d801580a6760eba9393e1cf67b01..52b5ddab7ecb405066fd55b8= d61abfd4186b9dcf 100644 >> --- a/net/core/page_pool.c >> +++ b/net/core/page_pool.c >> @@ -227,6 +227,8 @@ static int page_pool_init(struct page_pool *pool, >> =09=09=09return -EINVAL; >> =20 >> =09=09pool->dma_map =3D true; >> + >> +=09=09xa_init_flags(&pool->dma_mapped, XA_FLAGS_ALLOC1); > > nit: might be better to init/destroy unconditionally, it doesn't > allocate any memory. Hmm, yeah, suppose both could work; I do think this makes it clearer that it's tied to DMA mapping, but I won't insist. Not sure it's worth respinning just for this, though (see below). >> =09} >> =20 >> =09if (pool->slow.flags & PP_FLAG_DMA_SYNC_DEV) { >> @@ -276,9 +278,6 @@ static int page_pool_init(struct page_pool *pool, >> =09/* Driver calling page_pool_create() also call page_pool_destroy() = */ >> =09refcount_set(&pool->user_cnt, 1); >> =20 >> -=09if (pool->dma_map) >> -=09=09get_device(pool->p.dev); >> - >> =09if (pool->slow.flags & PP_FLAG_ALLOW_UNREADABLE_NETMEM) { >> =09=09netdev_assert_locked(pool->slow.netdev); >> =09=09rxq =3D __netif_get_rx_queue(pool->slow.netdev, >> @@ -322,7 +321,7 @@ static void page_pool_uninit(struct page_pool *pool) >> =09ptr_ring_cleanup(&pool->ring, NULL); >> =20 >> =09if (pool->dma_map) >> -=09=09put_device(pool->p.dev); >> +=09=09xa_destroy(&pool->dma_mapped); >> =20 >> #ifdef CONFIG_PAGE_POOL_STATS >> =09if (!pool->system) >> @@ -463,13 +462,21 @@ page_pool_dma_sync_for_device(const struct page_po= ol *pool, >> =09=09=09 netmem_ref netmem, >> =09=09=09 u32 dma_sync_size) >> { >> -=09if (pool->dma_sync && dma_dev_need_sync(pool->p.dev)) >> -=09=09__page_pool_dma_sync_for_device(pool, netmem, dma_sync_size); >> +=09if (READ_ONCE(pool->dma_sync) && dma_dev_need_sync(pool->p.dev)) { >> +=09=09rcu_read_lock(); >> +=09=09/* re-check under rcu_read_lock() to sync with page_pool_scrub() = */ >> +=09=09if (READ_ONCE(pool->dma_sync)) >> +=09=09=09__page_pool_dma_sync_for_device(pool, netmem, >> +=09=09=09=09=09=09=09dma_sync_size); >> +=09=09rcu_read_unlock(); >> +=09} >> } >> =20 >> -static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem= ) >> +static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem= , gfp_t gfp) >> { >> =09dma_addr_t dma; >> +=09int err; >> +=09u32 id; >> =20 >> =09/* Setup DMA mapping: use 'struct page' area for storing DMA-addr >> =09 * since dma_addr_t can be either 32 or 64 bits and does not always= fit >> @@ -483,15 +490,28 @@ static bool page_pool_dma_map(struct page_pool *po= ol, netmem_ref netmem) >> =09if (dma_mapping_error(pool->p.dev, dma)) >> =09=09return false; >> =20 >> -=09if (page_pool_set_dma_addr_netmem(netmem, dma)) >> +=09if (in_softirq()) >> +=09=09err =3D xa_alloc(&pool->dma_mapped, &id, netmem_to_page(netmem), >> +=09=09=09 PP_DMA_INDEX_LIMIT, gfp); >> +=09else >> +=09=09err =3D xa_alloc_bh(&pool->dma_mapped, &id, netmem_to_page(netmem= ), >> +=09=09=09=09 PP_DMA_INDEX_LIMIT, gfp); > > Is it an optimisation? bh disable should be reentrable and could > just be xa_alloc_bh(). Yeah, it's an optimisation. We do the same thing in page_pool_recycle_in_ring(), so I just kept the same pattern. > KERN_{NOTICE,INFO} Maybe? Erm? Was this supposed to be part of the comment below? >> +=09if (err) { >> +=09=09WARN_ONCE(1, "couldn't track DMA mapping, please report to netdev= @"); > > That can happen with enough memory pressure, I don't think > it should be a warning. Maybe some pr_info? So my reasoning here was that this code is only called in the alloc path, so if we're under memory pressure, the page allocation itself should fail before the xarray alloc does. And if it doesn't (i.e., if the use of xarray itself causes allocation failures), we really want to know about it so we can change things. Hence the loud warning. @maintainers, given the comments above I'm not going to respin for this unless you tell me to, but let me know! :) -Toke