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 6B0D7C7619A for ; Tue, 11 Apr 2023 22:31:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B2A83900003; Tue, 11 Apr 2023 18:31:41 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AB3E1900002; Tue, 11 Apr 2023 18:31:41 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 95432900003; Tue, 11 Apr 2023 18:31:41 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 7EF7D900002 for ; Tue, 11 Apr 2023 18:31:41 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 49ED71A05BB for ; Tue, 11 Apr 2023 22:31:41 +0000 (UTC) X-FDA: 80670558402.13.C4E8ACA Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by imf24.hostedemail.com (Postfix) with ESMTP id 1D88D180003 for ; Tue, 11 Apr 2023 22:31:38 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=devkernel.io header.s=fm3 header.b=TR7c6bvd; dkim=pass header.d=messagingengine.com header.s=fm3 header.b="igb/tx05"; spf=pass (imf24.hostedemail.com: domain of shr@devkernel.io designates 66.111.4.28 as permitted sender) smtp.mailfrom=shr@devkernel.io; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1681252299; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=/C0dh5AAS6LZy77yzp081bIaCHH24Kxd//814piP+ok=; b=bL8V2KDalCH/cWQCKmwTQs+KE/tQFpAEjEFsWh/Lo92pIPySBZasXMu+TNvdIW7Aekv1Oo pvzXm1+Me7CwFCl0tzBaa9SXTVOmhXukOLQyDXGjZiON+YU0zZ/NvloC0Ik8ARCAIA/o+1 4PnRq/2gZkp5jK0MWDx295XYUcM6uak= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=devkernel.io header.s=fm3 header.b=TR7c6bvd; dkim=pass header.d=messagingengine.com header.s=fm3 header.b="igb/tx05"; spf=pass (imf24.hostedemail.com: domain of shr@devkernel.io designates 66.111.4.28 as permitted sender) smtp.mailfrom=shr@devkernel.io; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681252299; a=rsa-sha256; cv=none; b=W3t7pmaoCgQQIMZ6Bb8I2MEjDG7ad+kZ1i6LhyUAz7XeF4D+zi+dqlo8Rare5oywTimWEd +PdXIIWZGFaVKGvgXTZRKm5vTZtg9nnNthIAdgzhKxEyGySmAxduKgRuYZXGJXAsVgkcJ8 /X045VQ3coYjcF3Z6VJ7iVq6eWBrQHI= Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 66D295C011D; Tue, 11 Apr 2023 18:31:38 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Tue, 11 Apr 2023 18:31:38 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=devkernel.io; h= cc:cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm3; t=1681252298; x=1681338698; bh=/C 0dh5AAS6LZy77yzp081bIaCHH24Kxd//814piP+ok=; b=TR7c6bvdxVpQpF2pkY TWbowemVqK9Yo7wlXyYaRtBte8wy1LG8TvnkKFqR/jwMWs62mlOCO8bKPrYfMx9G rX2uLjVUUARIOSjVbkrBk/nrY20ckB/2b+sCGoQ4eddibj5rj7k3AhA8fj5o5Hth W7VHyhNJVrfv2atjaqygNij5+luw4b5HiZ9mwj8oT33Li/8SjbGS72InN0PBcxtu QZ2VI09D/FJI7mazgwn4fTaWaUhwtNKdSSw619h4KsT4mKwOI0JypTwU+/wm4EDp Eqs0K5asWCoojqyltrf2rIHDV5M2R3g6KpG8uRtXIYSsOnhi+Hos84DdWrXofV96 FgQw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; t=1681252298; x=1681338698; bh=/C0dh5AAS6LZy 77yzp081bIaCHH24Kxd//814piP+ok=; b=igb/tx05Wrpw63U/EA2a+j+wNfeCV RTTD+5VMfMzZjlLGQ8o3UxhCx4F+XBhAo7tcjyU4v1Wkdy+AkntABcJEbfV1hEz8 Q1b2XGAp84xJEbgQV17dkbe4MYbuj+oMAsU3rHLOZ6kDculAMX99ECjCyrxCKZgS A+B07FLTXl/W2udamdmWR/X/RY/x+t9VQHDCQuZRdEQtuCnjGeN7k726HT+FM45c QizG52Ads9QPSDekNFZQyke6ERhNJgLALv8VlTfG7pNMn2lEKdnwAUjjkXwR432O LxoSOS/rDiI82Lnqj4ey+3CWxlmEis8xVbBptVvWkAYgFv1zRzhHXl6yg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrvdekhedgudduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfhgfhffvvefuffgjkfggtgesthdtredttdertdenucfhrhhomhepufhtvghf rghnucftohgvshgthhcuoehshhhrseguvghvkhgvrhhnvghlrdhioheqnecuggftrfgrth htvghrnhepveelgffghfehudeitdehjeevhedthfetvdfhledutedvgeeikeeggefgudeg uedtnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepsh hhrhesuggvvhhkvghrnhgvlhdrihho X-ME-Proxy: Feedback-ID: i84614614:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 11 Apr 2023 18:31:36 -0400 (EDT) References: <20230406165339.1017597-1-shr@devkernel.io> <20230406165339.1017597-3-shr@devkernel.io> User-agent: mu4e 1.6.11; emacs 28.2.50 From: Stefan Roesch To: David Hildenbrand Cc: 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, hannes@cmpxchg.org, Bagas Sanjaya Subject: Re: [PATCH v5 2/3] mm: add new KSM process and sysfs knobs Date: Tue, 11 Apr 2023 15:29:59 -0700 In-reply-to: Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-Rspam-User: X-Rspamd-Server: rspam03 X-Stat-Signature: j519s8cwwemfsngqtopd8heehapem35x X-Rspamd-Queue-Id: 1D88D180003 X-HE-Tag: 1681252298-638493 X-HE-Meta: U2FsdGVkX1/n7SU8XVre9tqbU3p2T1MK4q+G/ZSElvjn0dNr+Oo8NIslcIxAnry9NXINmdtkARIKAvZe7ouZqYkMQfjNrIKO11A66CXacZ5GbMUyW9jlePRrUxFur08LSs22ZS0D6WezFOTTMfuBaOuagPt01DenhViYIe0WM41RxqNb8iSawHmgc1hT6T85PqlBbdLtRvYn2yFkyjt9+7637CEzwLLSQ3ULSz+mHPitUIVZvfXnTVD2jl7kogHl9tAAuJ3TtnoeIbx5AK4klvmy4L/SnMeItOl1IOOMQjy6/5UCFhtPLFhQM670PqSBX8pWjSXLMZGzbWGacOYhGW5Fk8LMvw4s38o5PC6ZfOMIbLr6zxnSEwJ3LYPaSbQQJcdGabK7CWWHpUyJYA4VdDISnu1OE/C0yMa0s6OKGTNg27JZzqCYdrq2cqxL9xvqniFQlo+kZEb2uEH/n95XFTcDzx2LHimiECQmuMncYNzII00JIrimUaNEk0wdHjopF+4JL0ieUrmGe5Yh0dRcaagIiahvHJHLz8fayd51b6Y+tpYvxQL5DUq6iFGmIl3K0SHrQy3L1Mav1vvs7lQUEQZVaBrtXcgfJ5Sbtyp8dzlyFdEnxR8HvM5D25D0rthzkTvx2ys+VWakqJtgHBr07+hOYtEySDvrvv2tmWK9+b9DMOMpAA+yXzxRRW5GVdFDW0zYXyLXxDDh6+7PcirCAfVzbrrki1G9Llhohbmxegg7A/CZ+0p4PAxx7atdcWES7wimYBjf7BvhM7eLqK2wz7N175ausdpecu4H8m79p+bEpOWbeY53FCjHPCN43TeRuun3z3ej+C3RAkn6DUnqjezkm447LFRdcQpuiJdg7hNcsjAN8aNczuYT4eUDYYTLaL75ITzbl11Aj1pXJKuHVTgqn0qgohFV64Onrctc9cpNaQv00bIyPnNzc8hRAQDDaOIyT0QZYKyPJ/YRtW3 kcI0Z2Dy LlkNXUTpfq5eaRn4v82geYkrmZQOM3Z1txWNdr0NAcOhwCxwlrtsQre6AeVDqZJhzKKDdNa4BmdzNp3GeUwybrH+9AAkayyMgJo4omH4d/n/D09UtFdO5Dk5Cf0Oxtq+0JVF0mjMDP4L/BWCza0UIu3qfhiRUfcHq70WMqdEpupemEgPHqFu1FDWs7RFGYQxPDrRy9qusdDEWuL2iBN/kC+71InaP9kKKTIt6XNeR/ouQHrLqXFizlgX7FYgFU5yNDnOL39Q8PbN+7a71EFxVKtL5IAex527qWhPOKUdOthD8SHfJW1CMNUt5rp3cqjmjA0V3Jx9OAk3u0WiqUdwamZmrXGNc63TamBFRZ8o8ruyK/+yyU0fH9qa6yKv4eK773Ao3w+bro1r3nyXGplJWfQERMjLZ37k2lYRbrCgcMoFk+H8= 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: David Hildenbrand writes: > On 06.04.23 18:53, Stefan Roesch wrote: >> This adds the general_profit KSM sysfs knob and the process profit metric >> and process merge type knobs to ksm_stat. >> 1) expose general_profit metric >> The documentation mentions a general profit metric, however this >> metric is not calculated. In addition the formula depends on the size >> of internal structures, which makes it more difficult for an >> administrator to make the calculation. Adding the metric for a better >> user experience. >> 2) document general_profit sysfs knob >> 3) calculate ksm process profit metric >> The ksm documentation mentions the process profit metric and how to >> calculate it. This adds the calculation of the metric. >> 4) add ksm_merge_type() function >> This adds the ksm_merge_type function. The function returns the >> merge type for the process. For madvise it returns "madvise", for >> prctl it returns "process" and otherwise it returns "none". > > I'm curious, why exactly is this change required in this context? It might be > sufficient to observe if the prctl is set for a process. If not, the ksm stats > can reveal whether KSM is still active for that process -> madvise. > > For your use case, I'd assume it's pretty unnecessary to expose that. > > If there is no compelling reason, I'd suggest to drop this and limit this patch > to exposing the general/per-mm profit, which I can understand why it's desirable > when fine-tuning a workload. > > > [...] > In the next version, the ksm_merge_type function() is removed. > >> Signed-off-by: Stefan Roesch >> Reviewed-by: Bagas Sanjaya >> Cc: David Hildenbrand >> Cc: Johannes Weiner >> Cc: Michal Hocko >> Cc: Rik van Riel >> Signed-off-by: Andrew Morton >> --- >> Documentation/ABI/testing/sysfs-kernel-mm-ksm | 8 +++++ >> Documentation/admin-guide/mm/ksm.rst | 8 ++++- >> fs/proc/base.c | 5 +++ >> include/linux/ksm.h | 5 +++ >> mm/ksm.c | 32 +++++++++++++++++++ >> 5 files changed, 57 insertions(+), 1 deletion(-) >> diff --git a/Documentation/ABI/testing/sysfs-kernel-mm-ksm >> b/Documentation/ABI/testing/sysfs-kernel-mm-ksm >> index d244674a9480..7768e90f7a8f 100644 >> --- a/Documentation/ABI/testing/sysfs-kernel-mm-ksm >> +++ b/Documentation/ABI/testing/sysfs-kernel-mm-ksm >> @@ -51,3 +51,11 @@ Description: Control merging pages across different NUMA nodes. >> When it is set to 0 only pages from the same node are merged, >> otherwise pages from all nodes can be merged together (default). >> + >> +What: /sys/kernel/mm/ksm/general_profit >> +Date: January 2023 > > ^ N > Updated in the next version. >> +KernelVersion: 6.1 > > ^ Outdated > Updated in the next version. > (kind of weird having to come up with the right numbers before getting it > merged) > > [...] > >> diff --git a/fs/proc/base.c b/fs/proc/base.c >> index 07463ad4a70a..c74450318e05 100644 >> --- a/fs/proc/base.c >> +++ b/fs/proc/base.c >> @@ -96,6 +96,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include "internal.h" >> #include "fd.h" >> @@ -3199,6 +3200,7 @@ static int proc_pid_ksm_merging_pages(struct seq_file *m, struct pid_namespace * >> return 0; >> } >> + > > ^ unrelated change > Fixed in the next version. >> static int proc_pid_ksm_stat(struct seq_file *m, struct pid_namespace *ns, >> struct pid *pid, struct task_struct *task) >> { >> @@ -3208,6 +3210,9 @@ static int proc_pid_ksm_stat(struct seq_file *m, struct pid_namespace *ns, >> if (mm) { >> seq_printf(m, "ksm_rmap_items %lu\n", mm->ksm_rmap_items); >> seq_printf(m, "zero_pages_sharing %lu\n", mm->ksm_zero_pages_sharing); >> + seq_printf(m, "ksm_merging_pages %lu\n", mm->ksm_merging_pages); >> + seq_printf(m, "ksm_merge_type %s\n", ksm_merge_type(mm)); >> + seq_printf(m, "ksm_process_profit %ld\n", ksm_process_profit(mm)); >> mmput(mm); >> } >> diff --git a/include/linux/ksm.h b/include/linux/ksm.h >> index c65455bf124c..4c32f9bca723 100644 >> --- a/include/linux/ksm.h >> +++ b/include/linux/ksm.h >> @@ -60,6 +60,11 @@ struct page *ksm_might_need_to_copy(struct page *page, >> void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc); >> void folio_migrate_ksm(struct folio *newfolio, struct folio *folio); >> +#ifdef CONFIG_PROC_FS >> +long ksm_process_profit(struct mm_struct *); >> +const char *ksm_merge_type(struct mm_struct *mm); >> +#endif /* CONFIG_PROC_FS */ >> + >> #else /* !CONFIG_KSM */ >> static inline int ksm_add_mm(struct mm_struct *mm) >> diff --git a/mm/ksm.c b/mm/ksm.c >> index ab95ae0f9def..76b10ff840ac 100644 >> --- a/mm/ksm.c >> +++ b/mm/ksm.c >> @@ -3042,6 +3042,25 @@ static void wait_while_offlining(void) >> } >> #endif /* CONFIG_MEMORY_HOTREMOVE */ >> +#ifdef CONFIG_PROC_FS >> +long ksm_process_profit(struct mm_struct *mm) >> +{ >> + return (long)mm->ksm_merging_pages * PAGE_SIZE - > > Do we really need the cast to long? mm->ksm_merging_pages is defined as > "unsigned long". Just like "ksm_pages_sharing" below. > Removed the cast in the next version. >> + mm->ksm_rmap_items * sizeof(struct ksm_rmap_item); >> +} >> + >> +/* Return merge type name as string. */ >> +const char *ksm_merge_type(struct mm_struct *mm) >> +{ >> + if (test_bit(MMF_VM_MERGE_ANY, &mm->flags)) >> + return "process"; >> + else if (test_bit(MMF_VM_MERGEABLE, &mm->flags)) >> + return "madvise"; >> + else >> + return "none"; >> +} >> +#endif /* CONFIG_PROC_FS */ >> + > > Apart from these nits, LGTM (again, I don't see why the merge type should belong > into this patch, and why there is a real need to expose it like that). > > Acked-by: David Hildenbrand