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 E8610C77B61 for ; Fri, 28 Apr 2023 10:52:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 15D236B0071; Fri, 28 Apr 2023 06:52:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0E4CD6B0072; Fri, 28 Apr 2023 06:52:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E9F426B0074; Fri, 28 Apr 2023 06:52:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id D639D6B0071 for ; Fri, 28 Apr 2023 06:52:58 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id AED5D1603C2 for ; Fri, 28 Apr 2023 10:52:58 +0000 (UTC) X-FDA: 80730487236.02.B8F94F4 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf10.hostedemail.com (Postfix) with ESMTP id 27C23C001F for ; Fri, 28 Apr 2023 10:52:55 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=V9SShDZv; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf10.hostedemail.com: domain of toke@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=toke@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1682679176; 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=2zCc3ooe0b9bN/mb4wKrhZshV5sTAbrbDvOeaAxexq0=; b=PXhekGWo8xY/e2Vzg+a8EnaKpCI8Pg+13przKnJmaTDRlNQccdAkNkgnQ+gJB7W6waEPVM BNI9OnYQgD+aQmJLCgijANCGXUZCO7od898ADMr1aEYJrG/8o6mq1mIc619hKPH2VK19q4 C0oxB6GL/r4tkSZaglv1bVA5r8yOSI8= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=V9SShDZv; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf10.hostedemail.com: domain of toke@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=toke@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1682679176; a=rsa-sha256; cv=none; b=I/SQqh9gLbKt+HNU+5dTPZ3IdY434/iB6E9u0oK9GiXgi1EpCWaRIUvzwTIUF/FnSzizK3 TQzt3ns4pOgr/5P5d1R3nJvqq/zAQFcqCmK8qPh6WC8a704/vwYglAWSqMAL7GKlhhmg/U LWY4puxM7wwVQ9u09fEpZd+VEdxPvrA= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1682679175; 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=2zCc3ooe0b9bN/mb4wKrhZshV5sTAbrbDvOeaAxexq0=; b=V9SShDZv7z5UfKEZo2ZwDdKWeu8I0LtJngh/Y72y2zXrs9UjhBV/ku8S3V3l7M0ZVU0hbd /ffOGtoYgOKUIRgAuaTtq+sWlHxDUj4dNFNAPPUBBWwTgxhh+aRE2O24ny/zdMthTglxty 2uAel4XPn4yJpt565gGmwgei1+EeYic= 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-445-a-stqKzhNpy6YYJdXrZqFw-1; Fri, 28 Apr 2023 06:52:54 -0400 X-MC-Unique: a-stqKzhNpy6YYJdXrZqFw-1 Received: by mail-ej1-f70.google.com with SMTP id a640c23a62f3a-94a348facbbso1163377066b.1 for ; Fri, 28 Apr 2023 03:52:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682679173; x=1685271173; 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=P+ULg++VU6Z9Sfko72HfIvUtxTw3hDxjpqo3Ne7UJBw=; b=Y65CAmxesuNZBttDedg+ozoj8ErRXLdx1JPaeW7ooofr3twZv896bcIputKOFwZqu9 bg3/HV7pT6YpchtUZ5/I6qm4+jDGTI9PgN9rQvfztD59f93CnygBepay5SgKZOxeqEPJ WVqm7a2U4iV+dgs1aIwodEIh1HNrq2eQ9fbfTctYJWJBPfX13fy3aWF+E/dFhvYX4SX2 SHNZ/K75U0UOOmByKUdUjGyFwYfMKdjk7lL7ny+eNi2q1cD38RPafCcBPW4UNrsHmzkz Nm7QX6biSWuGC5YLmt8yOopCCRhFp5QEm2i+wPYJbVppK+oJqZ61doIgpxsckl/RJimE G2HQ== X-Gm-Message-State: AC+VfDxeXNvZ0kTWh7U9R9+nK2T6xrJzq5TI4aL6HH6dn9vk4GXUCHxZ gHBwL/ykor9GuEOf32Khit3BtlZrHFbPOVD1FNVHXYNmU5KX2/soBBDhyzT5EfBqvBYjfKYeW7Z LbCFIyvKnNQA= X-Received: by 2002:a17:907:368d:b0:94e:48ac:9a51 with SMTP id bi13-20020a170907368d00b0094e48ac9a51mr4905323ejc.4.1682679172551; Fri, 28 Apr 2023 03:52:52 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4pNYQfKdJa7/R3tHp6KZk6XXZGEyekEMVRJH02AdzCQHAhaOzZ8FY7SeF1FsXK21oUds+pxw== X-Received: by 2002:a17:907:368d:b0:94e:48ac:9a51 with SMTP id bi13-20020a170907368d00b0094e48ac9a51mr4905282ejc.4.1682679171643; Fri, 28 Apr 2023 03:52:51 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id g11-20020a170906394b00b0094f4f2db7e0sm11184180eje.143.2023.04.28.03.52.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 28 Apr 2023 03:52:51 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 7C654ADCA55; Fri, 28 Apr 2023 12:52:50 +0200 (CEST) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= To: Jesper Dangaard Brouer , Ilias Apalodimas , netdev@vger.kernel.org, Eric Dumazet , linux-mm@kvack.org, Mel Gorman Cc: brouer@redhat.com, lorenzo@kernel.org, linyunsheng@huawei.com, bpf@vger.kernel.org, "David S. Miller" , Jakub Kicinski , Paolo Abeni , Andrew Morton , willy@infradead.org Subject: Re: [PATCH RFC net-next/mm V2 1/2] page_pool: Remove workqueue in new shutdown scheme In-Reply-To: <4eab92af-251a-a9aa-e270-179634d0345b@redhat.com> References: <168262348084.2036355.16294550378793036683.stgit@firesoul> <168262351129.2036355.1136491155595493268.stgit@firesoul> <871qk582tn.fsf@toke.dk> <4eab92af-251a-a9aa-e270-179634d0345b@redhat.com> X-Clacks-Overhead: GNU Terry Pratchett Date: Fri, 28 Apr 2023 12:52:50 +0200 Message-ID: <87mt2s6zy5.fsf@toke.dk> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 27C23C001F X-Stat-Signature: na8y4keebuiajimq1f7se78htzaaz6cu X-HE-Tag: 1682679175-25877 X-HE-Meta: U2FsdGVkX19o2pupQMCB3ZVOxjJbiKoo9ebWdvBInK+m5Xx7F9mu29aHdkMx8/a7oWiLkCA4ibmmif0spVn3MgO3deZ9XngLMNBu8SkOtLKeHeUMjHDb4eD+inoL/EB8lcGS/vNi54PhznjDy0X/8jA0TEwLOCvaJMPNvSe/J/K/eZH6P3V5XU4ZGYT6KiKVOtV5a1YnXK3G0/KmMUqj/17GlYHjoAELhIDPjc6GH1eYweRye5DpcphLlRaQT1zMBFxgTOmzaTho9MCTFvsHLJbmI38Si9+wn5e1Fo98IO8H4YnVZ4ghLzNcQEjfTldS7pRgxybWQVQjMZa8NUTrxcvFENiwPdn5uYSajayvUwJU/N0m6H7AY+j5dAt7eOIh2Zz5z8eeQNcy4iAu3GLe7fZTsDJY79w42O16yoyjc4M1LYWjS9xZGfBGnP3s48/FQfaYoxhOq3gitUv8POZJF7ZgumqI9aZ9/wYy8jMIB+4roMFdFgoH/2vLMqxuXVJRQKkCAp+icRSduEFT4+X3kqRpW2m4xKNzhK8t/HqESwCcOqNW4Wa2ZMR6glZZXZaB5tncfL2D5925pHRfnmyjiA6EMyS8df1UoTaKxguPVbUQEndqV/HgpcbXHx/Lb92J/4ACLo/ivpyHYc8bKhWeHXht7kPCwbVq8KrTyouUt546fXEumlEZP6HJ7BP2XdUcghlnrwIf+HhgXMPuhGv6hBDHFkdO7Fy606E08J5iOhqgG71WWhPkZkCJ+ScjYSxCK372e1aDA3HLOKRGHQSw62d5fupFQQuM5dGQIf6NIr0huFMTh5xFlWqRcXHQlqnWT+4QfnL58LSzZzsiWqNJLvQIyKpp8+/0GUqfuXjQfPo9ce3/gcBjhwcdDT1R/ozU2TtBUbTj0ziB2r9BpnWOJYRiMgbdeaNeO3U6A2qViuAa3otrW7/RUGlyCfFtiuiYiLKU1aG5DXlhWeU0md8 c9mdQJsa R4CGVb90nKUseoXcCNfLqRgjSLE9cdAVVEXXA2XwgfSEFNb4oZtqYfOHBaf83Z5RS79UBl6LpMTYBaGRD4DLCr5hc04hIIhE58IGg8xcAxvMdfLYJ4LuNLdPeUYLnSE3PH2fnWhj9AhVRhDP0tN9QHSyolnuV82rH/0mC5+IglZu7iaoK3AJZbar3RoUtcZ9+B6MzBubtCnulbQsE+mdimSPlZCBwfZRUwVs4nDzdY8B6eXA0FkipegG8IymEtNhs+HAfz3kkR3KZ1YslayZYALlSUC+4WBBrVBRcGasTo+/a+TxPCjXW7Yx82vFaR6ZSUS0XvHhC0DT98TQTTHXa5ROfKbulp6rnJcvUclzadN/R/jndrlS5gjHx093y8hJDyszjlZtWhEqaoiBDdnaM0ER3y+wcP9zsrkqDXOkw+DyC8weuLUJv7LdsBVSfgQFBu7BdcxAddYb8yTGfWQ7haMpTKwPjy05SH4ll0exAWgRa00kBgkZ/YH5BZlaXSF94x+ZxsicTv2Om5OeBMcHTqYKJsk+3r3xXnIRP 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: Jesper Dangaard Brouer writes: > On 27/04/2023 22.53, Toke H=C3=B8iland-J=C3=B8rgensen wrote: >>> +noinline >>> static void page_pool_empty_ring(struct page_pool *pool) >>> { >>> =09struct page *page; >>> @@ -796,39 +828,29 @@ static void page_pool_scrub(struct page_pool *poo= l) >>> =09page_pool_empty_ring(pool); >>> } >> So this is not in the diff context, but page_pool_empty_ring() does >> this: >>=20 >> static void page_pool_empty_ring(struct page_pool *pool) >> { >> =09struct page *page; >>=20 >> =09/* Empty recycle ring */ >> =09while ((page =3D ptr_ring_consume_bh(&pool->ring))) { >> =09=09/* Verify the refcnt invariant of cached pages */ >> =09=09if (!(page_ref_count(page) =3D=3D 1)) >> =09=09=09pr_crit("%s() page_pool refcnt %d violation\n", >> =09=09=09=09__func__, page_ref_count(page)); >>=20 >> =09=09page_pool_return_page(pool, page); >> =09} >> } >>=20 >> ...and with this patch, that page_pool_return_page() call will now free >> the pool memory entirely when the last page is returned. When it does >> this, the condition in the while loop will still execute afterwards; it >> would return false, but if the pool was freed, it's now referencing >> freed memory when trying to read from pool->ring. > > Yes, that sounds like a problem. > >> So I think page_pool_empty_ring needs to either pull out all the pages >> in the ring to an on-stack buffer before calling page_pool_return_page() >> on them, or there needs to be some other way to break the loop early. > > Let me address this one first, I'll get back to the other in another > reply. The usual/idiom way of doing this is to have a next pointer that > is populated inside the loop before freeing the object. > It should look like this (only compile tested): > > static void page_pool_empty_ring(struct page_pool *pool) > { > =09struct page *page, *next; > > =09next =3D ptr_ring_consume_bh(&pool->ring); > > =09/* Empty recycle ring */ > =09while (next) { > =09=09page =3D next; > =09=09next =3D ptr_ring_consume_bh(&pool->ring); > > =09=09/* Verify the refcnt invariant of cached pages */ > =09=09if (!(page_ref_count(page) =3D=3D 1)) > =09=09=09pr_crit("%s() page_pool refcnt %d violation\n", > =09=09=09=09__func__, page_ref_count(page)); > > =09=09page_pool_return_page(pool, page); > =09} > } Yup, that works! >> There are a couple of other places where page_pool_return_page() is >> called in a loop where the loop variable lives inside struct page_pool, >> so we need to be absolutely sure they will never be called in the >> shutdown stage, or they'll have to be fixed as well. > > The other loops are okay, but I spotted another problem in=20 > __page_pool_put_page() in "Fallback/non-XDP mode", but that is fixable. Alright, great! -Toke