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 BC87BC71153 for ; Mon, 11 Sep 2023 20:31:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1C9D06B02F9; Mon, 11 Sep 2023 16:31:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 17A236B02FA; Mon, 11 Sep 2023 16:31:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 06A3C6B02FB; Mon, 11 Sep 2023 16:31:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id ED6BF6B02F9 for ; Mon, 11 Sep 2023 16:31:17 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id B992A140339 for ; Mon, 11 Sep 2023 20:31:17 +0000 (UTC) X-FDA: 81225461394.05.59EA4E8 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf04.hostedemail.com (Postfix) with ESMTP id E323540028 for ; Mon, 11 Sep 2023 20:31:15 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf04.hostedemail.com: domain of "SRS0=M4tJ=E3=goodmis.org=rostedt@kernel.org" designates 145.40.68.75 as permitted sender) smtp.mailfrom="SRS0=M4tJ=E3=goodmis.org=rostedt@kernel.org" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1694464276; 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-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=kXD7iK4A/b6uKGhz73mHyGW9LGhjqNPMD6h33GYFoWk=; b=PrmNN3FYQUlDJtMPGSNfc5fyIqTP6+a9brdJ/lUV3Hj1QXO/iPDqdF8X9JtJEvEu9a/P51 MzyfvfX7avv6D5f12XXWodkrtkhF/tnq54dCQP//Hu1y6TLBZ4sjdnZc9GrIxOeFpEzJZW eLiDt9SIpDP1KgurLEF01neeC2zOC6s= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf04.hostedemail.com: domain of "SRS0=M4tJ=E3=goodmis.org=rostedt@kernel.org" designates 145.40.68.75 as permitted sender) smtp.mailfrom="SRS0=M4tJ=E3=goodmis.org=rostedt@kernel.org" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1694464276; a=rsa-sha256; cv=none; b=NU1SJHBB+maTh9mXn+6ShykJrBlZsA8wXlIzBpnyWn3qBnfMxPCfCKnhn13FLCDXkjb0FM CUkQhvH934PxrpWFz7GudY+Uqjr98KvGOx8h7AcCSnxECiUZZfj9/tYg1DxKHIK34G0dC0 5u5HpmSKbWwmn4UwhA82/Wpmp8wnPWM= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id D4EDAB81887; Mon, 11 Sep 2023 20:31:13 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D09F8C433C7; Mon, 11 Sep 2023 20:31:11 +0000 (UTC) Date: Mon, 11 Sep 2023 16:31:27 -0400 From: Steven Rostedt To: SeongJae Park Cc: Andrew Morton , damon@lists.linux.dev, linux-mm@kvack.org, linux-trace-kernel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] mm/damon/core: add a tracepoint for damos apply target regions Message-ID: <20230911163127.167dccc2@gandalf.local.home> In-Reply-To: <20230911190504.102317-1-sj@kernel.org> References: <20230911141955.245d1397@gandalf.local.home> <20230911190504.102317-1-sj@kernel.org> X-Mailer: Claws Mail 3.19.1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspam-User: X-Stat-Signature: b3nub1y47irraxyz8pbgoepz1bnihbu8 X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: E323540028 X-HE-Tag: 1694464275-133641 X-HE-Meta: U2FsdGVkX1/jAKmZ/8YHATtyrPe0PT3aTqH0rNi3wmTHLnPBgdF1TQo5JcdIPcDjrcOdnOWscAPWjlMC54+ArYyXeiwjiTMxhq4t8dK74IfkI3KZaFC5PZazP2o80StexFcOlx+Vubn2jqBUyIEqVSUJekGb594GVFcD0JZlTyaGXWMLF29IhC+BQlEKyS545ryXTxKk4C9rCImy1eA4/kmzF9Wo66EZvb1GWbLPWvA/9lE3kYtQuHERNSVFbTDUDkE1qLQKWgarl9UxnKhrtnPJT0KFM8d4zvWx/opprccWgTb3yV4LaACmYuboX4Qg3V8deC98A1XBtA2G5xoQEJUzvbO2IDnm6oCYiDIEjKTaRllyFSB5qXHLVj71ia4PslLVfWu5ezmQs0CMenrI848uGck6mpRdS1/MP4ECQXWEw1n21lYQ/FmwdJihs7NomE98SMiRXkNHh+aY0cgWEu9FGe+mQ4fjM96dtpx5uyownOz1YgtykpeaqQeXXhtYgxtZ+fjer4ZYWSYUipMe9jlLFtwEZR8OJXUOWxoq2vWfVUHOvQcmRGWqDIeJDGqWGFIpJeom2JUNJfMWyzxA3uDVz2tHRA9/gZ8EilftV9MutOUIGva9XW3gJvxsfXPD8c+RHgmo+ey59hNSl1x2eQY+N3E4gXYqV3QUUB5vjZQ+ptWeFYeBaaRp3feds0pN58f5IdrBg/JJP6V+N4TuJYSljs6MZrf0vkXYFPQC0q3FLFhxHjAgBdRSZKHnE5v+2YF8BH4SF1JwmIlhrOlkXPoq7UEfrWphPnC07oC9wuzyMx7P9OcRyktPHC9ZTPubeJq9uxxqREKamt9zLuPBhjvj1N4AqjljFvKZ7x4u8vnDlhNPdBdNgK9ka8KTlqQ/9yexHRFE9rP/4QA/PYVaKCfC6nOIPR+mPrub3G8xFIlAzDJIRBv+e0oc8TsIsI50oAP1WaGOlJ0EdHZswsO f37G2SLI 4rV1JMPV26ffN3RSEhEGQ3C5vB+u5RU5rw8K10Q7k5cgMv8x8LU2RSS98HeEOlIFkobGw6V4Q4cPsrd+rh2HcMpnhD6XxgJdi+IsXQfCvn2e/6ZGSrDKWRTj30lvAlKH/QnA7PsbPM4HwFmNtqmhJja7pezp2+zyeIMJ7w4Z06Zb39sPTPKmzsjpQhY98ubfs5eDEmEWhsTCyfZHAQnVimqgpPSfJ0lQJNzha8ODaML3AuJ6zOOKH1rsyMrQbr0qCya2ZZth9UbglSwnSXs/9UP6F9CvXrH6iaYoQpDtll4C1+XUuX7dLBGRzukSHsxwfP+bDrhEC5urGgw9E5y9SVut0X5jAWHEccPz8KtUF6EBFeJXZdYYVMtaAurnD6yCC7iSLNuNbyOPlKkK/95GjckFnnQ== 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 Mon, 11 Sep 2023 19:05:04 +0000 SeongJae Park wrote: > > Also, this if statement is only done when the trace event is enabled, so > > it's equivalent to: > > > > if (trace_damos_before_apply_enabled()) { > > if (sdx >= 0) > > trace_damos_before_apply(cidx, sidx, tidx, r, > > damon_nr_regions(t)); > > } > > Again, thank you very much for letting me know this awesome feature. However, > sidx is supposed to be always >=0 here, since kdamond is running in single > thread and hence no race is expected. If it exists, it's a bug. So, I > wouldn't make this change. Appreciate again for letting me know this very > useful feature, and please let me know if I'm missing something, though! The race isn't with your code, but the enabling of tracing. Let's say you enable tracing just ass it passed the first: if (trace_damos_before_apply_enabled()) { damon_for_each_scheme(siter, c) { if (siter == s) break; sidx++; } damon_for_each_target(titer, c) { if (titer == t) break; tidx++; } Now, sidx and tidx is zero (when they were not computed, thus, they shouldn't be zero. Then tracing is fully enabled here, and now we enter: if (trace_damos_before_apply_enabled()) { trace_damos_before_apply(cidx, sidx, tidx, r, damon_nr_regions(t)); } Now the trace event is hit with sidx and tidx zero when they should not be. This could confuse you when looking at the report. What I suggested was to initialize sidx to zero, set it in the first trace_*_enabled() check, and ignore calling the tracepoint if it's not >= 0. -- Steve