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 000A6EE49A0 for ; Wed, 23 Aug 2023 15:31:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 39A5F280084; Wed, 23 Aug 2023 11:31:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2FAAD280080; Wed, 23 Aug 2023 11:31:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 174D5280084; Wed, 23 Aug 2023 11:31:57 -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 02673280080 for ; Wed, 23 Aug 2023 11:31:56 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id BD255A01D1 for ; Wed, 23 Aug 2023 15:31:56 +0000 (UTC) X-FDA: 81155759832.16.3EA3DBE Received: from mail-ej1-f52.google.com (mail-ej1-f52.google.com [209.85.218.52]) by imf07.hostedemail.com (Postfix) with ESMTP id B778940012 for ; Wed, 23 Aug 2023 15:31:54 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=FVFTR6zI; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf07.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.52 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1692804714; 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=BYrf2GpbGdlf7lCTIqcEvi/XXKloMRjnbxsd20jVdbA=; b=2BQu5H1RU1w2KSpaVn+HT8dMq1kq99toRGE54GY5eUiQyipG/U61xGuxQrJg70wB0McIBA kQMa0gq8Pixr/MWBa+UdHjWkOrBo1yHEb02XljzBEqSLRNac1ks3GaouW/wElQWieq9h09 /gQLAuPdSX4oQ24gL5emhApK9LpQKV4= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=FVFTR6zI; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf07.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.52 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1692804714; a=rsa-sha256; cv=none; b=MrtGsSqQJCa+/frEXaQAGdzHIdJKMoiH3ww84pwWgtnGwT9xwijwvmbMCdr328pF768D16 LIseqvoSg8KU4X1nLhfNLM64NKMpqZ1Kd3rGmpKq+UuDgr9hvGPvFxiPaPGiRLyR6kpqgx n6NxgOWcBOP17/FAdU96JqT6pVVoi+Q= Received: by mail-ej1-f52.google.com with SMTP id a640c23a62f3a-99c3c8adb27so748586366b.1 for ; Wed, 23 Aug 2023 08:31:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1692804713; x=1693409513; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=BYrf2GpbGdlf7lCTIqcEvi/XXKloMRjnbxsd20jVdbA=; b=FVFTR6zIZBCtNSVzEXQTnf6c+xo9f1ttBG/opfKcx6YFOgEw5AMtTB/0/nrJw2ZNha FqwIi/uJ0cop/dlfSSI5uiZDqRzj7jHartyg5x6GRdhCxtR3ovWY82hmBTT4UZHN1zxS zq5sFGmV/ZRLssqN3aaHWnVo5weRQ23UijbtBAdwtQb+TkqsRT1OWNolgEpwqeW0JSdo TZiTLxFWAY3nuqKsrZ3ENiSB5/mIKTOdo6a5BTRN9UchCwPawnmGK7g1Uy9NuhqX1mLy AJ9t3SEZz+s1MoExy1IqrMIx236qpmUvHyngxPUkJCKlByF0SGYNtqJh3K4B71uajEhS m5fw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692804713; x=1693409513; h=content-transfer-encoding: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=BYrf2GpbGdlf7lCTIqcEvi/XXKloMRjnbxsd20jVdbA=; b=BLuN/0q/Saz8/Gn2xjCu7/VAmOoRZ0LPtXwVvU6RiDowID5mNMwtmQT40ivqS6HhCb ugoF319cJjAtQ6rJIb4IykeDVmmTzqM3DbeavbXtkDAtvLn7U6ZoNwHWYop5CKZxEXtq Nx35jNSVd5dmqRWOLTo8hYBmJEQAwDrNX+WhIu87dt2/lf8bRuLMwZ+5StS1GLZhDhIX BBtr/reVywAYTlRVdPh5UFtKIYO1BJfjhjADR9U3lhvT8ysCckA6nU/K0AOc75L7Ophp +6Upuix9KFeLMzpAzIYh70cnSfyRN4w0hXU/LdMHLtVIo8SrWRqvSLEju428OEOUZXfj Z7Dw== X-Gm-Message-State: AOJu0YxNT8v6Nkx8s8pfk9g3VciN3TokFcaTbEQouCA5WODIZce19FFV leLpEul1Tv8nJkFq0nygTFlHzXwvSLPV0FiXyXnPYQ== X-Google-Smtp-Source: AGHT+IHhEhVQAm8bllNWnfvIrsbzwpalLHvKYZWfHuErDCbPhelT3wHAcn8MfG+cIeuWsS4XHaFk0nd4eZX2ohR1ycs= X-Received: by 2002:a17:906:31c1:b0:99d:a6b9:fd04 with SMTP id f1-20020a17090631c100b0099da6b9fd04mr9893550ejf.46.1692804713226; Wed, 23 Aug 2023 08:31:53 -0700 (PDT) MIME-Version: 1.0 References: <20230821160849.531668-1-david@redhat.com> <20230821160849.531668-2-david@redhat.com> In-Reply-To: From: Yosry Ahmed Date: Wed, 23 Aug 2023 08:31:16 -0700 Message-ID: Subject: Re: [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org, Andrew Morton , Matthew Wilcox , Peter Xu , Catalin Marinas , Will Deacon , Hugh Dickins , Seth Jennings , Dan Streetman , Vitaly Wool Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: B778940012 X-Stat-Signature: 95tkj5skf9oi94ytpczrihtaeu8rsmw1 X-Rspam-User: X-HE-Tag: 1692804714-351721 X-HE-Meta: U2FsdGVkX1/bC+iL06K13DBOZYbX8lJT8vEWdYzVytq6sfzNJIEcDrLxTVK+f43+orBQiJhm6It6nE5ai6qwvlmBkw+1FwEHm25dY9aeMQfWETkrVvH46GRnjRkW1aDAVDNNyq2L6kkwmCLrIgxMr1zWodSZm1DnmvdOg12Lfn+BksnIP4rem55hiwCRpMGm37HQ25soDcGiOEC38NwXE6bbQwsn12wV7V8N85ze3vwjd3iu2tpODRNIhTtDctCtYo5Llewe7P4s8/em1pCHXIWlRE9qd4IpMyiXlINkhWf4CmOTbdWXBihpq/CiFALdcPyFYDDK1Jtdd9zaCpuA+hNqRqK2gRqrlBP5JNvonLFKZmwL858dbYwcXDQb6XKSdiDESBbxRGjRguFKso11bOozq/V/+emCecx4hC0ndpqVXmC0B6RFN1dFD/CCh+SVP3OZkG+S9iUhB4D6VWa+g418oC3+0yCqAarKeFSY8thXwQ5JOhc0I9QaawKMeXxyY8QhCthE45GJNzFpTRPt8LegmUNnoh1zyg8hzx3IMKcDClTHc1DmwCHdwkjxOKm3uA38r4x+Jkp0HKFoZNPh6WW5Slx+BHuunPXCaeVoGLsG+DrDuRt3BUDJ6LkLDd/rsXLxmNzhD/oZ+lipSogUO41DO1jCOHhsynvViYCv2TQs7TD6a+XAxNqP48ZqFnT4aAQM2dF1GAV4qmoaK/+4Lx5JYECfIenRUEcUYJF1IUZCu+czm+oixIT8qp/SgUIkZpdTDxdEjkRC5BChr2Tjh2r1+8xMjZkI2+A1bQChEK4NX8y2H3qNctI0drD4vjs6K0bp4TaakAcpMMGHSrSvSdSwu13Lz68abJexjkcx+s0KokQzm8giOdj7ZVlqviFXEFbr+1HKHknSub0xNOxeP5AwZup43gKoQIWvkt4lSkIF0wKarcOSr4v4O3ujnNkijyjP6xenvaq2yZ47/5p 3atF2PoR ePHTbwZ4jNxrHrZeGAyQNox7XLQEW5jxsiBP3456m92r+PoYGGjZgx91YT6aw5AaXtv5OwxBV0wB4SbeOuQNdRYkFAdGz+7grBwhH+d3Oc4RYcLWDod/pq3P3P1MDk/8RhDzRohoEJV1e3RlQf1+yd+qwUXpVxCcuDSFMW97GRiMvcJtVa9pKWhkMrZQfn7Z4XvTNRBweCef1i7q2Ibo74MShwY/X2cKRmpDp58xQ//nhx4rf3r8Glg6HWJ6G2UFI5oG0+CUhvWZdUyw8g0322Wn+m5iYEGp64kEX8foeN4ncRnHZn8vru1kBhy4QG9Av6UaT 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: On Wed, Aug 23, 2023 at 8:26=E2=80=AFAM David Hildenbrand wrote: > > On 23.08.23 17:21, Yosry Ahmed wrote: > > On Wed, Aug 23, 2023 at 8:17=E2=80=AFAM David Hildenbrand wrote: > >> > >> On 23.08.23 17:12, Yosry Ahmed wrote: > >>> On Mon, Aug 21, 2023 at 9:09=E2=80=AFAM David Hildenbrand wrote: > >>>> > >>>> Let's stop using page->private on tail pages, making it possible to > >>>> just unconditionally reuse that field in the tail pages of large fol= ios. > >>>> > >>>> The remaining usage of the private field for THP_SWAP is in the THP > >>>> splitting code (mm/huge_memory.c), that we'll handle separately late= r. > >>>> > >>>> Update the THP_SWAP documentation and sanity checks in mm_types.h an= d > >>>> __split_huge_page_tail(). > >>>> > >>>> Signed-off-by: David Hildenbrand > >>> > >>> The mm part looks good to me (with the added fixup): > >>> > >>> Reviewed-by: Yosry Ahmed > >> > >> Thanks! > >> > >>>> /** > >>>> diff --git a/include/linux/swap.h b/include/linux/swap.h > >>>> index bb5adc604144..84fe0e94f5cd 100644 > >>>> --- a/include/linux/swap.h > >>>> +++ b/include/linux/swap.h > >>>> @@ -339,6 +339,15 @@ static inline swp_entry_t folio_swap_entry(stru= ct folio *folio) > >>>> return entry; > >>>> } > >>>> > >>>> +static inline swp_entry_t page_swap_entry(struct page *page) > >>>> +{ > >>>> + struct folio *folio =3D page_folio(page); > >>>> + swp_entry_t entry =3D folio_swap_entry(folio); > >>>> + > >>>> + entry.val +=3D page - &folio->page; > >>>> + return entry; > >>>> +} > >>>> + > >>>> static inline void folio_set_swap_entry(struct folio *folio, swp_= entry_t entry) > >>>> { > >>>> folio->private =3D (void *)entry.val; > >>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c > >>>> index cc2f65f8cc62..c04702ae71d2 100644 > >>>> --- a/mm/huge_memory.c > >>>> +++ b/mm/huge_memory.c > >>>> @@ -2446,18 +2446,15 @@ static void __split_huge_page_tail(struct pa= ge *head, int tail, > >>>> page_tail->index =3D head->index + tail; > >>>> > >>>> /* > >>>> - * page->private should not be set in tail pages with the ex= ception > >>>> - * of swap cache pages that store the swp_entry_t in tail pa= ges. > >>>> - * Fix up and warn once if private is unexpectedly set. > >>>> - * > >>>> - * What of 32-bit systems, on which folio->_pincount overlay= s > >>>> - * head[1].private? No problem: THP_SWAP is not enabled on = 32-bit, and > >>>> - * pincount must be 0 for folio_ref_freeze() to have succeed= ed. > >>>> + * page->private should not be set in tail pages. Fix up and= warn once > >>>> + * if private is unexpectedly set. > >>>> */ > >>>> - if (!folio_test_swapcache(page_folio(head))) { > >>>> - VM_WARN_ON_ONCE_PAGE(page_tail->private !=3D 0, page= _tail); > >>>> + if (unlikely(page_tail->private)) { > >>>> + VM_WARN_ON_ONCE_PAGE(true, page_tail); > >>>> page_tail->private =3D 0; > >>>> } > >>> > >>> Could probably save a couple of lines here: > >>> > >>> if (VM_WARN_ON_ONCE_PAGE(page_tail->private !=3D 0, page_tail)) > >>> > >>> page_tail->private =3D 0; > >>> > >> > >> That would mean that we eventually compile out the runtime check > >> > >> #define VM_WARN_ON_ONCE_PAGE(cond, page) BUILD_BUG_ON_INVALID(cond) > > > > I thought the warning would be compiled out but not the check, my bad. > > I even remembered that VM_WARN_ON_ONCE and friends could/should not be > used in conditionals. > > But we do seem to have two users now: > > $ git grep "if (VM_WARN_ON" > mm/mmap.c: if (VM_WARN_ON_ONCE_MM(vma->vm_end !=3D vmi_end, = mm)) > mm/mmap.c: if (VM_WARN_ON_ONCE_MM(vma->vm_start !=3D vmi_sta= rt, mm)) > > But they only do warning-related action, to dump the stack, the vma, ... > > So if the warnings get compiled out, also all the other stuff gets compil= ed out as well, > which makes sense here. Funny enough, I did the same grep and immediately thought that since we have users of that, then it's okay (i.e the check wouldn't be compiled out). I wasn't thorough enough to actually check what they are doing :) > > -- > Cheers, > > David / dhildenb >