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 42E83C433F5 for ; Tue, 12 Oct 2021 19:24:27 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id E4E5060F3A for ; Tue, 12 Oct 2021 19:24:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org E4E5060F3A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 75749900003; Tue, 12 Oct 2021 15:24:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7066E900002; Tue, 12 Oct 2021 15:24:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5F543900003; Tue, 12 Oct 2021 15:24:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0046.hostedemail.com [216.40.44.46]) by kanga.kvack.org (Postfix) with ESMTP id 50250900002 for ; Tue, 12 Oct 2021 15:24:26 -0400 (EDT) Received: from smtpin19.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 03D481803A573 for ; Tue, 12 Oct 2021 19:24:26 +0000 (UTC) X-FDA: 78688761732.19.B9E6AF9 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf18.hostedemail.com (Postfix) with ESMTP id A063640020AB for ; Tue, 12 Oct 2021 19:24:25 +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=/Io97xWCIAvhe+Q9IKSft7X2dCSmZRv6fmuAMd3ittU=; b=iaxMuyrlfJD+ROAL3kySF6k/hU FTklnjA5ekaLB9Qm7xsCFLWvYfEXqRtAbjZvC3a1juvWqLbyDSOjy4fq1L9oaPhmCzGasrWsbATVJ r4qwz/SeNol5SEF8vfzWbs1Iaclro0AY7+khNzfjxlgZhIvOa26BXb4gZg1/OxGy79DBDRa/zspTH ObivPVdPCuLo2moHmdBHaliOv+k3LnLY1Vkj7E0iuPOITteEF696W9dmP4zdas7zeqmNYu4JAEd+4 fea1w+W+M82ALPjS0RrjT6wrcMS6Ebxf8Qq4VWG+JagMYmW83I72RbLCStaD0Q0w8wvEcv6Gw0hLO tYW7QWfw==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1maNN0-006mCo-6N; Tue, 12 Oct 2021 19:23:37 +0000 Date: Tue, 12 Oct 2021 20:23:26 +0100 From: Matthew Wilcox To: Johannes Weiner Cc: linux-mm@kvack.org, Kent Overstreet , "Kirill A. Shutemov" , Vlastimil Babka , Michal Hocko , Roman Gushchin , linux-kernel@vger.kernel.org, kernel-team@fb.com Subject: Re: [PATCH 00/11] PageSlab: eliminate unnecessary compound_head() calls Message-ID: References: <20211012180148.1669685-1-hannes@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211012180148.1669685-1-hannes@cmpxchg.org> X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: A063640020AB X-Stat-Signature: apkg9yzmu7nu99bbrp7do8gjqaj3tp6m Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=iaxMuyrl; spf=none (imf18.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none X-HE-Tag: 1634066665-744472 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 Tue, Oct 12, 2021 at 02:01:37PM -0400, Johannes Weiner wrote: > PageSlab() currently imposes a compound_head() call on all callsites > even though only very few situations encounter tailpages. This short > series bubbles tailpage resolution up to the few sites that need it, > and eliminates it everywhere else. > > This is a stand-alone improvement. However, it's inspired by Willy's > patches to split struct slab from struct page. Those patches currently > resolve a slab object pointer to its struct slab as follows: > > slab = virt_to_slab(p); /* tailpage resolution */ > if (slab_test_cache(slab)) { /* slab or page alloc bypass? */ > do_slab_stuff(slab); > } else { > page = (struct page *)slab; > do_page_stuff(page); > } > > which makes struct slab an ambiguous type that needs to self-identify > with the slab_test_cache() test (which in turn relies on PG_slab in > the flags field shared between page and slab). > > It would be preferable to do: > > page = virt_to_head_page(p); /* tailpage resolution */ > if (PageSlab(page)) { /* slab or page alloc bypass? */ > slab = page_slab(page); > do_slab_stuff(slab); > } else { > do_page_stuff(page); > } > > and leave the ambiguity and the need to self-identify with struct > page, so that struct slab is a strong and unambiguous type, and a > non-NULL struct slab encountered in the wild is always a valid object > without the need to check another dedicated flag for validity first. > > However, because PageSlab() currently implies tailpage resolution, > writing the virt->slab lookup in the preferred way would add yet more > unnecessary compound_head() call to the hottest MM paths. > > The page flag helpers should eventually all be weaned off of those > compound_head() calls for their unnecessary overhead alone. But this > one in particular is now getting in the way of writing code in the > preferred manner, and bleeding page ambiguity into the new types that > are supposed to eliminate specifically that. It's ripe for a cleanup. So what I had in mind was more the patch at the end (which I now realise is missing the corresponding changes to __ClearPageSlab()). There is, however, some weirdness with kfence, which appears to be abusing PageSlab by setting it on pages which are not slab pages??? page = virt_to_page(p); if (PageSlab(page)) { /* slab or page alloc bypass? */ slab = page_slab(page); /* tail page resolution */ do_slab_stuff(slab); } else { do_page_stuff(page); /* or possibly compound_head(page) */ } There could also be a PageTail check in there for some of the cases -- catch people doing something like: kfree(kmalloc(65536, GFP_KERNEL) + 16384); which happens to work today, but should probably not. diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h index 8951da34aa51..b4b62fa100eb 100644 --- a/include/linux/page-flags.h +++ b/include/linux/page-flags.h @@ -344,7 +344,7 @@ PAGEFLAG(Active, active, PF_HEAD) __CLEARPAGEFLAG(Active, active, PF_HEAD) TESTCLEARFLAG(Active, active, PF_HEAD) PAGEFLAG(Workingset, workingset, PF_HEAD) TESTCLEARFLAG(Workingset, workingset, PF_HEAD) -__PAGEFLAG(Slab, slab, PF_NO_TAIL) +__PAGEFLAG(Slab, slab, PF_ANY) __PAGEFLAG(SlobFree, slob_free, PF_NO_TAIL) PAGEFLAG(Checked, checked, PF_NO_COMPOUND) /* Used by some filesystems */ diff --git a/mm/slab.c b/mm/slab.c index d0f725637663..c8c6e8576936 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -1371,6 +1371,7 @@ static struct page *kmem_getpages(struct kmem_cache *cachep, gfp_t flags, int nodeid) { struct page *page; + int i; flags |= cachep->allocflags; @@ -1381,7 +1382,8 @@ static struct page *kmem_getpages(struct kmem_cache *cachep, gfp_t flags, } account_slab_page(page, cachep->gfporder, cachep, flags); - __SetPageSlab(page); + for (i = 0; i < compound_nr(page); i++) + __SetPageSlab(page + i); /* Record if ALLOC_NO_WATERMARKS was set when allocating the slab */ if (sk_memalloc_socks() && page_is_pfmemalloc(page)) SetPageSlabPfmemalloc(page); diff --git a/mm/slub.c b/mm/slub.c index f7ac28646580..e442cebda4ef 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -1916,7 +1916,8 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node) account_slab_page(page, oo_order(oo), s, flags); page->slab_cache = s; - __SetPageSlab(page); + for (idx = 0; idx < compound_nr(page); idx++) + __SetPageSlab(page + idx); if (page_is_pfmemalloc(page)) SetPageSlabPfmemalloc(page);