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 8B3CCC4332F for ; Wed, 14 Dec 2022 17:04:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 02EE08E0003; Wed, 14 Dec 2022 12:04:22 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id F21BD8E0002; Wed, 14 Dec 2022 12:04:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DE95C8E0003; Wed, 14 Dec 2022 12:04:21 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id CE0128E0002 for ; Wed, 14 Dec 2022 12:04:21 -0500 (EST) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 982A7A0CB4 for ; Wed, 14 Dec 2022 17:04:21 +0000 (UTC) X-FDA: 80241535122.21.0B0E69A Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf26.hostedemail.com (Postfix) with ESMTP id D2B5D140017 for ; Wed, 14 Dec 2022 17:04:16 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b="a0+L0Q0/"; spf=none (imf26.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1671037457; 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=gOpGhv6UKz7VHR3jMFGZw2EISvCY2AuNGxNyyrkNXxA=; b=AMvChdY+AYHJbT69FGtaR0Nu1bcGX+eFKcxFx1HNPTb2DSi0PO+7tjzefy23qz0h5+KWbO pIXm5AaVYmEkpjZsjZTS5F1exWVRtbHWR3UblOYGhE1DPsO7IVKqH3Pn4zS1eW9X04oC8a RqzvuTntDRhuxtCR5NJT73ThvNpfxgw= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b="a0+L0Q0/"; spf=none (imf26.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1671037457; a=rsa-sha256; cv=none; b=vevYCCGrOZ0mi48x5RdcbG8pM4NMvgf4JQvtXk65wE0+tNvo0NNYv3w34JmiZRDx8hhhT0 L7lO/fxdDG71s8G8AL/rWSPWPACeW93P+3xUyVzO0Y+abq53LdOKx741hxekxGfVIBLJfn 6KJ6z4JNdTiha4XQiuXx2bHZifEooys= 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=gOpGhv6UKz7VHR3jMFGZw2EISvCY2AuNGxNyyrkNXxA=; b=a0+L0Q0/xdslYVEnG3wV0PLkbV w2MST8acNOJgFLkYs124lEDIZDn1yO+vxjDFs5Iv4K5iSkHIgcvNW+tPvYbJB8XNGVvUjK5Areghc duAsWRMQwVo4ObmiFjxgCBrQ2DRj+k+CXmK6XDAN/b+OnLaJoTVeWmRTkAhkMMgKYS5x4fLfkDqoq dLyxrp6e4/YQeisgHYNgge4iKFNMEMwNfX7yHVJ3ogsNt//Ep6P3voubw8H5O6DyIVaWFisBzUnRD RyO8VuSn00gmggOhzseEzT7BSFJSuv44tpGI6N6pCF2T/UJgYc945Xm7epcNg+ceOmzbh5G8Tn40i 6nZqWbnQ==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1p5VB1-00DR0S-U5; Wed, 14 Dec 2022 17:04:15 +0000 Date: Wed, 14 Dec 2022 17:04:15 +0000 From: Matthew Wilcox To: Nico Pache Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, muchun.song@linux.dev, mike.kravetz@oracle.com, akpm@linux-foundation.org, gerald.schaefer@linux.ibm.com Subject: Re: [RFC V2] mm: add the zero case to page[1].compound_nr in set_compound_order Message-ID: References: <20221213234505.173468-1-npache@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221213234505.173468-1-npache@redhat.com> X-Rspamd-Queue-Id: D2B5D140017 X-Stat-Signature: qi5wk9459cej356bk4boh8kzjwwrn5oz X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1671037456-746285 X-HE-Meta: U2FsdGVkX1/juwA3uz9ok0lZ2iB2LzReB3B44ahmubna/noPk8dTCoVJ/2AfuM4yoYQgCYdcLPAyAQJNaXnBt+eIM2UAmnPuBBlwTl0ABNT3Yp3gUlUNBJhh7alXGuaOO1dpt/6GObi+nGa65VivjbFl3innihb4QNFz13syt/Hi9VUeLIlUYjp2jNONkcfp/Iues9IasFyv20yFgpAmX0gnlh6IA2oMuUIakP5DVeAWd92i1uSACSSKzAHNCxWrwN/X5R7B6i2i4EfNt+yEx6J73oYAWzF8Cqvbhm08v2iegJQN6ZGh2TBvIn7+yjX0iiOnZWKnhPbWyDk65IBwuFHKIaDkucnpjYqaDg0ufFyZhU2aznMnFZKUQq21Rp/fR0Dt9nhXbtiyw38qKqPpwD34oWDtrF3pKLlZyEpep83RAIJ9LvcLbkA7Ilnq5cmCWLHRE5XvBfDW6BIUdinhWKNV5XGD1DZB7/8H6678W4R8cUP+Bzc6znkpesGU/ws9r19TGlI2trdASE+IBSbfemaS+cr8SVkN/9nI+/s7nu8HEjrK5MGMHnDTVVMi+ECzwS0eQkInJFP/K2gF/hOf0Q3kWbUYbJnHUFpE1Jumx7PxkLZI7XqpGHVG5Djox29Eqd74MJTWD//lZNfNUPnU1K/Aak+XGlYewQkCgkqXKJAU/GxwOuqT7FyjWy8VkxUVpT8LYWFUEeEItR8RWyHb1bbarS0XVaL/n5qFerDiht3qxplAfrg2F1EWBVx9avZVfv7jWKHZHw0SHFvIjA5FL8zGHmzJqqb8DDdm3iachoudxg/txdK+tuouqNPsXm11g6vuGiskeTbXlA2F6m+/ronQ8SLHGQEeuy4OAnzMwHs+9sTRtElPvvQC/xXM2UsrXUv1lUmiagXrYQrcsD1Avw== 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, Dec 13, 2022 at 04:45:05PM -0700, Nico Pache wrote: > Since commit 1378a5ee451a ("mm: store compound_nr as well as > compound_order") the page[1].compound_nr must be explicitly set to 0 if > calling set_compound_order(page, 0). > > This can lead to bugs if the caller of set_compound_order(page, 0) forgets > to explicitly set compound_nr=0. An example of this is commit ba9c1201beaa > ("mm/hugetlb: clear compound_nr before freeing gigantic pages") > > Collapse these calls into the set_compound_order by utilizing branchless > bitmaths [1]. > > [1] https://graphics.stanford.edu/~seander/bithacks.html#ConditionalSetOrClearBitsWithoutBranching > > V2: slight changes to commit log and remove extra '//' in the comments We don't usually use // comments anywhere in the kernel other than the SPDX header. > static inline void set_compound_order(struct page *page, unsigned int order) > { > + unsigned long shift = (1U << order); Shift is a funny name for this variable. order is the shift. this is 'nr'. > page[1].compound_order = order; > #ifdef CONFIG_64BIT > - page[1].compound_nr = 1U << order; > + // Branchless conditional: > + // order > 0 --> compound_nr = shift > + // order == 0 --> compound_nr = 0 > + page[1].compound_nr = shift ^ (-order ^ shift) & shift; Can the compiler see through this? Before, the compiler sees: page[1].compound_order = 0; page[1].compound_nr = 1U << 0; ... page[1].compound_nr = 0; and it can eliminate the first store. Now the compiler sees: unsigned long shift = (1U << 0); page[1].compound_order = order; page[1].compound_nr = shift ^ (0 ^ shift) & shift; Does it do the maths at compile-time, knowing that order is 0 at this callsite and deducing that it can just store a 0? I think it might, since shift is constant-1, page[1].compound_nr = 1 ^ (0 ^ 1) & 1; -> page[1].compound_nr = 1 ^ 1 & 1; -> page[1].compound_nr = 0 & 1; -> page[1].compound_nr = 0; But you should run it through the compiler and check the assembly output for __destroy_compound_gigantic_page().