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 55007C7EE2A for ; Mon, 23 Jun 2025 18:03:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DC34B6B00C1; Mon, 23 Jun 2025 14:03:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D9A6C6B00C3; Mon, 23 Jun 2025 14:03:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CD7316B00C5; Mon, 23 Jun 2025 14:03:11 -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 BC6AC6B00C1 for ; Mon, 23 Jun 2025 14:03:11 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 68A3B1A0B48 for ; Mon, 23 Jun 2025 18:03:11 +0000 (UTC) X-FDA: 83587436982.28.EA5FC58 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf08.hostedemail.com (Postfix) with ESMTP id BA1D9160018 for ; Mon, 23 Jun 2025 18:03:09 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=j02PsBsS; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf08.hostedemail.com: domain of sj@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=sj@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1750701789; a=rsa-sha256; cv=none; b=WK5gXAU5gxFvyXS5xlINZzK61cAj7rB3645TtUrAdlepONsr40ODdxPuc4cN4ITMl0c3+h AUFDareFoTsXq9ugm36PVJTqONiY393hBKs1BT1La1FcAqZP+Lyi/hZxG3Ja8zROftBBjU +J8sM8QYr+sTkAYtW/skrmDpEdzUrao= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=j02PsBsS; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf08.hostedemail.com: domain of sj@kernel.org designates 147.75.193.91 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=1750701789; 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=CEudLT7AuRAI4Zb5QPMpMaAWjTXA3hjpgtHauMk33gQ=; b=t8VnSyp9T6/n2OHLQmCfTrQKv7vUDV8VH0PhMJ3ZSxTxDZ9iJcNvkKzGGHUScxRX9HoIa8 LZddDypF0tr4SaIlY62ntdykMA5UuSRWfZYVJZxm79gqhlBtEzDpG3m58jLJfOhq5hjHMr 8dPDGIAx1yYjNnVIXV2mCFi2Hc7+M1M= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 21C6BA500BB; Mon, 23 Jun 2025 18:03:09 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9F11CC4CEED; Mon, 23 Jun 2025 18:03:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1750701788; bh=EjUF144md/K781GFtJ0hEHO/hvbh5XE87mCjkxYGsM4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=j02PsBsSaVPCzjELCgDieddIpt0pdnoOjIxmyGeZAvs/r7x4OYLYgOvE6sMpn+xWt xk6LVdi+inRRAswpM01XXa3T6rhDFdkMlf8GWXZid6ot5KofzN0W4XGmWyS380ioIK 77fmYZ294fBMuwUa9wZjFxaUlsK6L4CunyiC3pJG0Z1kiN78zcToaO5YDCxkW766jt c9IpwYzCfBAGYJPSXKYWcdlolPDmX/McQsWpbfM0K/ggjFxj4vW2xe5+MOn2kf+NYE KjzNw9uz/2ADujapJpUdmozbOL7BnZHujUjgXhlvpp4aDwBVldz7sbtV7FaFn7yxtr pA3F6D4u/sTUA== From: SeongJae Park To: Honggyu Kim Cc: SeongJae Park , kernel_team@skhynix.com, damon@lists.linux.dev, Andrew Morton , linux-mm@kvack.org, Yunjeong Mun Subject: Re: [PATCH 1/3] mm/damon: do not allow creating zero size region Date: Mon, 23 Jun 2025 11:03:05 -0700 Message-Id: <20250623180305.44277-1-sj@kernel.org> X-Mailer: git-send-email 2.39.5 In-Reply-To: <86e91607-f008-4fc1-8660-021b8e257a10@sk.com> References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Queue-Id: BA1D9160018 X-Rspamd-Server: rspam10 X-Stat-Signature: r8xu57oqk3n6b9ras7ibsz4cy3wyuk88 X-HE-Tag: 1750701789-875817 X-HE-Meta: U2FsdGVkX1/sP4n2FTSZ2F7WpNlF9g9nnTqpddDhcE1ZDUKh5gUsHefBPjhIVnpZ31s4rdt+DSELo+ejjswtMDQ02euXAgz2Dm6/TWhmNpfKbh1lP+SmcJnLioAqt6gf9+E+uK9oBv0/bjKLk0eY+fi14zmvzYqX1fH/3ugpHEjU2k1ZGjwv46BSOiwi0OeGEY7LKgvutxjU3Sevb3aW2mwzMLySA8M8h2AX/tylXkQ9sU7yezhum51NkPe4nwzF36kpvDGvHvQN1nHqBOKUF7lCNCc6neAyV1eCyhF5RIQWaB2IqMksbYGqUKg9yApQ64b+XH5A1mtqWzKaBw2JXgCk8JIKkD+vu5zgicCpZqj15bdxHXu6DOawdCY5uQxreomYOINQ2QtSpmNZ2r87xRzG+7tMH5jw9iqj6sPVswXVL4RIJUd6j6Ljm0ZoTl3K9QU2Vi+TlKOWSVVu1mcpH+6CDqKFpgjzPOT3YmHHdOJ6weS/D7xNGvbYvEgb40I+mV+8hKgE1onDGzOoBMHOgb12Rm2IOrmUfYJFGOw6t9A4sf1BTH8td8guFiiofgIKrA8knHYLNUThEGEUbdBgcuBg8vPXljnOyeH1XuxBk3SxAVryqeZIbnmzRKZK9iPaFK/IBYG9SA4IdxZvaJvxoxsiPApgTrEVqbVQeX6kFVPytivvSqwafs8SNwsBHHHAOlysU8r+oPbQowVzwMCyFYMBfoAg6gtPxsIPh4jQpej+2Ldy1+DDIYIBzkSLy7IeRpc+IKUrnqER5RAYKefXiVHUD6Qv1rGDxl3O9Lkc9zDlhQ0MzwPPs8ZCSDcIkDmXsU5rSo9q8c+aQOjfGqZqX/mRvCS3kO6g9IQN3p6NnB30ALKcf+/ScwgSUOeHsx28Ou7FxElqJ9gVOjr0CqK/H1n48d7w+Cpu4YAkJrX9qL8zMFNWf0owmfyLl1Kv8XYwUXScDmf8x8twnjtBZ+E AAa3b2nq JxWRnfl6dvhJlMYeUROQ7oyeNoPTaKZy+B+xaUCmo5FCoutAkn9jyIaDcJL7P88IwyE+xCq0nXfGOPt3MsLrYfSsdHnnhwzSUJGF35Ed1OiFae+VTfW9MFSRvmro2Kn//h/Emaz3oxmYwn0egPBLi/sCSLPvVdPcMmY5y3QQmgDJKl/JCwhbKAUTGGoESZZgF95zo2d/0Dm6+v+/rG0RkZpsK1yHhsJbajaEF+nLNfT+byp9PmglzqhHr0g80oeLwif3IrkYMfCW4Q2FdYLkGACldF7cVs3IMgWCJ9HA/6xDQECd7NRM0pQhJOg== 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: On Mon, 23 Jun 2025 11:58:53 +0900 Honggyu Kim wrote: > Hi SeongJae, > > Thanks for the kind review as always! My pleasure :) > > On 6/23/2025 1:04 AM, SeongJae Park wrote: > > On Sun, 22 Jun 2025 21:09:23 +0900 Honggyu Kim wrote: > > > >> Creating zero size region leads a divide by zero error inside > >> damon_get_intervals_score() as follows. > >> > >> static unsigned long damon_get_intervals_score(struct damon_ctx *c) > >> { > >> struct damon_target *t; > >> struct damon_region *r; > >> unsigned long sz_region, max_access_events = 0, access_events = 0; > >> unsigned long target_access_events; > >> unsigned long goal_bp = c->attrs.intervals_goal.access_bp; > >> > >> damon_for_each_target(t, c) { > >> damon_for_each_region(r, t) { > >> sz_region = damon_sz_region(r); > >> max_access_events += sz_region * c->attrs.aggr_samples; > >> access_events += sz_region * r->nr_accesses; > >> } > >> } > >> target_access_events = max_access_events * goal_bp / 10000; > >> return access_events * 10000 / target_access_events; /* divide by zero! */ > >> } > > > > Thank you for finding this issue! Coudl you please further share how zero size > > region can be made, and if user-space can make the situation? > > The initial values of node*_start_addr and node*_end_addr inside > /sys/module/mtier/parameters/ are all zeros so I saw this problem easily by > setting Y to "enabled". Thank you for clarifying! Please add this description on the commit message if you send a next version of this patch, with Fixes: tag. > > >> > >> This patch makes a NULL return for such cases when creating a region > >> inside damon_new_region(). > > > > I agree zero size region could look silly. But I don't really think it should > > be prohibited. What about modifying damon_get_intervals_score() instead? > > Maybe we can set target_access_events as 1 in this case. > > Hmm... I don't get what you mean by setting "target_access_events" to 1 for such > cases. Could you please explain more? I mean, something like below. @@ -1449,6 +1449,7 @@ static unsigned long damon_get_intervals_score(struct damon_ctx *c) } } target_access_events = max_access_events * goal_bp / 10000; + target_access_events = target_access_events ? : 1; return access_events * 10000 / target_access_events; } > > > > >> > >> Signed-off-by: Honggyu Kim > >> Cc: Yunjeong Mun > >> --- > >> mm/damon/core.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/mm/damon/core.c b/mm/damon/core.c > >> index b217e0120e09..44740da337fd 100644 > >> --- a/mm/damon/core.c > >> +++ b/mm/damon/core.c > >> @@ -126,8 +126,11 @@ struct damon_region *damon_new_region(unsigned long start, unsigned long end) > >> if (!region) > >> return NULL; > >> > >> + if (start == end) > >> + return NULL; > If you're okay, then I'd like to modify this as follows again. > > + if (start >= end) > + return NULL; > > What do you think about this? I still prefer fixing the found bug on the spot. I don't think having zero or negative size regions is really somewhat we always prohibit. Thanks, SJ [...]