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=-18.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 B0218C636CA for ; Wed, 21 Jul 2021 16:13:03 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 42F8E608FC for ; Wed, 21 Jul 2021 16:13:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 42F8E608FC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id BDF356B0081; Wed, 21 Jul 2021 12:13:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B91266B0088; Wed, 21 Jul 2021 12:13:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A7E876B0089; Wed, 21 Jul 2021 12:13:02 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0115.hostedemail.com [216.40.44.115]) by kanga.kvack.org (Postfix) with ESMTP id 8DB046B0081 for ; Wed, 21 Jul 2021 12:13:02 -0400 (EDT) Received: from smtpin19.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 2FF54250C9 for ; Wed, 21 Jul 2021 16:13:02 +0000 (UTC) X-FDA: 78387089004.19.9A090C9 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf09.hostedemail.com (Postfix) with ESMTP id B39FB3002917 for ; Wed, 21 Jul 2021 16:13:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1626883981; h=from:from: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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=q+1x1mENMFxc8VLP/jpqaTH0FYhxbACKoXkinVL26Wc=; b=ZHyTF170NfcPOTX8NolJl1xAFPyUHgfQFTO4w96hQE2WINm8rkkQmcdANofR2fzBWyZS72 5ulPu6B30q8ZgSqBnrB8nZlNHmWl12Usykobs/CDV2V5HDu8xpBFM3/o9Xp5ZaX3IjqMZU 33Eu4DQCRQ2uUZd1jXmf+eOriXncIeM= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-452-oC65bOCdN1CDhuFoD1Hm-Q-1; Wed, 21 Jul 2021 12:12:59 -0400 X-MC-Unique: oC65bOCdN1CDhuFoD1Hm-Q-1 Received: by mail-wr1-f71.google.com with SMTP id m4-20020adffa040000b02901404c442853so1200236wrr.12 for ; Wed, 21 Jul 2021 09:12:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=q+1x1mENMFxc8VLP/jpqaTH0FYhxbACKoXkinVL26Wc=; b=DmdQLHHVTnuDrsehCmVy2fB4XAXUGTcX4yr/FPFKWFn2Bom0WU2S1DH2kEsaflqQS3 QlJkfxDFnnxJqD4D/aPG3Fqs/vfsmVCvwakJsMtR9OEL0vSbxHuM6wo88M5nMfjxzG/i kFAsr2Lct5OuGtnR1n5sQPS/z/I3bTKvu2KO6MqS3AMapDo90v0NsjkP9coaQi8kOL+B R7GEx5Cnnd5SaDDrEzSIR0AfCdMVIBepk/vnT1ARudOA6DTxm9VBD6eFH1CKeN01f555 0w9qXv2PKS733OtMkPOURDP5a+YoduJqqY33D0DXpkUzi77hk75hG4MDtGbgkIWrjWAL gMxA== X-Gm-Message-State: AOAM531InLkyss7pU70s0CTE5bp4XB6uvxROIi6vxkJlTmmR0/rKuXAY wukkza+0qyesuOp3AV7xLoIj6BTl0GhP2L4fqnNryQV42Cl60VRNVqpbDpDtygEFOAoXAukwAOY WAIsmnwYB9uY= X-Received: by 2002:a05:600c:4108:: with SMTP id j8mr4782859wmi.67.1626883978719; Wed, 21 Jul 2021 09:12:58 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzBad4MtATTGEezDpg/5UCIzeBsvG9+jv9Q5/Fp96JqgG0VxamWxXA/f1dTpJs61LbL4pViDg== X-Received: by 2002:a05:600c:4108:: with SMTP id j8mr4782827wmi.67.1626883978487; Wed, 21 Jul 2021 09:12:58 -0700 (PDT) Received: from [192.168.3.132] (p5b0c65c3.dip0.t-ipconnect.de. [91.12.101.195]) by smtp.gmail.com with ESMTPSA id d8sm28546989wra.41.2021.07.21.09.12.57 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 21 Jul 2021 09:12:58 -0700 (PDT) Subject: Re: [PATCH v2 1/3] mm, oom: move task_will_free_mem up in the file to be used in process_mrelease To: Suren Baghdasaryan Cc: Andrew Morton , Michal Hocko , Michal Hocko , David Rientjes , Matthew Wilcox , Johannes Weiner , Roman Gushchin , Rik van Riel , Minchan Kim , Christian Brauner , Christoph Hellwig , Oleg Nesterov , Jann Horn , Shakeel Butt , Andy Lutomirski , Christian Brauner , Florian Weimer , Jan Engelhardt , Tim Murray , Linux API , linux-mm , LKML , kernel-team References: <20210718214134.2619099-1-surenb@google.com> <7eb17da6-03a6-5eaf-16e6-97b53ba163d8@redhat.com> <20210720160707.2332738708948f7d865d67c1@linux-foundation.org> From: David Hildenbrand Organization: Red Hat Message-ID: <0ee6775b-589c-3243-1c01-aafad5eecb73@redhat.com> Date: Wed, 21 Jul 2021 18:12:56 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: B39FB3002917 X-Stat-Signature: z98kqe4rhdqipa7ainib8ajp131cg9ne Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=ZHyTF170; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf09.hostedemail.com: domain of david@redhat.com has no SPF policy when checking 170.10.133.124) smtp.mailfrom=david@redhat.com X-HE-Tag: 1626883981-212325 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 21.07.21 17:33, Suren Baghdasaryan wrote: > On Wed, Jul 21, 2021 at 12:30 AM David Hildenbrand wrote: >> >> On 21.07.21 01:07, Andrew Morton wrote: >>> On Tue, 20 Jul 2021 14:43:52 +0200 David Hildenbrand wrote: >>> >>>> On 18.07.21 23:41, Suren Baghdasaryan wrote: >>>>> process_mrelease needs to be added in the CONFIG_MMU-dependent block which >>>>> comes before __task_will_free_mem and task_will_free_mem. Move these >>>>> functions before this block so that new process_mrelease syscall can use >>>>> them. >>>>> >>>>> Signed-off-by: Suren Baghdasaryan >>>>> --- >>>>> changes in v2: >>>>> - Fixed build error when CONFIG_MMU=n, reported by kernel test robot. This >>>>> required moving task_will_free_mem implemented in the first patch >>>>> - Renamed process_reap to process_mrelease, per majority of votes >>>>> - Replaced "dying process" with "process which was sent a SIGKILL signal" in >>>>> the manual page text, per Florian Weimer >>>>> - Added ERRORS section in the manual page text >>>>> - Resolved conflicts in syscall numbers caused by the new memfd_secret syscall >>>>> - Separated boilerplate code wiring-up the new syscall into a separate patch >>>>> to facilitate the review process >>>>> >>>>> mm/oom_kill.c | 150 +++++++++++++++++++++++++------------------------- >>>>> 1 file changed, 75 insertions(+), 75 deletions(-) >>>> >>>> TBH, I really dislike this move as it makes git blame a lot harder with >>>> any real benefit. >>>> >>>> Can't you just use prototypes to avoid the move for now in patch #2? >>>> >>>> static bool task_will_free_mem(struct task_struct *task); >>> >>> This change makes the code better - it's silly to be adding forward >>> declarations just because the functions are in the wrong place. >> >> I'd really love to learn what "better" here means and if it's rather >> subjective. When it comes to navigating the code, we do have established >> tools for that (ctags), and personally I couldn't care less where >> exactly in a file the code is located. >> >> Sure, ending up with a forward-declaration for every function might not >> be what we want ;) >> >>> >>> If that messes up git-blame then let's come up with better tooling >>> rather than suffering poorer kernel code because the tools aren't doing >>> what we want of them. Surely? >> >> I don't agree that what we get is "poorer kernel code" in this very >> instance; I can understand that we avoid forward-declarations when >> moving smallish functions. But moving two functions with 75 LOC is a bit >> too much for my taste at least -- speaking as someone who cares about >> easy backports and git-blame. > > There is a third alternative here to have process_mrelease() at the > end of the file with its own #ifdef CONFIG_MMU block, maybe even > embedded in the function like this: > > int process_mrelease(int pidfd, unsigned int flags) > { > #ifdef CONFIG_MMU > ... > #else > return ENOSYS; > #endif > } > > This would not require moving other functions. > Would that be better than the current approach or the forward declaration? IMHO that could be an easy, possible alternative. -- Thanks, David / dhildenb