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.8 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 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 D8FF0C433F5 for ; Thu, 2 Sep 2021 23:15:52 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 6058E6108E for ; Thu, 2 Sep 2021 23:15:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 6058E6108E 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 DC96F6B0071; Thu, 2 Sep 2021 19:15:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D78B16B0072; Thu, 2 Sep 2021 19:15:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C665A6B0073; Thu, 2 Sep 2021 19:15:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0131.hostedemail.com [216.40.44.131]) by kanga.kvack.org (Postfix) with ESMTP id B581B6B0071 for ; Thu, 2 Sep 2021 19:15:51 -0400 (EDT) Received: from smtpin36.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 79F792571F for ; Thu, 2 Sep 2021 23:15:51 +0000 (UTC) X-FDA: 78544192902.36.C5F0EF4 Received: from mail-lj1-f175.google.com (mail-lj1-f175.google.com [209.85.208.175]) by imf29.hostedemail.com (Postfix) with ESMTP id 36F98900024E for ; Thu, 2 Sep 2021 23:15:51 +0000 (UTC) Received: by mail-lj1-f175.google.com with SMTP id p15so6599259ljn.3 for ; Thu, 02 Sep 2021 16:15:50 -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=QEe1uUnB3VIjFnanRTFg5dOUu65jLiMP0LAdT+dmP0I=; b=ENRs3hCxFxVqQbIU8uvntM+TTwPpJnXQYJJ3102PI6+lk3shQTK4KN5aU9ENRLR+NP T+1l14GvTuu42eMZRSERXr3iVMGmCDC5juEDGcrVriJZQYPh+h902/Ez7xAg0H24teCU 9nulPtqVs8Ow5pmWkKNuUlUjs1cuY1rTmdQtg= 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=QEe1uUnB3VIjFnanRTFg5dOUu65jLiMP0LAdT+dmP0I=; b=HQqrqQ1WMXs22K9d3F/jx1CBXmej7AP28BQas+IHc4oMuJTCSNHsxdw/zhPZ2pR6vD sKhcW2W0N9UTAX1197ajo9/1VfwKXrkjV2XzoDaU5oRyHIpZx3VA0Bl1/NdUHfN9L/fl DJeeEYybPMO41cp+zIKvvYaNgAfoTIXp8PrGQso1dwVR0hWxI3B+tL75N4LqEj+CclXO d7ZNKofUWW9cwEQmylZJ8cdMfTIYrtGqDTmerAU83m8wOkTuR/oZdFdcCwqXxy1XSAwG KZqYU+KMaIKn7MdtvYfnl2VvkN4vgQyQ7PyorjLC59xMjSCQHjMliirsCKDO/1efOIDX bFdQ== X-Gm-Message-State: AOAM530gKvumS+gMX41vmk9OZ9vqPXaxC5qW3GX6IT2VjdkjVfOI5eQE gr7uro6fuGWJfA7j5089pfS8pF9D6X3IEmQSN3I= X-Google-Smtp-Source: ABdhPJx1SENdXE+h8zEwBDwUcVAdvaWgInTT9I6dW0w4WeXea6s1Dx0myJpNNujk4wHeuK9+EWfOQQ== X-Received: by 2002:a2e:bd84:: with SMTP id o4mr589160ljq.380.1630624549183; Thu, 02 Sep 2021 16:15:49 -0700 (PDT) Received: from mail-lf1-f43.google.com (mail-lf1-f43.google.com. [209.85.167.43]) by smtp.gmail.com with ESMTPSA id a13sm386316ljk.92.2021.09.02.16.15.48 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 02 Sep 2021 16:15:48 -0700 (PDT) Received: by mail-lf1-f43.google.com with SMTP id x27so7855852lfu.5 for ; Thu, 02 Sep 2021 16:15:48 -0700 (PDT) X-Received: by 2002:a05:6512:228f:: with SMTP id f15mr404545lfu.253.1630624548039; Thu, 02 Sep 2021 16:15:48 -0700 (PDT) MIME-Version: 1.0 References: <20210902144820.78957dff93d7bea620d55a89@linux-foundation.org> <20210902215152.ibWfL_bvd%akpm@linux-foundation.org> In-Reply-To: <20210902215152.ibWfL_bvd%akpm@linux-foundation.org> From: Linus Torvalds Date: Thu, 2 Sep 2021 16:15:32 -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: Andrew Morton Cc: Sebastian Andrzej Siewior , Jesper Dangaard Brouer , Christoph Lameter , Mike Galbraith , Joonsoo Kim , Jann Horn , Linux-MM , Mel Gorman , mm-commits@vger.kernel.org, Pekka Enberg , David Rientjes , Thomas Gleixner , Vlastimil Babka Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=ENRs3hCx; dmarc=none; spf=pass (imf29.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.208.175 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 36F98900024E X-Stat-Signature: ornrfscmsxoxpxxtq8j74g3wswafwxx9 X-HE-Tag: 1630624551-618780 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 2:51 PM Andrew Morton wrote: > > From: Vlastimil Babka > Subject: mm, slab: make flush_slab() possible to call with irqs enabled > > Currently flush_slab() is always called with disabled IRQs if it's needed, > but the following patches will change that, so add a parameter to control > IRQ disabling within the function, which only protects the kmem_cache_cpu > manipulation and not the call to deactivate_slab() which doesn't need it. I absolutely DETEST this patch. Code like this is just garbage: > -static inline void flush_slab(struct kmem_cache *s, struct kmem_cache_cpu *c) > +static inline void flush_slab(struct kmem_cache *s, struct kmem_cache_cpu *c, > + bool lock) .... > + if (lock) > + local_irq_save(flags); with actively misleading names ("lock"? WTF? There's no lock there!) and just disgusting semantics with conditionals etc. And the thing is, I don't even see that this would be in the least required. If that function had just been split into two: the first part that needed to absolutely be called with interrupts disabled, and the latter part that didn't, then none of this stupid ugly code would be needed at all. So flush_slab() could have been split into "that first part that returned the freelist and page, and then the second part that did the deactivate_slab() and the stats update. Then somebody who had interrupts disabled anyway (ie existing cases) would just do both. And then the new code that you want to have with interrupts disabled just for the first part would do the first part with interrupts disabled, and the second part without. See? No need for a "flags" field and odd conditional interrupt disables. And yes, I realize that the "inline" means that *most* of the time, the compiler will inline things, the "conditional" will become static, and it will not be a run-time conditional. But it's the human-readable conditional that is the ugly part here. Now any sane human that reads that flush_slab() code will see "oh, sometimes interrupts are disabled, and sometimes they aren't". Because that is what the code does, at that level. But no, what's actually going on is that interrupts are *always* disabled - it's just that sometimes they will have already been disabled in the caller. So all that misleading and misnamed "lock" argument does is really "were interrupts already disabled so that I don't need to disable and re-enable them". That's what the code actually *does*, but it's not how the code reads, and it's not how the code is written, or how the argument in question is named. So it's all very misleading, and ugly. I'm going to sleep on this, and maybe tomorrow morning my reaction will be "whatever, the code probably works". But more likely, tomorrow morning I will take another look, and still say "no, this is actually making the code less maintainable and actively misleading", and just throw the whole slab series out the window. Linus