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=-8.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, 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 C20B9C56202 for ; Thu, 26 Nov 2020 13:47:37 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 2588C2087C for ; Thu, 26 Nov 2020 13:47:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=amazon.com header.i=@amazon.com header.b="ZDwPC6to" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2588C2087C Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=amazon.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 9131A6B0070; Thu, 26 Nov 2020 08:47:36 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 89C3F6B0075; Thu, 26 Nov 2020 08:47:36 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 73E556B007E; Thu, 26 Nov 2020 08:47:36 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0084.hostedemail.com [216.40.44.84]) by kanga.kvack.org (Postfix) with ESMTP id 57BB06B0070 for ; Thu, 26 Nov 2020 08:47:36 -0500 (EST) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 2492E181AEF2A for ; Thu, 26 Nov 2020 13:47:36 +0000 (UTC) X-FDA: 77526696912.23.sugar04_5510dab2737f Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin23.hostedemail.com (Postfix) with ESMTP id F3F8A37606 for ; Thu, 26 Nov 2020 13:47:35 +0000 (UTC) X-HE-Tag: sugar04_5510dab2737f X-Filterd-Recvd-Size: 7445 Received: from smtp-fw-9101.amazon.com (smtp-fw-9101.amazon.com [207.171.184.25]) by imf03.hostedemail.com (Postfix) with ESMTP for ; Thu, 26 Nov 2020 13:47:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazon201209; t=1606398455; x=1637934455; h=from:to:cc:subject:date:message-id:in-reply-to: mime-version; bh=fyAnVHSM7DUGAUg/UQTCA37wZ5eyuf7HfQz0TTccKaA=; b=ZDwPC6toUlr2p06BL/5mNLsbk3Ug9+ADTBJkFgJk2r1SuhVlN8hSLiez XiZzPhTiPr4CbHCLYkpnpzTIfSbdH8kuENmRM2R022cYUEIZLnI6ElWlY WsxPTenWXiR8U4zOhMIuszXT88oKkJNfly1KrQztDLWskkGOrf5KlGle7 U=; X-IronPort-AV: E=Sophos;i="5.78,372,1599523200"; d="scan'208";a="91197811" Received: from sea32-co-svc-lb4-vlan3.sea.corp.amazon.com (HELO email-inbound-relay-2b-c7131dcf.us-west-2.amazon.com) ([10.47.23.38]) by smtp-border-fw-out-9101.sea19.amazon.com with ESMTP; 26 Nov 2020 13:46:15 +0000 Received: from EX13D31EUA001.ant.amazon.com (pdx1-ws-svc-p6-lb9-vlan2.pdx.amazon.com [10.236.137.194]) by email-inbound-relay-2b-c7131dcf.us-west-2.amazon.com (Postfix) with ESMTPS id 8C108A1F84; Thu, 26 Nov 2020 13:46:12 +0000 (UTC) Received: from u3f2cd687b01c55.ant.amazon.com (10.43.161.237) by EX13D31EUA001.ant.amazon.com (10.43.165.15) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Thu, 26 Nov 2020 13:45:55 +0000 From: SeongJae Park To: Shakeel Butt CC: SeongJae Park , SeongJae Park , , Andrea Arcangeli , , , , , , Brendan Higgins , Qian Cai , Colin Ian King , Jonathan Corbet , "David Hildenbrand" , , Marco Elver , "Du, Fan" , , "Greg Thelen" , Ian Rogers , , "Kirill A. Shutemov" , Mark Rutland , Mel Gorman , Minchan Kim , Ingo Molnar , , "Peter Zijlstra (Intel)" , Randy Dunlap , Rik van Riel , David Rientjes , Steven Rostedt , Mike Rapoport , , Shuah Khan , , , Vlastimil Babka , Vladimir Davydov , Yang Shi , Huang Ying , , , Linux MM , , LKML Subject: Re: [PATCH v22 10/18] mm/damon: Implement a debugfs-based user space interface Date: Thu, 26 Nov 2020 14:45:39 +0100 Message-ID: <20201126134539.5974-1-sjpark@amazon.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [10.43.161.237] X-ClientProxiedBy: EX13D42UWA001.ant.amazon.com (10.43.160.153) To EX13D31EUA001.ant.amazon.com (10.43.165.15) 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 Wed, 25 Nov 2020 07:30:36 -0800 Shakeel Butt wrote: > On Tue, Oct 20, 2020 at 2:06 AM SeongJae Park wrote: > > > > From: SeongJae Park > > > > DAMON is designed to be used by kernel space code such as the memory > > management subsystems, and therefore it provides only kernel space API. > > That said, letting the user space control DAMON could provide some > > benefits to them. For example, it will allow user space to analyze > > their specific workloads and make their own special optimizations. > > > > For such cases, this commit implements a simple DAMON application kernel > > module, namely 'damon-dbgfs', which merely wraps the DAMON api and > > exports those to the user space via the debugfs. > > > > 'damon-dbgfs' exports three files, ``attrs``, ``target_ids``, and > > ``monitor_on`` under its debugfs directory, ``/damon/``. > >> [...] > > +/** > > + * damon_nr_running_ctxs() - Return number of currently running contexts. > > + */ > > +int damon_nr_running_ctxs(void) > > +{ > > + int nr_ctxs; > > + > > + mutex_lock(&damon_lock); > > + nr_ctxs = nr_running_ctxs; > > + mutex_unlock(&damon_lock); > > + > > READ_ONCE() instead of mutex? Right, it would be ok and even make the code slightly faster. But, if you're ok, I'd like to keep this as is because this helps reader easily find what variables are protected by the mutex and this is not performance critical. > > > + return nr_ctxs; > > +} > > + [...] > > + > > +static ssize_t dbgfs_target_ids_write(struct file *file, > > + const char __user *buf, size_t count, loff_t *ppos) > > +{ > > + struct damon_ctx *ctx = file->private_data; > > + char *kbuf, *nrs; > > + bool received_pidfds = false; > > + unsigned long *targets; > > + ssize_t nr_targets; > > + ssize_t ret = count; > > + int i; > > + int err; > > + > > + kbuf = user_input_str(buf, count, ppos); > > + if (IS_ERR(kbuf)) > > + return PTR_ERR(kbuf); > > + > > + nrs = kbuf; > > + > > + if (!strncmp(kbuf, "pidfd ", 6)) { > > + received_pidfds = true; > > I am inclining towards having simple pids instead of pidfds. Basically > what cgroup/resctrl does. Ok, I will drop the pidfd support for simplicity. Restoring it back when real requirement comes out would not be too late. > > > > + nrs = &kbuf[6]; > > + } > > + > > + targets = str_to_target_ids(nrs, ret, &nr_targets); > > + if (!targets) { > > + ret = -ENOMEM; > > + goto out; > > + } > > + > > + if (received_pidfds) { > > + for (i = 0; i < nr_targets; i++) > > + targets[i] = (unsigned long)damon_get_pidfd_pid( > > + (unsigned int)targets[i]); > > + } else if (targetid_is_pid(ctx)) { > > + for (i = 0; i < nr_targets; i++) > > + targets[i] = (unsigned long)find_get_pid( > > + (int)targets[i]); > > + } > > + > > + mutex_lock(&ctx->kdamond_lock); > > + if (ctx->kdamond) { > > + ret = -EINVAL; > > + goto unlock_out; > > + } > > + > > + err = damon_set_targets(ctx, targets, nr_targets); > > Hmm this is leaking the references to the previous targets. 'damon_set_targets()' frees the previous targets itself, so we don't leak. > > > + if (err) > > + ret = err; > > +unlock_out: > > + mutex_unlock(&ctx->kdamond_lock); > > + kfree(targets); > > +out: > > + kfree(kbuf); > > + return ret; > > +} > > + > > Still looking. Looking forward your comments! Thanks, SeongJae Park