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 X-Spam-Level: X-Spam-Status: No, score=-3.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 21088C433DF for ; Wed, 26 Aug 2020 15:49:07 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id D560520738 for ; Wed, 26 Aug 2020 15:49:06 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="UkAgccpf" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D560520738 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 683BD6B000E; Wed, 26 Aug 2020 11:49:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6343E6B0010; Wed, 26 Aug 2020 11:49:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 549BB6B0023; Wed, 26 Aug 2020 11:49:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0155.hostedemail.com [216.40.44.155]) by kanga.kvack.org (Postfix) with ESMTP id 4157A6B000E for ; Wed, 26 Aug 2020 11:49:06 -0400 (EDT) Received: from smtpin27.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id F2ACD1F1A for ; Wed, 26 Aug 2020 15:49:05 +0000 (UTC) X-FDA: 77193153450.27.songs78_15011e527065 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin27.hostedemail.com (Postfix) with ESMTP id B20573D669 for ; Wed, 26 Aug 2020 15:49:05 +0000 (UTC) X-HE-Tag: songs78_15011e527065 X-Filterd-Recvd-Size: 4529 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf05.hostedemail.com (Postfix) with ESMTP for ; Wed, 26 Aug 2020 15:49:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=I18u0dNCe4slxMxufMguyF6f9uk6YgWYDMxiYXb8r/M=; b=UkAgccpf8C57KDxqkodEH9olO/ 1uRw550MvDayPZHtymBnFfxwmNtcgF2M6M96967ti7Q0kRqhAJD7EntbGZL40Lyy/dSXmziTiz7Hk lbnctZsn662cXzfd6hEb2utiYDSGSBQ9MMuBEMkU4epwpDT9aRW6rBD0kAE2/WhSDIlgYgN0uvYHv c5TtMuUjgUaaySu/tXn9XT3Fdu34TLsipV80GR1LCDvPxHlwYwEnRuupZUec5g7a9rqmgnBVv2OwM AfLYitWZTrvbKthc6NKOJtYTa0Bz0jsofeKe/qhWz/aTsyo6BYZPDxDJfSEiDV625F6g9O9aiVfKH 0a4QY2Ng==; Received: from willy by casper.infradead.org with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1kAxfX-0005m4-3j; Wed, 26 Aug 2020 15:48:59 +0000 Date: Wed, 26 Aug 2020 16:48:59 +0100 From: Matthew Wilcox To: Johannes Weiner Cc: linux-mm@kvack.org, Andrew Morton , Hugh Dickins , William Kucharski , Jani Nikula , Alexey Dobriyan , Chris Wilson , Matthew Auld , Huang Ying , intel-gfx@lists.freedesktop.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 6/8] mm: Convert find_get_entry to return the head page Message-ID: <20200826154859.GT17456@casper.infradead.org> References: <20200819184850.24779-1-willy@infradead.org> <20200819184850.24779-7-willy@infradead.org> <20200826150925.GE988805@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200826150925.GE988805@cmpxchg.org> X-Rspamd-Queue-Id: B20573D669 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam05 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 26, 2020 at 11:09:25AM -0400, Johannes Weiner wrote: > On Wed, Aug 19, 2020 at 07:48:48PM +0100, Matthew Wilcox (Oracle) wrote: > > There are only three callers remaining of find_get_entry(). > > find_get_swap_page() is happy to get the head page instead of the subpage. > > Add find_subpage() calls to find_lock_entry() and pagecache_get_page() > > to avoid auditing all their callers. > > I believe this would cause a subtle bug in memcg charge moving for pte > mapped huge pages. We currently skip over tail pages in the range > (they don't have page->mem_cgroup set) and account for the huge page > once from the headpage. After this change, we would see the headpage > and account for it 512 times (or whatever the number is on non-x86). Hmm ... so if you have the last 511 pages of a huge page mapped, you actually don't charge for it at all today? I think you're right that I'd introduce this bug, and so that needs to be fixed. > But that aside, I don't quite understand the intent. > > Before, all these functions simply return the base page at @index, > whether it's a regular page or a tail page. > > Afterwards, find_lock_entry(), find_get_page() et al still do, but > find_get_entry() returns headpage at @index & HPAGE_CACHE_INDEX_MASK. > > Shouldn't we be consistent about how we handle huge pages when > somebody queries the tree for a given base page index? > > [ Wouldn't that mean that e.g. find_get_swap_page() would return tail > pages for regular files and head pages for shmem files? ] What I'd _like_ to do is convert all the callers to cope with tail pages never being returned from all the find_* functions. That seems like a lot of disruption. My intent in this series is to get all the find_*_entr{y,ies} functions to the point where they don't return tail pages. Also find_get_pages_tag() because tags are only set on head pages. This is generally what the callers want anyway. There's even a hack in find_get_entries() in current to terminate early on finding a THP (see commit 71725ed10c40696dc6bdccf8e225815dcef24dba). If I want to remove that, I need to do _something_ to not put all the subpages of a THP into the pagevec. So the new rule will be that find_*_entry() don't return tail pages but find_*_page() do. With the full THP patchset in place, THPs become quite common, so bugs in this area will surface quickly instead of lingering for years and only popping out in rare circumstances.