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=-10.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham 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 EE0D5C48BDF for ; Thu, 24 Jun 2021 10:26:37 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 6582F61248 for ; Thu, 24 Jun 2021 10:26:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6582F61248 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 38E4C6B0036; Thu, 24 Jun 2021 06:26:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 33E516B005D; Thu, 24 Jun 2021 06:26:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 191088D0001; Thu, 24 Jun 2021 06:26:36 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0008.hostedemail.com [216.40.44.8]) by kanga.kvack.org (Postfix) with ESMTP id CC7AD6B0036 for ; Thu, 24 Jun 2021 06:26:35 -0400 (EDT) Received: from smtpin31.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id ED8531F845 for ; Thu, 24 Jun 2021 10:26:35 +0000 (UTC) X-FDA: 78288238350.31.827643B Received: from mail-qv1-f43.google.com (mail-qv1-f43.google.com [209.85.219.43]) by imf11.hostedemail.com (Postfix) with ESMTP id A9CDB200107C for ; Thu, 24 Jun 2021 10:26:35 +0000 (UTC) Received: by mail-qv1-f43.google.com with SMTP id m15so2989959qvc.9 for ; Thu, 24 Jun 2021 03:26:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to; bh=QHA5XL2z4ba2bDn1SI+fh6o/LaSMQqQKLJBCxBSck3k=; b=d+6hp2rCz8MMoOIaPRLxD+bKt/A+bFeWEVYlH8fTzLrId/ZyzWjGcjDN89QSKscmG1 YBZCU2GBs9vX6b07WZUzJKMjJ0+RqAxuOVJ0hme1hgo4DiVtdeHqJXqoYSIbvLCMLItu +FcFSESoe0wQM3bH00jN28XrTL9OQEDOuqelyFJg1T7zFu0wB0+bQvjyCXuHv6Zo590p +MR3ARY8r/DbQYF5v9s241Di1YF4vP66E0OWkPrqdEiE40taj7fJOwmk+YGcaXajQtKZ IXCvGooivBqY+zdW0bCLEs1I1/JFAKMaA8U6QzgFHuJLh04TjY+512Q/ecIiceJFEJ5x FPPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to; bh=QHA5XL2z4ba2bDn1SI+fh6o/LaSMQqQKLJBCxBSck3k=; b=fFOCuXwV8/u8PbBsBh8AD3t/LzDmjaqABalY8bWKOu/mgfnHQ/vGvtfegV8ltviaRF jAfudpebiukWVdu4LMZrD8AEM/FS1XGPA4qVXoP3JJhmOQJCEaWtNuDjFY8lo2gCv35U IsUQosxwsUdXbnmT30lR01W3p9cPoIHEMHxS1nCN43YWqKOEWFjA7nA61qUAB/9HnLGT 9r8q0YKvPMDuEomEvi23hAVglOPFVZg3KBrX5gdfbfDG+SkoNl4EOIzJzfHi83G/6XO5 jnSD0qmucE2/vkAIwysHSR5787bhgscTy6cxHXiAQd2U6mgp/HHDhY+ZjpWapVt5nHE1 SNlg== X-Gm-Message-State: AOAM530JH5aM0fT7Dx3eOoRHmgZJmUlLsGClXC2sSsmSMqEdQim9HwN/ RqZSiHkuihngyRp2jgiZbhY= X-Google-Smtp-Source: ABdhPJyzOCBhWMvw4oIaH/BVd+umviU6q7Sjs64gDoJ6+2Nqsa5FiJU6cAX0UNWNwI1r9VwbGIvK4A== X-Received: by 2002:ad4:5343:: with SMTP id v3mr4639008qvs.45.1624530395013; Thu, 24 Jun 2021 03:26:35 -0700 (PDT) Received: from localhost.localdomain (ec2-35-169-212-159.compute-1.amazonaws.com. [35.169.212.159]) by smtp.gmail.com with ESMTPSA id w195sm2132842qkb.127.2021.06.24.03.26.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Jun 2021 03:26:34 -0700 (PDT) From: SeongJae Park X-Google-Original-From: SeongJae Park To: Shakeel Butt Cc: SeongJae Park , SeongJae Park , Jonathan.Cameron@huawei.com, acme@kernel.org, alexander.shishkin@linux.intel.com, amit@kernel.org, benh@kernel.crashing.org, Brendan Higgins , Jonathan Corbet , David Hildenbrand , dwmw@amazon.com, Marco Elver , "Du, Fan" , foersleo@amazon.de, greg@kroah.com, Greg Thelen , guoju.fgj@alibaba-inc.com, jgowans@amazon.com, Mel Gorman , mheyne@amazon.de, Minchan Kim , Ingo Molnar , namhyung@kernel.org, "Peter Zijlstra (Intel)" , Rik van Riel , David Rientjes , Steven Rostedt , Mike Rapoport , Shuah Khan , sieberf@amazon.com, snu@zelle79.org, Vlastimil Babka , Vladimir Davydov , zgf574564920@gmail.com, linux-damon@amazon.com, Linux MM , linux-doc@vger.kernel.org, LKML Subject: Re: [PATCH v31 01/13] mm: Introduce Data Access MONitor (DAMON) Date: Thu, 24 Jun 2021 10:26:19 +0000 Message-Id: <20210624102623.24563-1-sjpark@amazon.de> X-Mailer: git-send-email 2.17.1 In-Reply-To: Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=gmail.com header.s=20161025 header.b=d+6hp2rC; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf11.hostedemail.com: domain of sj38park@gmail.com designates 209.85.219.43 as permitted sender) smtp.mailfrom=sj38park@gmail.com X-Rspamd-Server: rspam02 X-Stat-Signature: myhxorr73huhppu6fabare871xnubuwa X-Rspamd-Queue-Id: A9CDB200107C X-HE-Tag: 1624530395-397998 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: From: SeongJae Park On Tue, 22 Jun 2021 07:59:11 -0700 Shakeel Butt wrote: > On Mon, Jun 21, 2021 at 1:31 AM SeongJae Park wrote: > > > > From: SeongJae Park > > > > DAMON is a data access monitoring framework for the Linux kernel. The > > core mechanisms of DAMON make it > > > > - accurate (the monitoring output is useful enough for DRAM level > > performance-centric memory management; It might be inappropriate for > > CPU cache levels, though), > > - light-weight (the monitoring overhead is normally low enough to be > > applied online), and > > - scalable (the upper-bound of the overhead is in constant range > > regardless of the size of target workloads). > > > > Using this framework, hence, we can easily write efficient kernel space > > data access monitoring applications. For example, the kernel's memory > > management mechanisms can make advanced decisions using this. > > Experimental data access aware optimization works that incurring high > > access monitoring overhead could again be implemented on top of this. > > > > Due to its simple and flexible interface, providing user space interface > > would be also easy. Then, user space users who have some special > > workloads can write personalized applications for better understanding > > and optimizations of their workloads and systems. > > > > === > > > > Nevertheless, this commit is defining and implementing only basic access > > check part without the overhead-accuracy handling core logic. The basic > > access check is as below. > > > > The output of DAMON says what memory regions are how frequently accessed > > for a given duration. The resolution of the access frequency is > > controlled by setting ``sampling interval`` and ``aggregation > > interval``. In detail, DAMON checks access to each page per ``sampling > > interval`` and aggregates the results. In other words, counts the > > number of the accesses to each region. After each ``aggregation > > interval`` passes, DAMON calls callback functions that previously > > registered by users so that users can read the aggregated results and > > then clears the results. This can be described in below simple > > pseudo-code:: > > > > init() > > while monitoring_on: > > for page in monitoring_target: > > if accessed(page): > > nr_accesses[page] += 1 > > if time() % aggregation_interval == 0: > > for callback in user_registered_callbacks: > > callback(monitoring_target, nr_accesses) > > for page in monitoring_target: > > nr_accesses[page] = 0 > > if time() % update_interval == 0: > > regions_update_interval? It used the name before. But, I changed the name in this way to use it as a general periodic updates of monitoring primitives. Of course we can use the specific name only in this specific example, but also want to make this as similar to the actual code as possible. If you strongly want to rename this, please feel free to let me know. > > > update() > > sleep(sampling interval) > > > > The target regions constructed at the beginning of the monitoring and > > updated after each ``regions_update_interval``, because the target > > regions could be dynamically changed (e.g., mmap() or memory hotplug). > > The monitoring overhead of this mechanism will arbitrarily increase as > > the size of the target workload grows. > > > > The basic monitoring primitives for actual access check and dynamic > > target regions construction aren't in the core part of DAMON. Instead, > > it allows users to implement their own primitives that are optimized for > > their use case and configure DAMON to use those. In other words, users > > cannot use current version of DAMON without some additional works. > > > > Following commits will implement the core mechanisms for the > > overhead-accuracy control and default primitives implementations. > > > > Signed-off-by: SeongJae Park > > Reviewed-by: Leonard Foerster > > Reviewed-by: Fernand Sieber > > Few nits below otherwise look good to me. You can add: > > Acked-by: Shakeel Butt Thank you! > > [...] > > +/* > > + * __damon_start() - Starts monitoring with given context. > > + * @ctx: monitoring context > > + * > > + * This function should be called while damon_lock is hold. > > + * > > + * Return: 0 on success, negative error code otherwise. > > + */ > > +static int __damon_start(struct damon_ctx *ctx) > > +{ > > + int err = -EBUSY; > > + > > + mutex_lock(&ctx->kdamond_lock); > > + if (!ctx->kdamond) { > > + err = 0; > > + ctx->kdamond_stop = false; > > + ctx->kdamond = kthread_create(kdamond_fn, ctx, "kdamond.%d", > > + nr_running_ctxs); > > + if (IS_ERR(ctx->kdamond)) > > + err = PTR_ERR(ctx->kdamond); > > + else > > + wake_up_process(ctx->kdamond); > > Nit: You can use kthread_run() here. Ok, I will use that from the next spin. > > > + } > > + mutex_unlock(&ctx->kdamond_lock); > > + > > + return err; > > +} > > + > [...] > > +static int __damon_stop(struct damon_ctx *ctx) > > +{ > > + mutex_lock(&ctx->kdamond_lock); > > + if (ctx->kdamond) { > > + ctx->kdamond_stop = true; > > + mutex_unlock(&ctx->kdamond_lock); > > + while (damon_kdamond_running(ctx)) > > + usleep_range(ctx->sample_interval, > > + ctx->sample_interval * 2); > > Any reason to not use kthread_stop() here? Using 'kthread_stop()' here will make the code much simpler. But, 'kdamond' also stops itself when all monitoring targets became invalid (e.g., all monitoring target processes terminated). However, 'kthread_stop()' is not easy to be used for the use case (self stopping). It's of course possible, but it would make the code longer. That's why I use 'kdamond_stop' flag here. So, I'd like leave this as is. If you think 'kthread_stop()' should be used, please feel free to let me know. Thanks, SeongJae Park