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 D0555C76196 for ; Thu, 6 Apr 2023 14:32:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3D52C6B0071; Thu, 6 Apr 2023 10:32:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 386446B0074; Thu, 6 Apr 2023 10:32:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 24CCB6B0075; Thu, 6 Apr 2023 10:32:46 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 15E086B0071 for ; Thu, 6 Apr 2023 10:32:46 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id C7BC7121368 for ; Thu, 6 Apr 2023 14:32:45 +0000 (UTC) X-FDA: 80651207490.14.3769D8E Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf18.hostedemail.com (Postfix) with ESMTP id 55AC41C0029 for ; Thu, 6 Apr 2023 14:32:43 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=TWQsejWT; spf=pass (imf18.hostedemail.com: domain of david@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=david@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680791563; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=j895ymwzo5cFUK4EnI2MaN8LhhF7WBFR4yyDeDEaR/E=; b=pQHxQE7zsiKmEN4p+uOgrHe6LSnZ72cg8b8DPra8pWONwus2DAkKpwVPvpEDua0LHnvSsx AkBycgAZR5gHv81l1Sa3Pg7Rybz47L/FmZiYUfX232o356J5JAXHEjMEII9ZFeiD+3FoIE UjppQ3hUhB7wk1fAyX7YWAQbyaCR+pU= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=TWQsejWT; spf=pass (imf18.hostedemail.com: domain of david@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=david@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680791563; a=rsa-sha256; cv=none; b=Rijgv0vqq5+riJeAMqYW82bSbeHbPEJqN7JrHxJwinytonIYColV44BQ0S5Iberm5NOcvp o/nfEPA5gEg73kqH55meFDjo1S0D0hRQYMhQKuXw6N8ZEHH7PurUvJIcJrLLpBOixsa+cE v/vO3Uz+1t6J936OVKP7riq0X9dDPrU= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1680791562; 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=j895ymwzo5cFUK4EnI2MaN8LhhF7WBFR4yyDeDEaR/E=; b=TWQsejWTt+X4LwOwcCTD8MuuUhuaWdyxEE7WxoFrO+qMa1yMM2ioqgQ06fQpQRl3hjoh0d qTq2rNo4xN3k0s9JMfBf22pUcP6KNqYnNF/3iHaQ11vlCjp3eP2fpM3wGWOm+1+IK7n5wM F6Ze+/RFA4mAHmnvsi03XDa1F7Rtfw0= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-120-EhRu3v0rO2ycfPA1lTNRyQ-1; Thu, 06 Apr 2023 10:32:41 -0400 X-MC-Unique: EhRu3v0rO2ycfPA1lTNRyQ-1 Received: by mail-wr1-f69.google.com with SMTP id v30-20020adfa1de000000b002d557ec6d15so4972132wrv.18 for ; Thu, 06 Apr 2023 07:32:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680791560; x=1683383560; h=content-transfer-encoding:in-reply-to:subject:organization:from :references:cc:to:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=j895ymwzo5cFUK4EnI2MaN8LhhF7WBFR4yyDeDEaR/E=; b=BYHil8Z2uqyH5t4YggYqMKvvrQEbGTairM0+EYRNY2xm9Ng2jPLDi2AIZqO6mHg9sl SAK2UH7vuCcVZ8A5ZLbRXBOku2Lvjxy0IAWweemkkB5+51ddqXe0lHS8BtkMN5wSu1zG R/cCG/3kZq+tcWbvRos0dGPU3JGcWOFw2P5hPc0a0k4hEcCXzkXdug0e+0i3481AVxa5 cALYtB3z7RTCt89g0daxN0zn2joDJkYvh8ZQbNb1dxzYKXpUsRYbC9kbLYNn9HNBUZrO fu8kPhwURTTxMMNA11o6thMlIPmEpQEi62/jkKuAJ4Ejt4tYYFBeeqOLbM+nQEsMUpcD gapA== X-Gm-Message-State: AAQBX9dLQXa/T2fhs0rJihje7FIBcsGYL3YO8Y4St7wVkuvkk/tZroaw /495mnkibYP/gwsWUAUr06ixkhFXg9SrwVpLCTzlx7CFfFMJBYWACqvBOQmAfMDvOk81p/f/eQv WL6VIS9YCH6c= X-Received: by 2002:adf:e807:0:b0:2db:9ccf:f9f5 with SMTP id o7-20020adfe807000000b002db9ccff9f5mr7281889wrm.0.1680791560106; Thu, 06 Apr 2023 07:32:40 -0700 (PDT) X-Google-Smtp-Source: AKy350ZQFqd68J323693RzSjFbAIGhMIx+Sk7+HCdTARcTO1jXA82EQy191Q5dTyk14QuMR9BHz7KQ== X-Received: by 2002:adf:e807:0:b0:2db:9ccf:f9f5 with SMTP id o7-20020adfe807000000b002db9ccff9f5mr7281865wrm.0.1680791559754; Thu, 06 Apr 2023 07:32:39 -0700 (PDT) Received: from ?IPV6:2a09:80c0:192:0:5dac:bf3d:c41:c3e7? ([2a09:80c0:192:0:5dac:bf3d:c41:c3e7]) by smtp.gmail.com with ESMTPSA id a15-20020a5d4d4f000000b002cfe685bfd6sm1881580wru.108.2023.04.06.07.32.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 06 Apr 2023 07:32:38 -0700 (PDT) Message-ID: Date: Thu, 6 Apr 2023 16:32:37 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.1 To: Johannes Weiner Cc: Stefan Roesch , kernel-team@fb.com, linux-mm@kvack.org, riel@surriel.com, mhocko@suse.com, linux-kselftest@vger.kernel.org, linux-doc@vger.kernel.org, akpm@linux-foundation.org, Bagas Sanjaya References: <20230310182851.2579138-1-shr@devkernel.io> <20230310182851.2579138-3-shr@devkernel.io> <20230406141619.GB35884@cmpxchg.org> From: David Hildenbrand Organization: Red Hat Subject: Re: [PATCH v4 2/3] mm: add new KSM process and sysfs knobs In-Reply-To: <20230406141619.GB35884@cmpxchg.org> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 55AC41C0029 X-Stat-Signature: 4t3bcqqphesm4yzn1dm3o4jk6huogwwt X-HE-Tag: 1680791563-308492 X-HE-Meta: U2FsdGVkX19yyl1g0o6q4nGOz+enITojX7p6cY7KRp3Udjb7NV78dZSxFRtToG1Yt7C6z7FZMQ1EZoJxwHo4lFJ76fLzyL6RCL13xkTeaQTYlB3M4N7r8G7mvzzhc5HD9fB1zrqyKUnCC8m9S8LWGNx6cWeQvV1Igs+EJM24oAhBC4TSYyhkDtr0cC4VEJDiWnpfyUczgHqK4NlQDLhgClf3b3YLdI63dlcNtcZhTdFFQYsUqEN3Jbjmym0KPkaEz2BsrdSrrLeRN6AlMe5cSSEO/h6LH7yWgmF0t6oNQ9covxCX5EDzcp/sBXCvETEHcz1zvnNRADhdBgy6J2i3j1jRFIGWwnOhRcHIVMtMrG46XPIIuSZD90o3fwNO1fGzh15g1WFuJmE2zmBbDkA/LL7+6dTlHeuIcOpRxD62KmrKSlhonlW0C2vVhviIBcLzMI/tllbhDPYhelUzLCetLGJQZE+yNTTLgFc5PgTyYi67xhv93Irch3Y4QRxxZhyDwVrSHJKCzskwTMj0SwGbkxH7zYv9hmsU54xbiVoXf/+kWA3hw4FDQr+ceOBNneJwkRgeO7T+LKJyYka+KomOQ3VOiJ0B2cdeLElX7/eZWkA++bVFPDizkVxqz+cETK+Rf/2E8BBSVgMbNsBNQs5Jp5jo1JHo4Bph6bA0jLRAo96EZFN2e3Ttz8UP/7VB7YddrK4EqXm/nNMc7JWNDoiMk/a9yefd/Yy0FUf+UmIZ1vlmMaE3RkTr0exZBXoC2QICWRKHGgbbEyY5e8tLibiwoejTlcQWFVu2Mn/+uyVAzDj3pptn2FUv6nXnKzP+d9qsTWGGZH1WRVTadhb2aGTkHEBBzGu33bUpODPTd3qik91aWnJpIYz6XBnEwjAfLXPpVLbM5TzQvJ1DQOKZ2N39Atz3H3omMlJPa4/7MgxEfOs5CIN3ewWHHd3q1XyIfv4q8J0D5h5Xy8zJQhTikrl 5XtjCw0r +GBOiQ6a+UP1ldyed6O+T69cKLJD/JI/wJM2KRTRn/FuIfSSyEMbwlHS09bjGZIbKZOGtEUqIWC5DPKtLYe4q0ZaBzTWAJms2Ceyz2j+LShNYXc/RaqkMVZIfNjwIezMw9spXr25o11H5sEq3twX83I3Y5eWcVc4P2NVZYaFyLuFSvyQJjrd9gzlkRVcGi3zjUBKwEVKasH48JuGb2VmQFqWdNiIXTd5Kosbfod3BXDo8EwKbuCyQuIMIRZSwYSdwF+aiEPOLS5fba9dduSVxVDde83uW1EMRK7K3iGuS5cGbzMFEfvdrmuA8S1TdDMU7XwwY9q+yTJbKwDj5nPRX3kSRiTQq38VLCkcF/aEDOe3KtcO9eoI6/D5TwU+FLnE0CeOL6eDuHHf1BvKM54ft9V2beOZmV6mE7rlOMhIhwJvZaSQ= 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 06.04.23 16:16, Johannes Weiner wrote: > On Thu, Apr 06, 2023 at 03:23:11PM +0200, David Hildenbrand wrote: >>>> >>>> Often, when you have to start making a list of things that a patch does, it >>>> might make sense to split some of the items into separate patches such that you >>>> can avoid lists and just explain in list-free text how the pieces in the patch >>>> fit together. >>>> >>>> I'd suggest splitting this patch into logical pieces. For example, separating >>>> the general profit calculation/exposure from the per-mm profit and the per-mm >>>> ksm type indication. >>>> >>> >>> Originally these were individual patches. If I recall correctly Johannes >>> Weiner wanted them as one patch. I can certainly split them again. >> >> That's why I remember that v1 contained more patches :) >> >> Again, just my opinion on patches that require a description in form of a >> list ... > > My concern was the initial splitting of patch 1. I found it easier to > review with the new prctl() being one logical change, and fully > implemented in one go. The changelog is still in the form of a list, > but it essentially enumerates diff hunks that make up the interface. > > No objection to splitting out the multiple sysfs knobs, though! > > The strategy I usually follow is this: > > 1. Split out changes based on user-visible behavior (new prctl(), new > sysfs knob) > > 2. Extract changes made along the way that have stand-alone value in > existing code (bug fixes, simplifying/documenting tricky sections, > cleanups). > > 3. Split out noisy prep work such as renames and refactors that make > the user-visible functional changes more difficult to understand. > > and then order the series into sections 2, 3, and 1. > I agree. The most important part is the "logical change" part. Once it's down to a list of 3 items or so for a single commit we can usually express it like (just an example that does no longer apply due to pages_volatile() not being required) the following when combining items 1+2+3 from the list: " Let's expose the general KSM profit via sysfs and document that new toggle. [add details about that and especially why we are doing that] As we need the number of volatile pages to calculate the general KSM profit, factor out existing functionality into a helper. " Much easier to read. -- Thanks, David / dhildenb