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=-5.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 22C59C433F5 for ; Fri, 3 Sep 2021 16:18:44 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id B7128610CE for ; Fri, 3 Sep 2021 16:18:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org B7128610CE Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linux-foundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 52BE7940008; Fri, 3 Sep 2021 12:18:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4DB70900002; Fri, 3 Sep 2021 12:18:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3A2AB940008; Fri, 3 Sep 2021 12:18:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0059.hostedemail.com [216.40.44.59]) by kanga.kvack.org (Postfix) with ESMTP id 29EBB900002 for ; Fri, 3 Sep 2021 12:18:42 -0400 (EDT) Received: from smtpin30.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id D401C80AE847 for ; Fri, 3 Sep 2021 16:18:41 +0000 (UTC) X-FDA: 78546770442.30.10FB41B Received: from mail-lf1-f51.google.com (mail-lf1-f51.google.com [209.85.167.51]) by imf02.hostedemail.com (Postfix) with ESMTP id 7A3B77001A05 for ; Fri, 3 Sep 2021 16:18:41 +0000 (UTC) Received: by mail-lf1-f51.google.com with SMTP id s10so12724518lfr.11 for ; Fri, 03 Sep 2021 09:18:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Q+Ifm1Ubzf/9uVSh73mk7zdYrVrljFyDr53eQvvjhmE=; b=VvoY+GV60+vFaTTSTAXY/UA26ZqynQ7iZJyEAtYxN2Bc31ew6yGcV++VYWl2XVReF0 WZGfLonRSdg58CtPZ9bVY4Gyl0QEqRwaJYbD26pCd+m0CIt0PehAohS4+ab++JmvwhfQ pv84XxshkaFRsNbuy5kAh3Aap28CH5h8hqceI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Q+Ifm1Ubzf/9uVSh73mk7zdYrVrljFyDr53eQvvjhmE=; b=GqvU6LBE2LF6uQuG8zhQEQg8hBfjkdaR9f9zeldlDpyeZVkxX1i8jGQy2hzpTQp7jn q/H7cejk2v3XFZj97NFcDjLZ6ER5VNBedF9d3135mwUX6TA57MbIhWOk4PL/xtP4lMEF DfntEyyDKcJzcEBSP5yobXvcimnJ9TWGu0893mfqPrPIKeMuH+Kx7VOVS4hhZMpzN4Dp CngjJiAZVBWH1esWBjPCKmWTB4Y25E7ipXukETa2b7rse+3USROshjagkWP9qrnJsEKw J9T/6YlOTyR2MgGXRCW4AmCpXm/ZMijdBgPFJ/NIpkZlqEfNS7myj4akQubSfv+IscpX 9Zqg== X-Gm-Message-State: AOAM530kOjOyNRg4Mx9wi8H1bdwBx0/4ao+F3AcO/MYBWfuNiMFtbEmV VIk4gQgTDOnWmgjt+XeHFPTDpdUj5kjABCDlamg= X-Google-Smtp-Source: ABdhPJzbwXCVeih+x1Yz9S4bzbgsCjdqfoP4tPzFm8/R4bpOrMnxqkPJ2pVb9RwB7zgMEwoWWgyPxQ== X-Received: by 2002:ac2:5c04:: with SMTP id r4mr3288926lfp.182.1630685919089; Fri, 03 Sep 2021 09:18:39 -0700 (PDT) Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com. [209.85.167.44]) by smtp.gmail.com with ESMTPSA id t30sm548987lfg.289.2021.09.03.09.18.37 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 03 Sep 2021 09:18:38 -0700 (PDT) Received: by mail-lf1-f44.google.com with SMTP id y34so12736545lfa.8 for ; Fri, 03 Sep 2021 09:18:37 -0700 (PDT) X-Received: by 2002:a05:6512:3d28:: with SMTP id d40mr3444816lfv.474.1630685916389; Fri, 03 Sep 2021 09:18:36 -0700 (PDT) MIME-Version: 1.0 References: <20210902144820.78957dff93d7bea620d55a89@linux-foundation.org> <20210902215152.ibWfL_bvd%akpm@linux-foundation.org> In-Reply-To: From: Linus Torvalds Date: Fri, 3 Sep 2021 09:18:20 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [patch 036/212] mm, slab: make flush_slab() possible to call with irqs enabled To: Mike Galbraith Cc: Vlastimil Babka , Andrew Morton , Sebastian Andrzej Siewior , Jesper Dangaard Brouer , Christoph Lameter , Joonsoo Kim , Jann Horn , Linux-MM , Mel Gorman , mm-commits@vger.kernel.org, Pekka Enberg , David Rientjes , Thomas Gleixner Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=VvoY+GV6; spf=pass (imf02.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.167.51 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org; dmarc=none X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 7A3B77001A05 X-Stat-Signature: 7zqee5etdcuod1cd39uknaz7e9hib8x5 X-HE-Tag: 1630685921-686478 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 Thu, Sep 2, 2021 at 11:22 PM Mike Galbraith wrote: > > To my eyeballs, below duplication of a couple lines of initialization > needed by the lockless function is less icky than the double return. Ack. Something like this seems to avoid the whole issue. Vlastimil - the alternative I was actually going to suggest for that "double return value" thing is something we use elsewhere in the VM code - just use a structure for "state". It's not hugely common, and I think Mike's solution in this case is the much simpler one, but I'll mention it anyway because it's actually been quite successful in the few cases we use it, and it's both readable to humans and generates fairly good code. The "generates fairly good code" is generally more true when inlining, but it's not horrible even outside of that. So the idea with the "state structure" is that you just pass in both the arguments and the return values in a struct. Some examples of this are - 'struct follow_page_context' in mm/gup.c is an example of a small local one. - 'struct vm_fault' is an example of a *big* one, that has a lot of state, and that is actually exported as an interface. That 'struct vm_fault' one is interesting as an example because it does that "both inputs and outputs", and has that unnamed "const struct" member to actually catch mis-uses where some callee would change those fields (which is a no-no). It's also an example of something that generates good code despite not inlining, because it actively makes argument passing simpler and avoids copying arguments as you call other functions. Those issues are non-issues in this case, which is why I point to that 'follow_page_context' in the gup code as a very different example of this, which is much more along the lines of the slub case. It's purely local to that call chain, and it's basically used to return more state. In both cases, the top-level caller just initializes things on the stack. For that 'struct vm_fault' case you actually *have* to use an initializer, since the constant fields cannot be changed later. It makes for pretty legible code, and when things are inlined, all those fields actually act exactly like just regular local variables shared across functions. When things aren't inlined, it can generate extra memory accesses, but that 'struct vm_fault' is an example of when that isn't even necessarily worse. Anyway, I wanted to just point that out as a pattern that has been quite successful. Side note: one very special case of that pattern goes back decades: the VM use of structures that get returned and passed *as* structures. It's in fact so common that people don't even think about it: 'pte_t' and friends. They are designed to be opaque data types that are often multi-word structures. Quite often those structures have only one or two words in them, which helps code generation (returning two words can be done in registers), but they _can_ have more. Eg x86: typedef union { struct { unsigned long pte_low, pte_high; }; pteval_t pte; } pte_t; or some powerpc cases: typedef struct { pte_basic_t pte, pte1, pte2, pte3; } pte_t; just to show that you can actually pass structures not by reference, but by value, and it's not even unusual in the kernel. That can also be used to return multiple values if you want to. Note: code generation really matters, and when doing those multi-value structures, for code generation it's generally important to limit it to at most two words. Anything more, and the compiler will generally be forced to pass it by copying it to the stack and using a pointer to it, and then you get the worst of both worlds. You're better off just passing the structure by reference, and avoiding extra copies on the stack. ANYWAY. I'll drop this part from Andrew's patch-bomb, and I can take it later in its cleaned-up form. You mentioned a git tree elsewhere, maybe even that way. Linus