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 33AC0C433EF for ; Wed, 23 Feb 2022 07:11:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C3FF18D0003; Wed, 23 Feb 2022 02:11:31 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id BC7F18D0001; Wed, 23 Feb 2022 02:11:31 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A42918D0003; Wed, 23 Feb 2022 02:11:31 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0189.hostedemail.com [216.40.44.189]) by kanga.kvack.org (Postfix) with ESMTP id 947A98D0001 for ; Wed, 23 Feb 2022 02:11:31 -0500 (EST) Received: from smtpin20.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 47FC9181C990D for ; Wed, 23 Feb 2022 07:11:31 +0000 (UTC) X-FDA: 79173173982.20.8C917CC Received: from mail-pl1-f170.google.com (mail-pl1-f170.google.com [209.85.214.170]) by imf19.hostedemail.com (Postfix) with ESMTP id D3DC61A0002 for ; Wed, 23 Feb 2022 07:11:30 +0000 (UTC) Received: by mail-pl1-f170.google.com with SMTP id m11so14281803pls.5 for ; Tue, 22 Feb 2022 23:11:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ajou.ac.kr; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=eO4P8gNctIGm1EeUOiCxmIMQcDwPflV40V22xmuGQ1Y=; b=bBctnH/MZFlR4I0JUfB6Gqn3rQP8GWwrFKhCihvfRsVSV26vqP50YSur5zSj4Afa1U G7RTlvytoyBoMpOXekUbGCyvVn2KJJ3h20lzQr7OvBCrQH3VTscSKnOSuI+qyocYqyb+ BTTKn+JeimFJLev56fTuW5vM+66C4No9wMjCs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=eO4P8gNctIGm1EeUOiCxmIMQcDwPflV40V22xmuGQ1Y=; b=2qjN6AwMw+mMSx4iBwm/QoOdP/37o+uEWv3A3+HOHDfB5HAACn6mt8JgKrOHE8cNRc STe7xxqfUjRkJEk1jCYRgcgGKzSatERboDxHmUCgsmwBxjTePGnbNqyIekGls9CjmE3D jexI0CNtyVgdE+hIfpBWAwoDI1L0ANZbLEq5QmMqMG/40AHv0qmofotJlprmmxxtcqGZ J/R/S41bM9Twcwg+jHyWeErUxcIiKRQs33pdgRfT3lVUnieJJKWYXCmkVKrOIzEQpGFZ wmgYtlW5GMC3fuMQIFsP6E+Bv0Py/aQbEKlSseTRXShqdZumSJzf6blPaDHEMGzMkwc0 fo+Q== X-Gm-Message-State: AOAM530lg/l+UMjpCvsNMEEgaQUfjCd9yD2kzJg9XNgXsAZRV7C+mEzu bEOmdljwLihgwqTGZvYYNTq/Hg== X-Google-Smtp-Source: ABdhPJxxlJCmHoQ7UMp15+G8GXj41mXtG9w/xA5wheg6qRT0hvnTW3/qEjdvFqQjmDjErIbqyz/BHQ== X-Received: by 2002:a17:90a:664b:b0:1bc:72df:54e4 with SMTP id f11-20020a17090a664b00b001bc72df54e4mr6776354pjm.10.1645600289667; Tue, 22 Feb 2022 23:11:29 -0800 (PST) Received: from swarm08 ([210.107.197.32]) by smtp.gmail.com with ESMTPSA id k16sm20903338pfu.149.2022.02.22.23.11.25 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Tue, 22 Feb 2022 23:11:29 -0800 (PST) Date: Wed, 23 Feb 2022 16:11:23 +0900 From: Jonghyeon Kim To: SeongJae Park Cc: Jonghyeon Kim , Jonathan.Cameron@Huawei.com, amit@kernel.org, benh@kernel.crashing.org, corbet@lwn.net, david@redhat.com, dwmw@amazon.com, elver@google.com, foersleo@amazon.de, gthelen@google.com, markubo@amazon.de, rientjes@google.com, shakeelb@google.com, shuah@kernel.org, linux-damon@amazon.com, linux-mm@kvack.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH v1 2/3] mm/damon/core: Add damon_start_one() Message-ID: <20220223071123.GB6270@swarm08> References: <20220218102611.31895-3-tome01@ajou.ac.kr> <20220222095328.7962-1-sj@kernel.org> <20220223051113.GA3535@swarm08> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220223051113.GA3535@swarm08> User-Agent: Mutt/1.9.4 (2018-02-28) X-Rspamd-Queue-Id: D3DC61A0002 X-Stat-Signature: 8wmfwwk819iai4f3ccmtc7mjo8n88ybi Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=ajou.ac.kr header.s=google header.b="bBctnH/M"; dmarc=pass (policy=reject) header.from=ajou.ac.kr; spf=pass (imf19.hostedemail.com: domain of tome01@ajou.ac.kr designates 209.85.214.170 as permitted sender) smtp.mailfrom=tome01@ajou.ac.kr X-Rspam-User: X-Rspamd-Server: rspam11 X-HE-Tag: 1645600290-897905 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, Feb 23, 2022 at 02:11:13PM +0900, Jonghyeon Kim wrote: > On Tue, Feb 22, 2022 at 09:53:28AM +0000, SeongJae Park wrote: > > Hello Jonghyeon, > > > > On Fri, 18 Feb 2022 19:26:10 +0900 Jonghyeon Kim wrote: > > > > > damon_start() function is designed to start multiple damon monitoring > > > contexts. But, sometimes we need to start monitoring one context. > > > Although __damon_start() could be considered to start for one monitoring > > > context, it seems reasonable to adopt a new function that does not need > > > to handle 'damon_lock' from the caller. > > > > > > Signed-off-by: Jonghyeon Kim > > > --- > > > include/linux/damon.h | 1 + > > > mm/damon/core.c | 25 +++++++++++++++++++++++++ > > > 2 files changed, 26 insertions(+) > > > > > > diff --git a/include/linux/damon.h b/include/linux/damon.h > > > index c0adf1566603..069577477662 100644 > > > --- a/include/linux/damon.h > > > +++ b/include/linux/damon.h > > > @@ -511,6 +511,7 @@ int damon_register_ops(struct damon_operations *ops); > > > int damon_select_ops(struct damon_ctx *ctx, enum damon_ops_id id); > > > > > > int damon_start(struct damon_ctx **ctxs, int nr_ctxs); > > > +int damon_start_one(struct damon_ctx *ctx); > > > int damon_stop(struct damon_ctx **ctxs, int nr_ctxs); > > > > > > #endif /* CONFIG_DAMON */ > > > diff --git a/mm/damon/core.c b/mm/damon/core.c > > > index 290c9c0535ee..e43f138a3489 100644 > > > --- a/mm/damon/core.c > > > +++ b/mm/damon/core.c > > > @@ -466,6 +466,31 @@ int damon_start(struct damon_ctx **ctxs, int nr_ctxs) > > > return err; > > > } > > > > > > +/** > > > + * damon_start_one() - Starts the monitorings for one context. > > > + * @ctx: monitoring context > > > + * > > > + * This function starts one monitoring thread for only one monitoring context > > > + * handling damon_lock. > > > + * > > > + * Return: 0 on success, negative error code otherwise. > > > + */ > > > +int damon_start_one(struct damon_ctx *ctx) > > > +{ > > > + int err = 0; > > > + > > > + mutex_lock(&damon_lock); > > > + err = __damon_start(ctx); > > > + if (err) { > > > + mutex_unlock(&damon_lock); > > > + return err; > > > + } > > > + nr_running_ctxs++; > > > + mutex_unlock(&damon_lock); > > > + > > > + return err; > > > +} > > > + > > > > IMHO, this looks like an unnecessary duplication of code. Unless this is > > needed for a real usecase, this change might unnecessarily make the code only a > > little bit more complicated. And to my understanding of the next patch, this > > is not really needed for this patchset. I will left comments on the patch. If > > I'm missing something, please clarify why this is really needed. > > > > Furthermore, damon_start() starts a set of DAMON contexts in exclusive manner, > > to ensure there will be no interference. This patch breaks the assumption. > > That is, contexts that started with damon_start() could be interfered by other > > contexts that started with damon_start_one(). I have a plan to make > > damon_start() also work for non-exclusive contexts group[1], though. > > > > [1] https://lore.kernel.org/linux-mm/20220217161938.8874-3-sj@kernel.org/ > > > > > > Thanks, > > SJ > > > > I understand your opinion, and it makes sense. I will drop this patch in the > next version. > > > Thanks, > Jonghyeon