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 C5EBAC41513 for ; Thu, 16 May 2024 22:17:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 23E7A6B0085; Thu, 16 May 2024 18:17:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1EE996B0088; Thu, 16 May 2024 18:17:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0B6AE6B0089; Thu, 16 May 2024 18:17:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id E316B6B0085 for ; Thu, 16 May 2024 18:17:41 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 8863EA3056 for ; Thu, 16 May 2024 22:17:41 +0000 (UTC) X-FDA: 82125671922.14.728E952 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf03.hostedemail.com (Postfix) with ESMTP id D6F292000F for ; Thu, 16 May 2024 22:17:39 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=JxD8ae5I; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf03.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1715897860; 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-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=dkjRnCp8vJwWD8suhwux5n8gWwEBAVCtZYWZK2C5Vro=; b=f7PB4t5oBI8IpKqGgz+y6v+yrUDqKrzbZ4VFb7Jhnm4wuieJIl0giDm+p4Cb2HQw/YOlcF iljBEtMaBPfltfmR66clnaYzd2gPH+OyEFfvJ71x0SArBnTZqUUVkp9Pt6vznrrWC2XifW MnyYkvCPYxjuPx//xTD9HD7rkkwBBj4= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=JxD8ae5I; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf03.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1715897860; a=rsa-sha256; cv=none; b=wN6a9k06zI7M0yC9fiGxSAUa8KMgJnF3S4fwjg7rBwfVW1eCbzwNC4/yLo/NmU3Oq4U/2C Ebkl9J4meniGLJsoEZ/KTXualQ/JRYFBzmSbspeMS3qXkTyLojofOujqLzbAZM6F0BfuQ1 KEDNucEZF2OU6Auu8VfQTLv0d11C0ns= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id C917E61776; Thu, 16 May 2024 22:17:38 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 12B7FC113CC; Thu, 16 May 2024 22:17:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1715897858; bh=AAD7OIjCqFmLMcNQ5v7ono7dluP+5mcNBRp7Eg58oQo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=JxD8ae5IBngYzxe+Zyl4LQ9G5AqJ7yHFqIM0EvRpFImFiNOBflfEE1BrJBCbfWmr/ dOZ9/ZsN1TSpE2axXkQPYBz1kM7E8bWS9DvLuejr3+zMj0EcwQz2nq8N6/sMD7NIPi bpadvLwYHJsya+qjB9wwSvyEwU9WndwIpmlsR0ASUxIwDO6RUdYCMTFdYl+0Ki/B3Q efNMggAy0dZjNiHYl88RyEU7Pcs4m2izSe4x/OCVvYO/PplzryTP6qeVh8/BQUIARn QyB124mDs+KUtb7rvLCOruihnSEa7RZZdAPWeYG02okKDQuE7tRyRL1/AMyIFOf3hq US9p2HKQY3IHQ== From: SeongJae Park To: Alex Rusuf Cc: SeongJae Park , damon@lists.linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH v1 0/7] DAMON multiple contexts support Date: Thu, 16 May 2024 15:17:35 -0700 Message-Id: <20240516221735.82564-1-sj@kernel.org> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240515152457.603724-1-yorha.op@gmail.com> References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: D6F292000F X-Stat-Signature: zisqi3f1accr75ch4y6jsjg1bxxrxz8h X-HE-Tag: 1715897859-671377 X-HE-Meta: U2FsdGVkX19kFDt29ZBm0sLPFKZl9IJM6AgiSf04GIfHBiq8uLwI6vdosS7WnnUGgAgB27jYvwx1arcdgGYSzRDOye8kyayuqS92rAQKszCqZxg5r6kh237pw28MEMl3aUXFs6ddc9WDbmpCM2JlB8zM42K1OuWxRdV5+kFaewwf4z/1s001XCkjJKzaPnp5D8FHvwM0CvMctv8mQttDjFHInvuXUIusqV4fMFYGAOdwhls421j/JICu8JdRZerMa/qE/TDNb+sBG/VDA/PJf+cB5qyZVQMo6DdDakjmQuF4elWPccXLFob5JwAbjcbf5+zlJgZhm2kciY2NmmS/wAEAC1g5MvFuuo1oaAjFaAzWvvK8mA5STjOSD4YS3cGsbGCCqcBwhpSOlYJwSAowTruClr1AHhVSFPqTUTS/t1mqU1L5t+kfPDPLdR5LCHsUBMu3nQmQ12gvFPBLjK14Ip8uDSdmloXMBTYmSQNRUuTMyL5Cqtmsbt0gkujX09Rn98ENIUVG/zmQeRZWJhf+5cRa938JZKfviDlGyACpev1oLoT8dEKL13J114Ag9xCTR6tO3IiyS6gDSISffcWA9Q+LxVzxXxCku6Too039cCgaHR9xjbLqBLTj15bh4YX+s/pGtbQiF+9FQz2Ykg3rAoa0kpRV26Qo2s1sy4zRfBS20ZqA+LOV0HK8oPn1v/kJlxZI/2yeBJ2R0bZte0LB1Cuv02+9dHOTwPdv7XMNQ4bq5AfkU56MRdBa5r2P8qgOxu3Q0b4vSqRANThIenFgWvan4synYmqZA5TrL3hlJgbxEk/ltlNrY/pfFxwaa4fCf7RNoIAhc/3qSflPo/pdVEF/C+O9cCcPuiykYXGnAF7uTFiU1h7HX3INmb/PCE522VdDqamZacwzZAs95zktNz2C7pof9iio+epkNe30OSTMj8dzbseKt2TKUe6gIzJg8CY9RAbtOP8RPvx5m0H BqwQ/IPj 5xXoCQk4kZN03VcZW8dVCiLiCbEvMuX1W0gQc2O/+Y9iwoUkpBF584Q+taicAkoqWgpOXiZsMAiDnzAeI53Gtr35GmKtjFVBXYg3xzU42/6c+PAN/nZcJGRRUU0U7begZRg7YrwDex9/T3VxSZ/I2PFzIi7ByAnNV/FvcH5RvZpwLQ2rLpvCyOFfBu8DWEETBBnIuvcTINZ3veJKWlyOcVCPqc2zU0Vd3fEGubZBeO2h79+/ZBdCdXQ4D6+0mi3kN95x7FgvsVL08R1TZCdt1K/j8ci4CS4yurZcazXStPK+VwEk= 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: List-Subscribe: List-Unsubscribe: Hello Alex, Adding high level comments first. I will try to read each patch and add detailed comments to those as soon as I get some time. Also, please Cc linux-mm@ for DAMON patches. I'd also recommend cc-ing linux-kernel@. On Wed, 15 May 2024 18:24:50 +0300 Alex Rusuf wrote: > Currently kdamond uses only one context per kthread > and most of its time it sleeps, so utilizing several > contexts can scale kdamond and allow it to use > another set of operations. Thank you for this patchset. I believe this change is important for DAMON's long term vision. A quick question for a clarification and proper prioritization, though, since size of this patch series is not very tiny. Does this patch series is for your real usage? If so, could you please clarify your usage and how this patch series can help? > > This patch-set implements support for multiple contexts > per kdamond. > > In pseudo code previous versions worked like > the following: > > while (!kdamond_should_stop()) { > > /* prepare accesses for only 1 context */ > prepare_accesses(damon_context); > > sleep(sample_interval); > > /* check accesses for only 1 context */ > check_accesses(damon_context); > > ... > } > > With this patch kdamond workflow will look > like the following: > > while (!kdamond_shoule_stop()) { > > /* prepare accesses for all contexts in kdamond */ > damon_for_each_context(ctx, kdamond) > prepare_accesses(ctx); > > sleep(sample_interval); > > /* check_accesses for all contexts in kdamond */ > damon_for_each_context(ctx, kdamond) > check_accesses(ctx); > > ... > } The overall idea makes sense to me. > > To try this you can use modified kernel[1] and > damo[2]. I also have written few simple shell scripts[3] > to collect data for damo. > > [1] https://github.com/onlyoneofme/damon-multi-contexts.git > [2] https://github.com/onlyoneofme/damo/tree/multi-contexts Looking forward to the patch for DAMO be submitted, or PR-ed! > [3] https://github.com/onlyoneofme/damon-multi-contexts-tests.git Do you have a plan to integrate this into DAMON selftests or damon-tests? > > Alex Rusuf (7): > mm/damon/core: kdamond_struct abstraction layer Let's make the subjects clear what it does. For example, this patch's subject could be "add kdamonds_struct abstraction layer". Similar comment for other patches. Also, I think '_struct' suffix of 'kdamond_struct' is not really needed. Let's remove it if there is no special reason to add it. > mm/damon/core: list-based contexts organization I think this can be squashed into the first patch? If not, could you please let clarify? > mm/damon/lru_sort: kdamond_struct abstraction layer > mm/damon/reclaim: kdamon_struct abstraction layer Does these two patches mean lru_sort and reclaim are broken by the first patch? Let's keep everything unbroken in middle of the patchset, to help bisect. > mm/damon/core: rename nr_running_ctxs -> nr_running_kdamonds I think this would also better to be together with the first patch? I know this does not break something, but makes reading patch bit complex. > mm/damon/core: multi-context support > mm/damon/core: multi-context awarness for trace events I think these two patches should be squashed into one patch. Otherwise, the trace point is broken in the middle of the patch series, right? > > include/linux/damon.h | 48 +++- > include/trace/events/damon.h | 14 +- > mm/damon/core.c | 497 +++++++++++++++++++++-------------- > mm/damon/lru_sort.c | 31 ++- > mm/damon/modules-common.c | 35 ++- > mm/damon/modules-common.h | 3 +- > mm/damon/reclaim.c | 30 ++- > mm/damon/sysfs.c | 306 +++++++++++++-------- > 8 files changed, 629 insertions(+), 335 deletions(-) > > -- > 2.42.0 Thanks, SJ