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 8F59EC369C2 for ; Tue, 22 Apr 2025 16:55:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EA3086B0006; Tue, 22 Apr 2025 12:55:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E526B6B0008; Tue, 22 Apr 2025 12:55:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D192A6B000A; Tue, 22 Apr 2025 12:55:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id B58096B0006 for ; Tue, 22 Apr 2025 12:55:15 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 7C949C0B79 for ; Tue, 22 Apr 2025 16:55:16 +0000 (UTC) X-FDA: 83362280232.11.4EBF059 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf22.hostedemail.com (Postfix) with ESMTP id 0B85AC000A for ; Tue, 22 Apr 2025 16:55:14 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=h0OQQvme; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf22.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=1745340915; a=rsa-sha256; cv=none; b=PLAS/mbsQVAletj6vJnFDIA6WfkHcb0yy7LZWYjr4jqFBiq+skS+kEef7cTxjCuRh5sV4h cuV8EEX42vUdoVfW3JocgvMiS/olwGvjeo39eTCliNC1UpHDuE7WWo/DCwTynUjk48FPGZ lcmNDwYdQeh4B5HyzOPJv0Wu8oe71fM= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=h0OQQvme; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf22.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=1745340915; 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=88MDO4fJusMu8xK3htUFWAirOPYMU86Z7D0VOFG+tpc=; b=tiRXN02OSTAYIORCnYIuESROt5kPp+gLvl7gwHbWl4u63lSPx8TxF0nFWl4aKTC3kYBHE7 ej1wAz5IrFl3Nz48AQJ+p7bEWTP7beJYjGewwsv+ooieotsCvepyHZAA64bi4Hg0Ka7YfD bczqFTC3ntvs57C+BKsl2IQOmSctWlc= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id D71C85C5CE6; Tue, 22 Apr 2025 16:52:56 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6F801C4CEE9; Tue, 22 Apr 2025 16:55:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1745340913; bh=6NlG+4FrVHI1VglAibdakrhmpUdt9KakPG7b2mDPRz8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=h0OQQvmepvkbfYBH+fgcYuZYP5iy4e+LKpYVhOpM420cQv6RohfAo7lx/Ud+2d1N7 oJ+qUItt14wyIu5u9XL2yAyjiQ8Je2sbrKH0F4qBv+6w+QbcxKAsIP7/ZCUC4s5rMu I7W41s+tE0owYmpcTLAczZiwYMPFo3RrHEIkqJ7RFwa/YoL1wvqf7Ppn+jOSsIIdMv s2LtrdUB55acyccaEucijjB7rDPIou4PgN2Toikp/qsCKKbKCRLaUJ/MGQhprS0HJ7 L0YWMpwAsK/nkZ5gp4QVIJwkmqDLa/pX5FmBPjMvSBweLu3tTw0LvtYRpuTJqjWrVc S2XSVOoTdi2HQ== From: SeongJae Park To: Enze Li Cc: SeongJae Park , akpm@linux-foundation.org, damon@lists.linux.dev, linux-mm@kvack.org Subject: Re: [PATCH] mm/damon/core: Initialize sampling_addr in damon_new_region() Date: Tue, 22 Apr 2025 09:55:11 -0700 Message-Id: <20250422165511.58332-1-sj@kernel.org> X-Mailer: git-send-email 2.39.5 In-Reply-To: <87plh46gf1.fsf@kylinos.cn> References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 0B85AC000A X-Stat-Signature: gigkfhunp7i5xmgtmd1te5r94bwx3ax5 X-Rspam-User: X-HE-Tag: 1745340914-385131 X-HE-Meta: U2FsdGVkX190jPNxcl2WyYqfoyfBSLDiWGTcqhcZ6nnhlWInEjLJ6lhYNuMLUu/Iqk5UebkrmDJXbxlsbKAgFJ2yUEd209d/fbv9cFrOHHemRdVJcBUImAG6LHtONEVE3209JIDqGOOFj3YsYLxqgtxScmEYnVwu4E4P7PU1sokinFXtW5zQL3CkeuzoV7Ypd/PAQdkIMcVCAO6C7OMClbRQ3J6d50HttJ8P4xfwGzbxN6P6Gwb1uwUTSrKVICXXwVhmS8JH/x/QS4y9JG/TSO9I1+baTjZMdsz7d3s535+3IioeeLrqYU89I8+TMkHFLP6XQGqx9TpRDyxWaH5MWPCaPkYoYKN7mRfXuo57n+IKVXurHngpwtrBYP0WoUwNh0cjuaie4gthesZ4QTLPK44dZV9MDkoitVWLFcHaMcgU/md9UPN/MLOqXPWiVXqtLD6XhwHO2dDZoPCPKzJ/cObzozPLBuQLG3u0Yu3crSPIPjX8QjduIfgFWLjBMq61mTyLqJcj5pF+7AKDNDa3oyS/87EhtLVI4xoJhBC2Epq91U7OpUTpKBAgfqxBdCrsIPzauZdWEdFvIjBjiv8FQuti6qLeltSoVKLbhNZgCMeqpPg4adu3yS8KfPeuxqn8fO0W9qJlj7uDllOtqKFqsHcXAo9E53+tM5wC+iKz8TV28YxlOTZ+/PiN06Uh0ZX07z3+qdQ9U+h/LmYN9oCInkfFo7U3WfiZvgMptRWJTa5XKVnytJDCg2n6qLdXUhXVnwsjtM/BjW0BYEZp7Soaw+ouoOqvYvi1/9Bch6j+POucfi93hHBnq0H/Z1T5cn0w5a37o8+7EY6EwSCQKIVcfUYxB7s7/z5xpjNcOX4HSGz5coFcADzeXBXxgeNjOdD9LNd7zx6u3SG0deglE3YwVjgXBjjVJMQlNodwF1Bt+Wn6uGyR/1mIkGDDfo00VpCFF/3UZCwxYNYRBmBjECc kBBqIjMw ZGPq+0/YlrX3dLLY= 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 Tue, 22 Apr 2025 14:09:09 +0800 Enze Li wrote: > Hi SeongJae, > > Thanks for your review! When examining this function, I instinctively > verified the initialization of all member variables. Personally, I'm > quite partial to the practice of initializing all fields when creating > an object - though I fully acknowledge that initializing sampling_addr > may currently have no practical consequences. This patch serves purely > as a preventive measure, Thank you for kindly clarifying the context. > so please don't hesitate to drop it if you > consider it superfluous. Yes, I don't think we need this exact change, based on the details that you kindly shared. Thank you again for the sharing and generously understanding this decision. Nevertheless, I feel like maybe the documentations and comments could be improved to reduce this kind of confusion. If you find the room to improve, please feel free to submit a patch or suggest changes. FYI, please consider trimmed interleaved replies in future: https://docs.kernel.org/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions Thanks, SJ > > Best Regards, > Enze > > On Mon, Apr 21 2025 at 09:55:25 AM -0700, SeongJae Park wrote: > > On Mon, 21 Apr 2025 11:39:19 +0800 Enze Li wrote: > > > >> Since sampling_addr is used across vaddr and paddr modules, initialize > >> it in damon_new_region(). > >> > >> Signed-off-by: Enze Li > >> --- > >> mm/damon/core.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/mm/damon/core.c b/mm/damon/core.c > >> index f0c1676f0599..d197a5e3901c 100644 > >> --- a/mm/damon/core.c > >> +++ b/mm/damon/core.c > >> @@ -128,6 +128,7 @@ struct damon_region *damon_new_region(unsigned long start, unsigned long end) > >> > >> region->ar.start = start; > >> region->ar.end = end; > >> + region->sampling_addr = 0; > >> region->nr_accesses = 0; > >> region->nr_accesses_bp = 0; > >> INIT_LIST_HEAD(®ion->list); > > > > I unfortunately cannot find why this is required. Is there a use case that > > reads uninitialized sampling_addr or any problem that comes from the fact that > > damon_new_regions() is not initializing the field? > > > > Only operations set implementations write and read damon_region->sampling_addr, > > so I was thinking not initializing the field on the core layer is no problem. > > I will be happy to be corrected if I'm missing something. > > > > > > Thanks, > > SJ > > > >> > >> base-commit: 9d7a0577c9db35c4cc52db90bc415ea248446472 > >> -- > >> 2.43.0