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 90F15CDB465 for ; Thu, 19 Oct 2023 16:20:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2D5178009F; Thu, 19 Oct 2023 12:20:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 25D4E80090; Thu, 19 Oct 2023 12:20:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 124CF8009F; Thu, 19 Oct 2023 12:20:48 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id F09DA80090 for ; Thu, 19 Oct 2023 12:20:47 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 691F0B5BF5 for ; Thu, 19 Oct 2023 16:20:47 +0000 (UTC) X-FDA: 81362724534.20.2C4682F Received: from mail-ej1-f47.google.com (mail-ej1-f47.google.com [209.85.218.47]) by imf08.hostedemail.com (Postfix) with ESMTP id 8A5E716001E for ; Thu, 19 Oct 2023 16:20:45 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=yTHJ2al3; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf08.hostedemail.com: domain of souravpanda@google.com designates 209.85.218.47 as permitted sender) smtp.mailfrom=souravpanda@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1697732445; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=aB4z4zOp1dSTncT5D3KrAGwlZG+RAcM1Y8Kk9oa5psg=; b=RngkX3anAgNfcySqzwvTrhsTO9A9fD7Y2Dq1DHVU8GiqRFmKx3waHvryNAD3ptvNibuP6i MjG15bQMWP1xAf+EDPKYoiVJPbwC0ank0pdmP1EA/1cYu5felZnh4/VCdi6ZhJ/zi3PSNG i1EHTOHnQRRofzH3yNhnU44D+kNQ+sQ= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=yTHJ2al3; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf08.hostedemail.com: domain of souravpanda@google.com designates 209.85.218.47 as permitted sender) smtp.mailfrom=souravpanda@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1697732445; a=rsa-sha256; cv=none; b=tMF36X6upB+gtRDHwm/wXL4rRcaSKFX7DpZYVi8MhqYOqgV/fD7bc+dBTep6pOFc0gXe6R OLkVt1HVDlnnmKcIE503h95yL742qChzOmTJV7tivXivKvXv+/rjAIJjOjfDCsRAfto4X2 PQlDgGvOPcdN3fQiGmD2O5HAZzffApQ= Received: by mail-ej1-f47.google.com with SMTP id a640c23a62f3a-99de884ad25so1313256766b.3 for ; Thu, 19 Oct 2023 09:20:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1697732444; x=1698337244; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=aB4z4zOp1dSTncT5D3KrAGwlZG+RAcM1Y8Kk9oa5psg=; b=yTHJ2al3xJyPlaOZIJ9K00h6xBZN9+K61E/6g4tpm2AUKkvwDF7XhgDvWBSPdZdH2Z xXB9uYaQvkAptHbGqRKrxz0DLYuZEfrYyQznVUrEDEDeHwTmmXg5X0xYo/kviFDjW2Iz utZhnU/2P6gqXXBtE5tZ+eZ4yyTsA20nR5MjE2E5f6NTUiFvV7ls1PomiTZMTqwx9Dp3 x2tTX1QlWV6POpZJzlVWD/2P70Y5NnCWvb449XaHtN5jnrFe2bL/gMJ20tKxv6DlU/zW /MVj25MgIuQEgI2VET7Bq4ntDT9cn89WKQYLfMBl+kdNjpbs6IQXOM7H7wY+dfQLOH3P ltXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697732444; x=1698337244; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=aB4z4zOp1dSTncT5D3KrAGwlZG+RAcM1Y8Kk9oa5psg=; b=IvrFOtWt1qJ+8jV9Lg+BgnIx9C1ErXurOiI49Psql2z7InsAA0LWxxaWAFEYhbK/JN HrQPGXreH2szfnA2b8qwDsB2mNXfoIXEjddXkC03iqF3DUvrpO2qaY2qdMMDRIFDi3A5 8abVTi5Odnuty0Nq0kYa4xrQLhLLx6R3lzSaAQQcL7kKm6+wq2m4GzGHWUkqfIf4oHnJ lltaaZFKKvYxAyRWDYD0wI6oChkWtSNYbdM/BTB/e/t/pyLOJ5HXrI6/iDhEYfg/CgIq 1lqwA0tGyNARU+/qxET7iNIZbiUxbv2QgWxgPcmZ8+14hEMZYOrAdLD7asR7SWNOj1pB ssHA== X-Gm-Message-State: AOJu0YyUdl6Jas7J0qd1jbpbsiieO1aqxp6LXiJkGGZHbSHYTWz3M2bS wOAGWQJXDnlgDuw4AXgFH8ZAoReRyqlKAJ610P5tzQ== X-Google-Smtp-Source: AGHT+IG7YH/Y0x3KOlEz2W0OFQjXDKGTXJIkX3uDtzjnPumUTREk+J4WE5mSMMxBN9FjT9EjIsyC7FHiFydRcHIY14M= X-Received: by 2002:a17:907:8687:b0:9c5:2806:72e9 with SMTP id qa7-20020a170907868700b009c5280672e9mr2012952ejc.34.1697732443733; Thu, 19 Oct 2023 09:20:43 -0700 (PDT) MIME-Version: 1.0 References: <20231018005548.3505662-1-souravpanda@google.com> <20231018005548.3505662-2-souravpanda@google.com> In-Reply-To: From: Sourav Panda Date: Thu, 19 Oct 2023 09:20:32 -0700 Message-ID: Subject: Re: [PATCH v2 1/1] mm: report per-page metadata information To: Matthew Wilcox Cc: corbet@lwn.net, gregkh@linuxfoundation.org, rafael@kernel.org, akpm@linux-foundation.org, mike.kravetz@oracle.com, muchun.song@linux.dev, rppt@kernel.org, david@redhat.com, rdunlap@infradead.org, chenlinxuan@uniontech.com, yang.yang29@zte.com.cn, tomas.mudrunka@gmail.com, bhelgaas@google.com, ivan@cloudflare.com, pasha.tatashin@soleen.com, yosryahmed@google.com, hannes@cmpxchg.org, shakeelb@google.com, kirill.shutemov@linux.intel.com, wangkefeng.wang@huawei.com, adobriyan@gmail.com, vbabka@suse.cz, Liam.Howlett@oracle.com, surenb@google.com, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-doc@vger.kernel.org, linux-mm@kvack.org Content-Type: multipart/alternative; boundary="0000000000003d8d630608142366" X-Rspamd-Queue-Id: 8A5E716001E X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: dio5zzrdbor3m94erac3d37q98irjh7f X-HE-Tag: 1697732445-332084 X-HE-Meta: U2FsdGVkX19Zy7TSD2k465RX2+G67kj5EiJTAvH66Su43B0jZXbhrtZZSSWem6G5Tbjj34sKpTCizWQxqWIo2rd7gAnzOhYB1Ds7VO5q6xvt4JpuwlPCDRHC1aW3tz3MY7utK8lrXP2K8yM5aOLpIAapZMCjSviDlw4YUKtJ665TQIiHBJSg3a7WBcN4cdm6PtW1o59rMpknASS7vf7YJJPZm8ae9Lr5KvIjTCYzi7TR/EwMVK9cTq6HjvlswJTRQRRA+83bRids6Ve9xzcpkN7P2NupQGdxWzyGz4Fv48uGM+cYFLydbYHo/08o3kUv9A4CNyRyeIw7j9H02ti5W1n/Ukg40DNYhTFVEX6XBGarcRoZQJSMItsfDUQVGnP/VMuH71rcR1Ti2T+vVrDq1rQtoXvApHOPGX8Rum63YE4WMf157DXuhG0IkH5HYDJNKCSiSB8cQfwCLg9zxVhoTVv9SPmHRuFqXJm+1XQBIOSrfnapoTHVCNBOtd9+jJFC0h55/YFFubLa6mMBQrpajMnKU19TeJDFtMULomcChx7Cza5Id91OdIT+mRQDFdZlU6Ag39S9q+WH2T7Cpyi2TjFcjsjDDEWSB1hGnhUXf5joznHNS00sDJJZE0RdjXl4YvD9uRndFGLMCOYN8QVpqh1YfBWy+Z9atqP1C1boaqTOcM9HPwLI/3LhHsJSHpP/DpYqRW6Yv6SXyQNrxYn0BrMk7hnyRxj9WKCbb87HFieqyf++oEWLY5H9lHGEAAej4Upk4KgvrVhIAnzde/QKxcZqfC440/nUqQSkt04cvMavnWnT0DgotuG0UGPHEEef5W0ZRzyL0ZdPx6izlhg+WAajw/Qb/5n5fjo4OOnoXlz5aHpO5tzuqOYvW1lTacg+CxECtq7aX2J98/FQpFkMIZUs0Y282CbYcAfquZKUCrDaDzq1uXXsYiF052mUgPImHcZ1nuO001dM3vA/EPU jl16rBw9 tUhUeeZ2cxxR6uO1eXjc8dLbLOzOISK2YgIYuljU7DIleS7QA/oHoLsRMKYUOG+IHOyhl8wNTIt2VVchrYBoLooxqZBvFlgrvgFg2E0BXvKQdNtKitqvJSfUqtFPpOZEKuIvqlF43NbmN/ano9DWVMpyo5wG7zl7MBqdzcUWaT0r0KDyCTU3mr00mWZ7izJyqB+BYxTx6oHxoIya3Cwyl4xg/Etfaf3QHr9fxL4/+1k0RwkCkp2KOmA1MK9F0qqMB8gTdGtJ4lFcTs52gPBWqR1SBNeYoiz8+SoSIKtwPoljt9E9f1jXqvDJU8BGy8n2o692JupH1EgGhYHpFRVmOX5JIGFslOSqiYSZ+ypZdLcVvdfr0F9HvhonZEnd5guX53MHnOuipcjwwXO2qb/wN8lFtvDjv7tjky81wy5MDBCNDwN1d05FOm9WspqxIFHH2WGwMy2M3FHSebbg= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000018, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: --0000000000003d8d630608142366 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Oct 18, 2023 at 7:08=E2=80=AFAM Matthew Wilcox wrote: > On Tue, Oct 17, 2023 at 05:55:48PM -0700, Sourav Panda wrote: > > + mod_node_early_perpage_metadata(pgdat->node_id, > > + PAGE_ALIGN(size) >> > PAGE_SHIFT); > > What a curious way of writing DIV_ROUND_UP(size, PAGE_SIZE) > (throughout). A quick grep shows about 230 DIV_ROUND_UP and 110 of what > you wrote, so it's not something you invented, but DIV_ROUND_UP is > clearer. > > > @@ -303,18 +308,25 @@ static int __meminit > init_section_page_ext(unsigned long pfn, int nid) > > > > static void free_page_ext(void *addr) > > { > > + size_t table_size; > > + struct page *page; > > + > > + table_size =3D page_ext_size * PAGES_PER_SECTION; > > + > > if (is_vmalloc_addr(addr)) { > > + page =3D vmalloc_to_page(addr); > > vfree(addr); > > } else { > > - struct page *page =3D virt_to_page(addr); > > - size_t table_size; > > - > > - table_size =3D page_ext_size * PAGES_PER_SECTION; > > + page =3D virt_to_page(addr); > > > > BUG_ON(PageReserved(page)); > > kmemleak_free(addr); > > free_pages_exact(addr, table_size); > > } > > + > > + __mod_node_page_state(page_pgdat(page), NR_PAGE_METADATA, > > + -1L * (PAGE_ALIGN(table_size) >> > PAGE_SHIFT)); > > This troubles me. We're freeing the memory and then dereferencing > the page that was freed. I know that struct pages don't go away when > they're freed, and they don't change which node they're allocated to, > but it feels wrong. I'd be happier if the page_pgdat() were extracted > prior to freeing the memory. > Thank you Matthew Wilcox for pointing this out. I will extract page_pgdat() prior to freeing the memory in v3. With regards, Sourav Panda --0000000000003d8d630608142366 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable



