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 77B53CF6491 for ; Mon, 30 Sep 2024 08:38:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 055A28001B; Mon, 30 Sep 2024 04:38:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 003B280017; Mon, 30 Sep 2024 04:38:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DE7138001B; Mon, 30 Sep 2024 04:38:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id C1FAF80017 for ; Mon, 30 Sep 2024 04:38:50 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 78B3A81C7C for ; Mon, 30 Sep 2024 08:38:50 +0000 (UTC) X-FDA: 82620754020.14.ABCE960 Received: from szxga07-in.huawei.com (szxga07-in.huawei.com [45.249.212.35]) by imf13.hostedemail.com (Postfix) with ESMTP id 66D0620004 for ; Mon, 30 Sep 2024 08:38:47 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf13.hostedemail.com: domain of linyunsheng@huawei.com designates 45.249.212.35 as permitted sender) smtp.mailfrom=linyunsheng@huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727685436; a=rsa-sha256; cv=none; b=F35pN70tTPhJC/cZGrpB5QxSgnSiJbxG1QtM33Sy18MPWzQ3wFMkVovGbcZn8f1mWWNXg8 vva7PbijWJIF9DZA0x7/8zVS+hFAIy8Z3d1NRTX1F6u+acHzQ5gwnsM5SEEYdHIACLw5JL yCVN8aAsHv2nbL9EaRDdDRP9hQgQBt4= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf13.hostedemail.com: domain of linyunsheng@huawei.com designates 45.249.212.35 as permitted sender) smtp.mailfrom=linyunsheng@huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727685436; 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; bh=vb2zM4IqM30wmhgEjy5HJ1zOzTDut+wSQnIln9R+sY0=; b=kZDYWk0ZNssy4RSBowBCyYgBa3Jk4xiJf0i6BRU6uQ9Kxg/7Tw45Q/xsGFastmd7XfcXPz zxTOHwfpHBiStkd/s9xsZw0yIWyuPHSCA8iIKIRFk3ZM88vFFRUf1wVVP23Z3A3LRuI6i8 cq5zrGoXz26gTpYxCdIze7hsb+uzHaQ= Received: from mail.maildlp.com (unknown [172.19.162.112]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4XHDwY3YV3z1SC0g; Mon, 30 Sep 2024 16:37:49 +0800 (CST) Received: from dggpemf200006.china.huawei.com (unknown [7.185.36.61]) by mail.maildlp.com (Postfix) with ESMTPS id B68231400CB; Mon, 30 Sep 2024 16:38:43 +0800 (CST) Received: from [10.67.120.129] (10.67.120.129) by dggpemf200006.china.huawei.com (7.185.36.61) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Mon, 30 Sep 2024 16:38:43 +0800 Message-ID: Date: Mon, 30 Sep 2024 16:38:42 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net v2 2/2] page_pool: fix IOMMU crash when driver has already unbound To: Ilias Apalodimas CC: Mina Almasry , , , , , , , Robin Murphy , Alexander Duyck , IOMMU , Wei Fang , Shenwei Wang , Clark Wang , Eric Dumazet , Tony Nguyen , Przemek Kitszel , Alexander Lobakin , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend , Saeed Mahameed , Leon Romanovsky , Tariq Toukan , Felix Fietkau , Lorenzo Bianconi , Ryder Lee , Shayne Chen , Sean Wang , Kalle Valo , Matthias Brugger , AngeloGioacchino Del Regno , Andrew Morton , , , , , , , , , , References: <20240925075707.3970187-1-linyunsheng@huawei.com> <20240925075707.3970187-3-linyunsheng@huawei.com> <842c8cc6-f716-437a-bc98-70bc26d6fd38@huawei.com> <0ef315df-e8e9-41e8-9ba8-dcb69492c616@huawei.com> <934d601f-be43-4e04-b126-dc86890a4bfa@huawei.com> Content-Language: en-US From: Yunsheng Lin In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.120.129] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To dggpemf200006.china.huawei.com (7.185.36.61) X-Rspamd-Queue-Id: 66D0620004 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: zjszb4s4yt8gmf784uz31enyct4ipz3s X-HE-Tag: 1727685527-582252 X-HE-Meta: U2FsdGVkX1+jLs1On9QR05sGXsP9pn8/QA8WuyzNiscRmIDR0RxQ47+/npXGYvJLlUgRaUYDFCVr9/dCGKW3NlKcuP7lR3NrrCb4B8xxaxKZVcpYdmfWsk6wUNL/4p4yuP78+TddLqUuVk1zM1TWm/Bk/gXK3NdElQAx7E00HDCzr4pF13gAEosYo8/HnNnVxJljLJM1rzmtZFuobOtm7O7UPp7VBNFVV3DByLP4ctCTF7MJQnbgrNiytYvu8Q4UGFgGLs142eFYA6V3XmX9c+4H4KnVYgOIfgKlhpVdU++Ka6+GlaR2c/CgRlpobiPIf5KMqW5B79g8eBAR89aWVhBu+rhPf6iKvCHNkrQVUJjU7p8rsPnZsRXjVeQupYsKQvg/PSPZQrg0HMDgcstF0toVP0NUcMHpCLPiDpOlQO1328y1c5/EdvQD4qZXEscyxM5fz74iaQC/uJA+SsUkGBbRn8XkT2siYuPWoLLbVEPkn0EN50Yy3S+gZxBpwBr3EsU+q1rm++Q3Iys6dq5QWy98G4CuqvxqTDZl4KNr7oW4MH62Z9F6OOI9AEYNkSHk29c30srhiSaVWvbFrYpsN5icBqhj8NFWAZfVwRUH/eCiQL2h5vsbxNCvE/X83TOgXy3aMOU2iA3+QRJMdqHmv8qH1q4O0SDrXusdnojEoX4nQQwLQVu1yL+RbIBD4MbrRNzFOf5YSECkjB37+g88kWDeX5sG7tXMCRgw4pmSoMyRbh7eX+Tm48VWiPMWbZ2TIbax+MxS7wKJYsUmbAbbHojt7cj7E9KEUofMZ7FfWYrUGrj8bO9Pzt986JpWQOt1WT0lxY7oJQ5S7z01pMTQ6eOkqIJOn/ztlIvCsisTaNZS3TvEqcNO+f2i2DFAaM1taPjk4lz7xjsHLsPjbEJ+/tGJnZx9rNP6kGrSGCYIVhYZbWU0GOixThntIelzLx7gRQg+DdpEwsmMvG5wCsV ARoOs+Nu tZSFz5qWUFRBl8fMhcT9yrd5IBLNGYPAWnh6KhzB6S4JCAPBOhl6/aSZqayL1prkw8vOUbJ6meQi5oN3qQz4N6Z0aeThsBKCMF4s8xhbfPJgAg2pXD6Ti6v/yznhlIfy2XRWp1XvWwEmSyzmoHT3xeTWCfW4XpSJwunouFlw8+KvJQpDw3cbAkCa41IygbAdcWilCqV5dv1+SPu4hPY+EpMnDaWVeNTiVcOEkfFetxRoUaYedDn98cKOdjEAmwJqJZT3Vq/C1LJO1DcA3NrOpUch4nmWbcnK6xl0+iZ/ukWgCkZQxwUiFP/S4I+HZvbNKlMeVGwc0WsfafAlMKF/d8lw660wDmeHtx2O6eSOyy6ruZEbuyh8/Cl9NBA== 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 2024/9/30 16:09, Ilias Apalodimas wrote: > On Sun, 29 Sept 2024 at 05:44, Yunsheng Lin wrote: >> >> On 2024/9/28 15:34, Ilias Apalodimas wrote: >> >> ... >> >>> >>> Yes, that wasn't very clear indeed, apologies for any confusion. I was >>> trying to ask on a linked list that only lives in struct page_pool. >>> But I now realize this was a bad idea since the lookup would be way >>> slower. >>> >>>> If I understand question correctly, the single/doubly linked list >>>> is more costly than array as the page_pool case as my understanding. >>>> >>>> For single linked list, it doesn't allow deleting a specific entry but >>>> only support deleting the first entry and all the entries. It does support >>>> lockless operation using llist, but have limitation as below: >>>> https://elixir.bootlin.com/linux/v6.7-rc8/source/include/linux/llist.h#L13 >>>> >>>> For doubly linked list, it needs two pointer to support deleting a specific >>>> entry and it does not support lockless operation. >>> >>> I didn't look at the patch too carefully at first. Looking a bit >>> closer now, the array is indeed better, since the lookup is faster. >>> You just need the stored index in struct page to find the page we need >>> to unmap. Do you remember if we can reduce the atomic pp_ref_count to >>> 32bits? If so we can reuse that space for the index. Looking at it >> >> For 64 bits system, yes, we can reuse that. >> But for 32 bits system, we may have only 16 bits for each of them, and it >> seems that there is no atomic operation for variable that is less than 32 >> bits. >> >>> requires a bit more work in netmem, but that's mostly swapping all the >>> atomic64 calls to atomic ones. >>> >>>> >>>> For pool->items, as the alloc side is protected by NAPI context, and the >>>> free side use item->pp_idx to ensure there is only one producer for each >>>> item, which means for each item in pool->items, there is only one consumer >>>> and one producer, which seems much like the case when the page is not >>>> recyclable in __page_pool_put_page, we don't need a lock protection when >>>> calling page_pool_return_page(), the 'struct page' is also one consumer >>>> and one producer as the pool->items[item->pp_idx] does: >>>> https://elixir.bootlin.com/linux/v6.7-rc8/source/net/core/page_pool.c#L645 >>>> >>>> We only need a lock protection when page_pool_destroy() is called to >>>> check if there is inflight page to be unmapped as a consumer, and the >>>> __page_pool_put_page() may also called to unmapped the inflight page as >>>> another consumer, >>> >>> Thanks for the explanation. On the locking side, page_pool_destroy is >>> called once from the driver and then it's either the workqueue for >>> inflight packets or an SKB that got freed and tried to recycle right? >>> But do we still need to do all the unmapping etc from the delayed >>> work? Since the new function will unmap all packets in >>> page_pool_destroy, we can just skip unmapping when the delayed work >>> runs >> >> Yes, the pool->dma_map is clear in page_pool_item_uninit() after it does >> the unmapping for all inflight pages with the protection of pool->destroy_lock, >> so that the unmapping is skipped in page_pool_return_page() when those inflight >> pages are returned back to page_pool. > > Ah yes, the entire destruction path is protected which seems correct. > Instead of that WARN_ONCE in page_pool_item_uninit() can we instead > check the number of inflight packets vs what we just unmapped? IOW > check 'mask' against what page_pool_inflight() gives you and warn if > those aren't equal. Yes, it seems it is quite normal to trigger the warning from testing, it makes sense to check it against page_pool_inflight() to catch some bug of tracking/calculating inflight pages. > > > Thanks > /Ilias >> >>>