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 6DFA9C433FE for ; Wed, 9 Nov 2022 18:01:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DB4796B0072; Wed, 9 Nov 2022 13:01:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D65818E0001; Wed, 9 Nov 2022 13:01:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C2CBE6B0074; Wed, 9 Nov 2022 13:01:09 -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 B25096B0072 for ; Wed, 9 Nov 2022 13:01:09 -0500 (EST) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 57A4AC1415 for ; Wed, 9 Nov 2022 18:01:09 +0000 (UTC) X-FDA: 80114670258.15.66869F9 Received: from mail-qk1-f174.google.com (mail-qk1-f174.google.com [209.85.222.174]) by imf11.hostedemail.com (Postfix) with ESMTP id 79A8140016 for ; Wed, 9 Nov 2022 18:01:06 +0000 (UTC) Received: by mail-qk1-f174.google.com with SMTP id f8so11419246qkg.3 for ; Wed, 09 Nov 2022 10:01:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=JRzIGvRnX1544Be7qGWYKxjG2G0UEG52dC2pZ/P5njY=; b=Q2jYdJ6BbKHuyhyzDPeRs1/ZAg/T1ka3/R6bWojOn+Omzgy5Dj4jKfPUPoXigYRz80 U1uLTzqZSA0bShmbIh2qSyRbsyOr3g9pSkEB6QlT4emsVFcwnugFBjwDaP89EpUhOqYg XC/lT32oxBfKrKZtT6su0wEqn9DzOp1W7zPnY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=JRzIGvRnX1544Be7qGWYKxjG2G0UEG52dC2pZ/P5njY=; b=YOl6Li3M9ybet9Wrb16hyZ0J2N7W+1MJ1oKaJnfmI1I9BSARhWuiShrOULOJNg2fcy wkFAmNS1awGT4w+ON4tLtiy6yUsgDKIlCbjPLA5UVhgylgefNObwF4Zx4CE2jiuC+jeC HtKzTTwQ0bcQoe+h/1ejCpDEhKYKhFbyKdDLCqKW6DpNVW1emj9wB9EtjqXUjx0vJzIY x+o/wVRsbQGQGYmyfyvXxG4+GSZhXEZdRWEKPuRSiCGrHSGYK2sFDyEMwzxBvZUwWuvx ZnLB+TuvO+8Zna22v42eyKbv4cJE29lRRkSbh55j1/qYLFeMiltW/YgD0f0YDRXpXwHl Qg6Q== X-Gm-Message-State: ACrzQf1VuGBMee6aVCXjWkbJ9LhejB9G/ZyGV9PN3si3uq8yCCzaMdBv uGKY/fCQFE7MPrgxNj02jzNgsJKHKu8i9Q== X-Google-Smtp-Source: AMsMyM7AZ1oqo3/E1KuVIz/Cx19uR7dJeEEXRAH8Skq2x+IlR70xB5oNwEsn6ij0XWuv/W+zR35H+g== X-Received: by 2002:a05:620a:410f:b0:6cf:c34b:3c64 with SMTP id j15-20020a05620a410f00b006cfc34b3c64mr44037553qko.52.1668016864707; Wed, 09 Nov 2022 10:01:04 -0800 (PST) Received: from mail-yb1-f177.google.com (mail-yb1-f177.google.com. [209.85.219.177]) by smtp.gmail.com with ESMTPSA id s13-20020a05620a0bcd00b006bb8b5b79efsm11508217qki.129.2022.11.09.10.01.03 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 09 Nov 2022 10:01:03 -0800 (PST) Received: by mail-yb1-f177.google.com with SMTP id 63so21924023ybq.4 for ; Wed, 09 Nov 2022 10:01:03 -0800 (PST) X-Received: by 2002:a05:6902:124f:b0:66e:e3da:487e with SMTP id t15-20020a056902124f00b0066ee3da487emr62350908ybu.310.1668016862980; Wed, 09 Nov 2022 10:01:02 -0800 (PST) MIME-Version: 1.0 References: <20221108194139.57604-1-torvalds@linux-foundation.org> In-Reply-To: From: Linus Torvalds Date: Wed, 9 Nov 2022 10:00:46 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/4] mm: introduce 'encoded' page pointers with embedded extra bits To: Alexander Gordeev Cc: Hugh Dickins , Johannes Weiner , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1668016866; 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=JRzIGvRnX1544Be7qGWYKxjG2G0UEG52dC2pZ/P5njY=; b=ztpIp4GIYb4A9RZu1M042vQJqGbaU5SMLKIy2Yw7JKNUc9FE5QRTAqJ92bEQmwIUhsAWqW uffRDcbZzlox5h7LR7HFHIgUXC4pDJ0k+CXlfWRxIAbAFDMtQ3noPGCkVgfYXXMnMk/kjv SCk7T5+UWpcged6jWdEfRAPDQB8jujo= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=Q2jYdJ6B; spf=pass (imf11.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.222.174 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1668016866; a=rsa-sha256; cv=none; b=UcLFdSNIF2JrXQhVXRqwaM9rOqpPfSvS+J4HKNiFpE/nSc8H8OGgfvDQHu2DRlsgASdfAg VlF+EkcMzXDA6DV3SYEyEj+MDU2cubAINg2HhBe0LfdDAG/fTFYocn52Fa7zQdVazmiKqz ZhthpCqu1yfS8A3b3uxnfwZxDDPg0Wg= X-Rspamd-Queue-Id: 79A8140016 Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=Q2jYdJ6B; spf=pass (imf11.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.222.174 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org; dmarc=none X-Rspamd-Server: rspam10 X-Rspam-User: X-Stat-Signature: 7w1jumno9ttkpezbbgahou76fmg97xer X-HE-Tag: 1668016866-910234 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, Nov 8, 2022 at 10:38 PM Alexander Gordeev wrote: > > On Tue, Nov 08, 2022 at 11:41:36AM -0800, Linus Torvalds wrote: > > > +static inline struct encoded_page *encode_page(struct page *page, unsigned long flags) > > +{ > > Any reaction in case ((flags & ~ENCODE_PAGE_BITS) != 0)? Heh. I've actually had three different implementations for that during the development series, and I think I even posted them all at one point or another (although usually just as attachments). And none of them are good. Those three trivial versions are: (a) use VM_BUG_ON(), (b) just silently mask the bits and (c) just silently add them. And (c) is that least annoying option that this latest patch uses, because both (a) and (b) are just nasty. Basically, all users are locally trivial to verify statically, so VM_BUG_ON() is just conceptually wrong and generates extra pointless code. And the silent masking - if it makes any difference - is just another version of "just silently add the bits": regardless of whether it clears them or not, it does the wrong thing if the bits don't fit. So there are three bad options, I've gone back and forth between them all, and I chose the least offensive one that is "invisible", in that it at least doesn't do any extra pointless work. Now, there are two non-offensive options too, and I actually considered, but never implemented them. They both fix the problem properly, by making it a *buildtime* check, but they have other issues. There's two ways to just make it a build-time check, and it's annoyingly _close_ to being usable, but not quite there. One is simply to require that the flags argument is always a plain constant, and simply using BUILD_BUG_ON(). I actually almost went down that path - one of the things I considered was to not add a 'flags' argument to __tlb_remove_page() at all, but instead just have separate __tlb_remove_page() and __tlb_remove_page_dirty() functions. That would have meant that the argument to __tlb_remove_page_size would have always been a built-time constant, and then it would be trivial to just have that BUILD_BUG_ON(). Problem solved. But it turns out that it's just nasty, particularly with different configurations wanting different rules for what the dirty bit is. So forcing it to some constant value was really not acceptable. The thing that I actually *wanted* to do, but didn't actually dare, was to just say "I will trust the compiler to do the value range tracking". Because *technically* our BUILD_BUG_ON() doesn't need a compile-time constant. Because our implementation of BUILD_BUG_ON() is not the garbage that the compiler gives us in "_Static_assert()" that really requires a syntactically pure integer constant expression. So the kernel version of BUILD_BUG_ON() is actually something much smarter: it depends on the compiler actually *optimizing* the expression, and it's only that optimized value that needs to be determined at compile-time to be either true or false. You can use things like inline functions etc, just as long as the end result is obvious enough that the compiler ends up saying "ok, that's never the case". And *if* the compiler does any kind of reasonable range analysis, then a BUILD_BUG_ON(flags > ENCODE_PAGE_BITS); should actually work. In theory. In practice? Not so much. Because while the argument isn't constant (not even in the caller), the compiler *should* be smart enough to see that in the use in mm/memory.c, 'flags' is always that unsigned int delay_rmap; which then gets initialized to delay_rmap = 0; and conditionally set to '1' later. So it's not a *constant*, but the compiler can see that the value of flags is clearly never larger than ENCODE_PAGE_BITS. But right now the compiler cannot track that over the non-inline function in __tlb_remove_page_size(). Maybe if the 'encode_page()' was done in the caller, and __tlb_remove_page_size() were to just take an encoded_page as the argument, then the compiler would always only see this all through inlined functions, and it would work. But even if it were to work for me (I never tried), I'd have been much too worried that some other compiler version, with some other config options, on some other architecture, wouldn't make the required optimizations. We do require compiler optimizations to be on for 'BUILD_BUG_ON()' to do anything at all: #ifdef __OPTIMIZE__ # define __compiletime_assert(condition, msg, prefix, suffix) \ .. #else # define __compiletime_assert(condition, msg, prefix, suffix) do { } while (0) #endif and we have a lot of places that depend on BUILD_BUG_ON() to do basic constant folding and other fairly simple optimizations. But while I think a BUILD_BUG_ON() would be the right thing to do here, I do not feel confident enough to really put that to the test. Linus