On Wed, Oct 18, 2023 at 7:08=E2=80=AFAM Matthew Wilcox <willy@infradead.org&g= t; wrote:
On Tue= , Oct 17, 2023 at 05:55:48PM -0700, Sourav Panda wrote:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0mod_node_early_perpag= e_metadata(pgdat->node_id,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0PAGE_ALIGN(size) >> PAGE_SHIFT);

What a curious way of writing DIV_ROUND_UP(size, PAGE_SIZE)
(throughout).=C2=A0 A quick grep shows about 230 DIV_ROUND_UP and 110 of wh= at
you wrote, so it's not something you invented, but DIV_ROUND_UP is
clearer.

> @@ -303,18 +308,25 @@ static int __meminit init_section_page_ext(unsig= ned long pfn, int nid)
>=C2=A0
>=C2=A0 static void free_page_ext(void *addr)
>=C2=A0 {
> +=C2=A0 =C2=A0 =C2=A0size_t table_size;
> +=C2=A0 =C2=A0 =C2=A0struct page *page;
> +
> +=C2=A0 =C2=A0 =C2=A0table_size =3D page_ext_size * PAGES_PER_SECTION;=
> +
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (is_vmalloc_addr(addr)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0page =3D vmalloc_to_p= age(addr);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0vfree(addr);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0struct page *page =3D= virt_to_page(addr);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0size_t table_size; > -
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0table_size =3D page_e= xt_size * PAGES_PER_SECTION;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0page =3D virt_to_page= (addr);
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0BUG_ON(PageReser= ved(page));
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0kmemleak_free(ad= dr);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0free_pages_exact= (addr, table_size);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0}
> +
> +=C2=A0 =C2=A0 =C2=A0__mod_node_page_state(page_pgdat(page), NR_PAGE_M= ETADATA,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0-1L * (PAGE_ALIGN(table_size) >> PAGE_SHI= FT));

This troubles me.=C2=A0 We're freeing the memory and then dereferencing=
the page that was freed.=C2=A0 I know that struct pages don't go away w= hen
they're freed, and they don't change which node they're allocat= ed to,
but it feels wrong.=C2=A0 I'd be happier if the page_pgdat() were extra= cted
prior to freeing the memory.


=
Thank you=C2=A0Matthew Wilcox for pointing this out. I will extract pa= ge_pgdat() prior to freeing the memory in v3.

With= =C2=A0regards,
Sourav Panda
--0000000000003d8d630608142366--