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 023C3C433FE for ; Mon, 28 Nov 2022 16:59:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4FCAF6B0073; Mon, 28 Nov 2022 11:59:55 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4ACEC6B0074; Mon, 28 Nov 2022 11:59:55 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 374296B0075; Mon, 28 Nov 2022 11:59:55 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 296326B0073 for ; Mon, 28 Nov 2022 11:59:55 -0500 (EST) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id E8FF5A0A12 for ; Mon, 28 Nov 2022 16:59:54 +0000 (UTC) X-FDA: 80183463108.06.9CE2309 Received: from mail-qv1-f44.google.com (mail-qv1-f44.google.com [209.85.219.44]) by imf27.hostedemail.com (Postfix) with ESMTP id 60D0A40014 for ; Mon, 28 Nov 2022 16:59:54 +0000 (UTC) Received: by mail-qv1-f44.google.com with SMTP id c14so3808810qvq.0 for ; Mon, 28 Nov 2022 08:59:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20210112.gappssmtp.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=OXdDTBqVegaar+pLWmnzFVWbntmDwb7qj9MvTJ7r5tA=; b=SpduupwX9+hgKbJAt1uBrr8GR8cVFBKX0qw5ONERsYzdWfB/N9QQYj9qCTlogNAj9M kEK1FZaNNPnXmtE+uVPtkinRgEpJ1JoknUQoYwGF0Q32h29zIr9jgsAs8IBYPk+9JHOn Su5WwW7J6E51pqICAcNNXMFqDgpDkcudvpSBi+GuH7OKHiHtB7StrdVPvzjyKor5dNKH qSYxxiWRzhj/yNfWFvtLA5Lw0fHWShpuoKz07eFZ+qSags11ZrQ6yOn7HI7EsRUagcth y3wAQAJjBGqBze7yEFUSoIPh0PUs+jQdGn2yb51ZjJ1Qw/2m/kffiVI1MZUrJWniHcRb 5pxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=OXdDTBqVegaar+pLWmnzFVWbntmDwb7qj9MvTJ7r5tA=; b=pp/4Y1QOS2Gfxswk/wkuzwEATpyZ2/aOePUwBBuC9ROipeCTTux7JQFQWyAsNB8A1P MuGRzGPmFUNDLLGuzvIXaZxQz49/QRdcs3ksVAp3ugbzt75vtl9EpyKMB8IhfWUhAOE1 6N+ZpqPQDn7MOs97DBmsf1NMLVZjHD3SEfBV9YK2gb2loI16RGTe3dAwUPcwzIJ5n9A/ pOlLLsl7GNs6owTH5GBz/gMCbFSZy0jfiswqf2Q3U+xF52Dfg6vhUclZ/8ZR5tunaKzv 1onXZ5mFA1MGLsuBOwTtZwTVDjVSvCvTtSWovsFnrqQd4MyRELEvi0sHsYSU89dHCCGf tjPA== X-Gm-Message-State: ANoB5pn0Us4ZDXB/J+UxoXROwA6CC4RTMF1knSbwzRhCgWUKXaOjbCDv H6zpa/ZvI9hgE//qth5OrizDLw== X-Google-Smtp-Source: AA0mqf6YL2EMCu1emEAeBljTAyGqg+e4upqSkexn0n7tt8PvbvgqT5YQUqibJtyHYwze4hQcv1N9Yg== X-Received: by 2002:a05:6214:328f:b0:4c6:82cd:92d1 with SMTP id mu15-20020a056214328f00b004c682cd92d1mr32083981qvb.82.1669654793427; Mon, 28 Nov 2022 08:59:53 -0800 (PST) Received: from localhost (2603-7000-0c01-2716-9175-2920-760a-79fa.res6.spectrum.com. [2603:7000:c01:2716:9175:2920:760a:79fa]) by smtp.gmail.com with ESMTPSA id s23-20020ac85297000000b003a5430ee366sm7141054qtn.60.2022.11.28.08.59.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 28 Nov 2022 08:59:53 -0800 (PST) Date: Mon, 28 Nov 2022 11:59:52 -0500 From: Johannes Weiner To: Hugh Dickins Cc: Andrew Morton , Linus Torvalds , Shakeel Butt , Stephen Rothwell , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm: remove lock_page_memcg() from rmap Message-ID: References: <20221123181838.1373440-1-hannes@cmpxchg.org> <16dd09c-bb6c-6058-2b3-7559b5aefe9@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <16dd09c-bb6c-6058-2b3-7559b5aefe9@google.com> ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1669654794; a=rsa-sha256; cv=none; b=LyFwIw0Nw1irnBhL9BJVpcwqU8xXKaqBorbqsJCM3eGaq7t/6DFYfjBhJ4SHVN26xX2o7i SbXs1PTKpa1ZaoVpuwae7o5es5hhTJKVZzCxWjR5epbOyyL/d7VFZuui58bNBiyrhT4WiB uBY5PPRsUtvuQ0H4Kh15aATMUFD4AUg= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b=SpduupwX; spf=pass (imf27.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.44 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1669654794; 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=OXdDTBqVegaar+pLWmnzFVWbntmDwb7qj9MvTJ7r5tA=; b=2cfk87eW2mJ+vIedXzfV69hacU5cbABkkPYuCIACG1Cgoj6oVhwPvzqK9Kf1vq3KwRGcma X3rsPycS1y7x4K1tL0DC8l5g51cAmjylsHtcrtMaOeZyv6tcytAWWLwpn1/ChAQHloAWAQ AgTg9gLlw0Xz7Me2fGL0gwPeow4Pz/w= X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 60D0A40014 X-Rspam-User: Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b=SpduupwX; spf=pass (imf27.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.44 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org X-Stat-Signature: p88eubkfracuamnu85ds3wwp7opow5u1 X-HE-Tag: 1669654794-543387 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, Nov 23, 2022 at 10:03:00PM -0800, Hugh Dickins wrote: > On Wed, 23 Nov 2022, Johannes Weiner wrote: > > rmap changes (mapping and unmapping) of a page currently take > > lock_page_memcg() to serialize 1) update of the mapcount and the > > cgroup mapped counter with 2) cgroup moving the page and updating the > > old cgroup and the new cgroup counters based on page_mapped(). > > > > Before b2052564e66d ("mm: memcontrol: continue cache reclaim from > > offlined groups"), we used to reassign all pages that could be found > > on a cgroup's LRU list on deletion - something that rmap didn't > > naturally serialize against. Since that commit, however, the only > > pages that get moved are those mapped into page tables of a task > > that's being migrated. In that case, the pte lock is always held (and > > we know the page is mapped), which keeps rmap changes at bay already. > > > > The additional lock_page_memcg() by rmap is redundant. Remove it. > > > > Signed-off-by: Johannes Weiner > > Thank you, I love it: but with sorrow and shame, NAK to this version. > > I was gearing up to rush in the crash fix at the bottom, when testing > showed that the new VM_WARN_ON_ONCE(!folio_mapped(folio)) actually hits. > > So I've asked Stephen to drop this mm-unstable commit from -next for > tonight, while we think about what more is needed. > > I was disbelieving when I saw the warning, couldn't understand at all. > But a look at get_mctgt_type() shatters my illusion: it's doesn't just > return a page for pte_present(ptent), it goes off looking up swap > cache and page cache; plus I've no idea whether an MC_TARGET_DEVICE > page would appear as folio_mapped() or not. Thanks for catching this. A device_private pte always has a matching mapcount in the fake page it points to, so we should be good here. Those pages migrate back and forth between system and device memory, relying on the migration code's unmap and restore bits. Hence they participate in regular rmap. The swapcache/pagecache bit was a brainfart. We acquire the folio lock in move_account(), which would lock out concurrent faults. If it's not mapped, I don't see how it could become mapped behind our backs. But we do need to be prepared for it to be unmapped. > Does that mean that we just have to reinstate the folio_mapped() checks > in mm/memcontrol.c i.e. revert all mm/memcontrol.c changes from the > commit? Or does it invalidate the whole project to remove > lock_page_memcg() from mm/rmap.c? I think we have to bring back the folio_mapped() conditional and update the comments. But it shouldn't invalidate the whole project. I'll triple check this, however. Thanks