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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 89448C4332F for ; Fri, 15 Oct 2021 01:37:49 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 26E6E61090 for ; Fri, 15 Oct 2021 01:37:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 26E6E61090 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id AC2806B006C; Thu, 14 Oct 2021 21:37:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A7085900002; Thu, 14 Oct 2021 21:37:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 985E06B0072; Thu, 14 Oct 2021 21:37:48 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0233.hostedemail.com [216.40.44.233]) by kanga.kvack.org (Postfix) with ESMTP id 8A1DB6B006C for ; Thu, 14 Oct 2021 21:37:48 -0400 (EDT) Received: from smtpin16.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 4A3788249980 for ; Fri, 15 Oct 2021 01:37:48 +0000 (UTC) X-FDA: 78696960216.16.EB01611 Received: from mail-pj1-f41.google.com (mail-pj1-f41.google.com [209.85.216.41]) by imf28.hostedemail.com (Postfix) with ESMTP id 3F8E3900009E for ; Fri, 15 Oct 2021 01:37:47 +0000 (UTC) Received: by mail-pj1-f41.google.com with SMTP id d13-20020a17090ad3cd00b0019e746f7bd4so8266962pjw.0 for ; Thu, 14 Oct 2021 18:37:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Nr8k1aUv3z2OzOm3G6XuPV/NnMghC3CxLAXi8H1jgq0=; b=Mq60g49KQfqJSx9Hlnpx1LkISuHFFWCkdhMNhbH31PDqxasC1fBD6r9vXBbknkaWMK lChJJPt+7b3pndrbO9E/vbjDaPud20IbnvRAMOogvsJsZT2U7U2B08GDOjq5mJAz//YT pUFXQlTk6FHl/Y5XqZwpAwYXkmA/nd/FXzsy+ADUyTODVVWD0me/6KKjEDCOhGFa4tU4 wSxvbpsQXT+KAhdAkd80WrKMMrzTxWLBZMYNVG9REGY3Ujp+K9XNl6UD/fgSRvtPtXtz 0QqqOunHYakkVaxUm+vSHdYXntEMMCYCrb+aUsaBUqYAJXyC9rdXR8U+kphiap+Pl4pP hPXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Nr8k1aUv3z2OzOm3G6XuPV/NnMghC3CxLAXi8H1jgq0=; b=C8h/m69IbnORO471CVJGWGv1r1hYYrQwwZmEfZAn1lSJ1qIHHaAHqJ/EIMmfMlt+AW oN1OZh3YltP5QV5ODRD+Fcc6vOvuUeWd6rvcZCKTLqwxYJK9wDtnhwJJmj64V0u0Xqlh OIkG1qp/3fO+zQQCZZbk5sU3bnvkhoIPJkHigIPryJDkIgJVNaLt+quWEVgTy+wsIspF smfEQkCE7KlylOpHKmbeBgdw73R1LtXQk8dK9zY+I6A6pom7FDOvn/76bSCyymrDxmW/ BREt+pyE71Gxjpfp1JxW4OUspNZ8qxVJx2A6oMMFE8FyMIX2wnxo2zLBLO1uraDunbTH bIXA== X-Gm-Message-State: AOAM531TgjMKoUS9Gya9/2TXciAX1/LYNAqNAaTRb7i4ivAUdSty9ZsM EHlOQba4Zg5jTxxp56NFNeF3/qEEI3EbojGgV355EQ== X-Google-Smtp-Source: ABdhPJzJeyDQ3E7K9800YlGpr55I5n0doDZS1y4ODUC2mgk0GCjS+vEI9q19Hu7ebbpj4/v0RR4BsGE6NqO67YdMecY= X-Received: by 2002:a17:902:ab50:b0:13f:4c70:9322 with SMTP id ij16-20020a170902ab5000b0013f4c709322mr8238204plb.89.1634261866111; Thu, 14 Oct 2021 18:37:46 -0700 (PDT) MIME-Version: 1.0 References: <20211014153928.16805-1-alex.sierra@amd.com> <20211014153928.16805-3-alex.sierra@amd.com> <20211014170634.GV2744544@nvidia.com> <20211014230606.GZ2744544@nvidia.com> In-Reply-To: <20211014230606.GZ2744544@nvidia.com> From: Dan Williams Date: Thu, 14 Oct 2021 18:37:35 -0700 Message-ID: Subject: Re: [PATCH v1 2/2] mm: remove extra ZONE_DEVICE struct page refcount To: Jason Gunthorpe Cc: Matthew Wilcox , Alex Sierra , Andrew Morton , "Kuehling, Felix" , Linux MM , Ralph Campbell , linux-ext4 , linux-xfs , amd-gfx list , Maling list - DRI developers , Christoph Hellwig , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , Alistair Popple , Vishal Verma , Dave Jiang , Linux NVDIMM Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: mnc3adwh1eii3bx65j3a1d71c5r9nbio Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=intel-com.20210112.gappssmtp.com header.s=20210112 header.b=Mq60g49K; spf=none (imf28.hostedemail.com: domain of dan.j.williams@intel.com has no SPF policy when checking 209.85.216.41) smtp.mailfrom=dan.j.williams@intel.com; dmarc=fail reason="No valid SPF, DKIM not aligned (relaxed)" header.from=intel.com (policy=none) X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 3F8E3900009E X-HE-Tag: 1634261867-926080 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 Thu, Oct 14, 2021 at 4:06 PM Jason Gunthorpe wrote: > > On Thu, Oct 14, 2021 at 12:01:14PM -0700, Dan Williams wrote: > > > > Does anyone know why devmap is pte_special anyhow? > > > > It does not need to be special as mentioned here: > > > > https://lore.kernel.org/all/CAPcyv4iFeVDVPn6uc=aKsyUvkiu3-fK-N16iJVZQ3N8oT00hWA@mail.gmail.com/ > > I added a remark there > > Not special means more to me, it means devmap should do the refcounts > properly like normal memory pages. > > It means vm_normal_page should return !NULL and it means insert_page, > not insert_pfn should be used to install them in the PTE. VMAs should > not be MIXED MAP, but normal struct page maps. > > I think this change alone would fix all the refcount problems > everwhere in DAX and devmap. > > > The refcount dependencies also go away after this... > > > > https://lore.kernel.org/all/161604050866.1463742.7759521510383551055.stgit@dwillia2-desk3.amr.corp.intel.com/ > > > > ...but you can see that patches 1 and 2 in that series depend on being > > able to guarantee that all mappings are invalidated when the undelying > > device that owns the pgmap goes away. > > If I have put everything together right this is because of what I > pointed to here. FS-DAX is installing 0 refcount pages into PTEs and > expecting that to work sanely. > > This means the page map cannot be removed until all the PTEs are fully > flushed, which buggily doesn't happen because of the missing unplug. > > However, this is all because nobody incrd a refcount to represent the > reference in the PTE and since this ment that 0 refcount pages were > wrongly stuffed into PTEs then devmap used the refcount == 1 hack to > unbreak GUP? > > So.. Is there some reason why devmap pages are trying so hard to avoid > sane refcounting??? I wouldn't put it that way. It's more that the original sin of ZONE_DEVICE that sought to reuse the lru field space, by never having a zero recount, then got layered upon and calcified in malignant ways. In the meantime surrounding infrastructure got decrustified. Work like the 'struct page' cleanup among other things, made it clearer and clearer over time that the original design choice needed to be fixed. > If the PTE itself holds the refcount (by not being special) then there > is no need for the pagemap stuff in GUP. pagemap already waits for > refs to go to 0 so the missing shootdown during nvdimm unplug will > cause pagemap to block until the address spaces are invalidated. IMHO > this is already better than the current buggy situation of allowing > continued PTE reference to memory that is now removed from the system. > > > For that to happen there needs to be communication back to the FS for > > device-gone / failure events. That work is in progress via this > > series: > > > > https://lore.kernel.org/all/20210924130959.2695749-1-ruansy.fnst@fujitsu.com/ > > This is fine, but I don't think it should block fixing the mm side - > the end result here still cannot be 0 ref count pages installed in > PTEs. > > Fixing that does not depend on shootdown during device removal, right? > > It requires holding refcounts while pages are installed into address > spaces - and this lack is a direct cause of making the PTEs all > special and using insert_pfn and MIXED_MAP. The MIXED_MAP and insert_pfn were a holdover from page-less DAX, but now that we have page-available DAX, yes, we can skip the FS notification and just rely on typical refcounting and hanging until the FS has a chance to uninstall the PTEs. You're right, the FS notification is an improvement to the conversion, not a requirement. However, there still needs to be something in the gup-fast path to indicate that GUP_LONGTERM is not possible because the PTE represents a pfn that can not support typical page-cache behavior for truncate which is to just disconnect the page from the file and keep the page pinned indefinitely. I think the "no longterm" caveat would be the only remaining utility of PTE_DEVMAP after the above conversion to use typical page refcounts throughout DAX.