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 CFB6EC433EF for ; Mon, 27 Sep 2021 11:05:52 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 5C72F61058 for ; Mon, 27 Sep 2021 11:05:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 5C72F61058 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id DAE4C6B0071; Mon, 27 Sep 2021 07:05:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D379D900003; Mon, 27 Sep 2021 07:05:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BD8A7900002; Mon, 27 Sep 2021 07:05:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0194.hostedemail.com [216.40.44.194]) by kanga.kvack.org (Postfix) with ESMTP id A801A6B0071 for ; Mon, 27 Sep 2021 07:05:51 -0400 (EDT) Received: from smtpin06.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 63CFC181B048E for ; Mon, 27 Sep 2021 11:05:51 +0000 (UTC) X-FDA: 78633073302.06.BC777FF Received: from mail-pf1-f171.google.com (mail-pf1-f171.google.com [209.85.210.171]) by imf27.hostedemail.com (Postfix) with ESMTP id 24E5C70000A7 for ; Mon, 27 Sep 2021 11:05:51 +0000 (UTC) Received: by mail-pf1-f171.google.com with SMTP id y8so15488591pfa.7 for ; Mon, 27 Sep 2021 04:05:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=L/js/+QIAeCH/e3oDGYGUuW3olbH7OQ+NYutC4UOl0Y=; b=PMc1sKOeGmB+O66/la6pC7P/0/IoeFQ1HY5bLSqnJbEP5r3KwxVoWqM9/Dz+iKJ77M tegUs/+E16t4p++UWJQ3I1PpOHiQ40+XF0JkQb0dvpaX7rv3MCHuzBseeRIEQ4f7kUXa GGRlwenK8R24PGD80cuJc05Hfga9KeIScZrWDotmdttB54jele21oBUS6svPtu9+MaOp a186aGU8NUePIlefKL+O44ozpQMRdmJjfuhBOAmXYrMUjYo4DSjNtbSb2qde5QX6MKLS S21RxHx5i43r7z2pXfnT8q/fjKwwwr8wZjWzo94vzRTVBdzqS4RWwFkIHXhBi9K26zbn L1Yg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=L/js/+QIAeCH/e3oDGYGUuW3olbH7OQ+NYutC4UOl0Y=; b=lMMOmCxoDAstVd7uK9J/Hy2pThUU3COSpaxlQKRYO82bZA7xgf6FTs653z/zDuLtkJ uAvt+mgm7VpLpdKEY1ARh3P5ONCS75P7wIPt+a2zWfXHDMe/40IZQyQcd+M8kkMY4WT/ jYHhmoJ+KAGOEZO5osX1H19K4F05lPiyu9x1ExQUEuzYpTGWSNXg4EK5fOzht7MYCc3g 3c3aUSb0at4nP1Xiqko5hqJSMivyQsrPDuCj773c4Yag2WX4u+Gf5LQHeohrKCZD/Jmk 7n+ADRDqegLUS2WTMO1y1fd7oAY9pJ2Kr5WJpyxZJBCVmt/SaYVgJdagUJ/VxOpFcOSl lURw== X-Gm-Message-State: AOAM5338A3mUs/bupYJGV33NgrkofOKDRIR/sqvCPR5ro1N6DN2UInQ2 VFGCMO+g8tGr6dZ2W4yMMOOz06TtR/I= X-Google-Smtp-Source: ABdhPJy/1CsOZhw9V9Hi0MqFMmcVeQjk9bYbkOoZVDtlFRb6X6fmqYBTtL5dUhBSY/14YW80ALlmsQ== X-Received: by 2002:a63:33cb:: with SMTP id z194mr16254012pgz.380.1632740749950; Mon, 27 Sep 2021 04:05:49 -0700 (PDT) Received: from smtpclient.apple (c-24-6-216-183.hsd1.ca.comcast.net. [24.6.216.183]) by smtp.gmail.com with ESMTPSA id z202sm17093973pfc.40.2021.09.27.04.05.48 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 27 Sep 2021 04:05:49 -0700 (PDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.120.0.1.13\)) Subject: Re: [RFC PATCH 2/8] mm/madvise: remove unnecessary check on madvise_dontneed_free() From: Nadav Amit In-Reply-To: <20210927091143.tn6ediykqycu6rtu@box.shutemov.name> Date: Mon, 27 Sep 2021 04:05:47 -0700 Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Peter Xu , Andrea Arcangeli , Minchan Kim , Colin Cross , Suren Baghdasarya , Mike Rapoport Content-Transfer-Encoding: quoted-printable Message-Id: References: <20210926161259.238054-1-namit@vmware.com> <20210926161259.238054-3-namit@vmware.com> <20210927091143.tn6ediykqycu6rtu@box.shutemov.name> To: "Kirill A. Shutemov" X-Mailer: Apple Mail (2.3654.120.0.1.13) X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 24E5C70000A7 X-Stat-Signature: yihobd1wo9kogqgsisqb67mfchxomniy Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=PMc1sKOe; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf27.hostedemail.com: domain of nadav.amit@gmail.com designates 209.85.210.171 as permitted sender) smtp.mailfrom=nadav.amit@gmail.com X-HE-Tag: 1632740751-915172 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 Sep 27, 2021, at 2:11 AM, Kirill A. Shutemov = wrote: >=20 > On Sun, Sep 26, 2021 at 09:12:53AM -0700, Nadav Amit wrote: >> From: Nadav Amit >>=20 >> madvise_dontneed_free() is called only from madvise_vma() and the >> behavior is always either MADV_FREE or MADV_DONTNEED. There is no = need >> to check again in madvise_dontneed_free() if the behavior is any >> different. >=20 > So what. The check is free. Compiler should be clever enough to = eliminate > the additional check. If there's a new MADV_DONTNEED flavour, the = change > would have to be effectively reverted. >=20 > NAK. I hate bikeshedding, but I will take the bait, since I see no reason for this NAK. I do not know what future change you have in mind in which quietly failing in madvise_dontneed_free() would be the right behavior. If the current code is presumed to be more =E2=80=9Crobust=E2=80=9D = against future changes since there is an additional check, I would argue that this is not the case: failing silently on a code-path that should never run is not the right thing to do. Having redundant checks that are not documented as such do not make the code more readable or maintainable. Having said that, if you want, I can turn this condition into WARN_ON_ONCE() or VM_BUG_ON(), although I really see no reason to do so. [ You might just as well add a default statement to the switch in madvise_behavior(), which BTW would have been much more reasonable, but only if it does not fail silently as the one we discuss. ] Note that I made this change not out of boredom, but because I needed to change this piece of code later for TLB batching. I did not want to sneak this change in another patch or to leave this confusing code. Anyhow, I wasted enough time on this trivial patch.