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 B2B9CD1D896 for ; Tue, 15 Oct 2024 18:40:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4FB216B008A; Tue, 15 Oct 2024 14:40:01 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4AB236B008C; Tue, 15 Oct 2024 14:40:01 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 372DF6B0092; Tue, 15 Oct 2024 14:40:01 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 13A5B6B008A for ; Tue, 15 Oct 2024 14:40:01 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 958F5AC9F8 for ; Tue, 15 Oct 2024 18:39:42 +0000 (UTC) X-FDA: 82676700708.06.2083E52 Received: from out-184.mta0.migadu.com (out-184.mta0.migadu.com [91.218.175.184]) by imf19.hostedemail.com (Postfix) with ESMTP id E5F941A0005 for ; Tue, 15 Oct 2024 18:39:48 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=gK5O0U6F; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf19.hostedemail.com: domain of shakeel.butt@linux.dev designates 91.218.175.184 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729017482; a=rsa-sha256; cv=none; b=rWQV8qDHjSa/CsRD5PMaiUdy9VcDsY2VjbWEZOSO9guRkYSqJrEJJY6MppSIcAzQWMHERQ pnn1jfrHo1RjnuTpU47jC5DNAzhjRQo9wV3pfmzgsW4nQQ3IJW2bNRKRVR7jbi5BgJawRm YFDoR5yH/WtZxufQe9K+yhrvZ88Dwuo= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=gK5O0U6F; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf19.hostedemail.com: domain of shakeel.butt@linux.dev designates 91.218.175.184 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729017482; 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:dkim-signature; bh=FlbzcQpu5GWkPP9o7pT9LceNEMQSVIKeLV87OlrVJ+I=; b=O5ivV2ksGycWxsI0n2oCM8aXit9ovsE04foqXpaI3XmgDbWEXXb78frIhWH3A+8WiWassk jTGRJ524dpwaX228Ttfwgh8Y5dGZcYNfJlbKi/jf7TWJYzd/geJj91zWwWDuLSbrnX+Kul A4AEhkgUC8tcGvsNUzwvf4Uz3w56zO8= Date: Tue, 15 Oct 2024 11:39:49 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1729017596; h=from:from: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=FlbzcQpu5GWkPP9o7pT9LceNEMQSVIKeLV87OlrVJ+I=; b=gK5O0U6FvMq1snMMBlg6SI+rj2WSWU40BUGuuN0mmpAom/kOcDVTGCPbsIL/IcR68J4XMW K2u9S0vvk3KjZGL+gYVPpiTlM8nFYWUza9rfbBStEeodcnuWTp87gBOt8hQULvNELAZgT0 9zuttan0WDCEVECf2W2Jl8FnbKvzfjo= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Shakeel Butt To: Yosry Ahmed Cc: Andrew Morton , Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , Steven Rostedt , JP Kobryn , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, Meta kernel team Subject: Re: [PATCH] memcg: add tracing for memcg stat updates Message-ID: References: <20241010003550.3695245-1-shakeel.butt@linux.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Migadu-Flow: FLOW_OUT X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: E5F941A0005 X-Stat-Signature: 9ppfidtbenod9tehkzwoencrg864bd6t X-Rspam-User: X-HE-Tag: 1729017588-498348 X-HE-Meta: U2FsdGVkX18Rf4Q6TtsO74scD4AiO0rbBQV6Tibvv2ejgJZTGJkbxoDhrRLsnIBSZoA4imQPQX2+b3cT9TgthArvqPO535RApmooJL87rakhzTGOfYvS0Icb+bMbxKtLznachuiSl4H9/podZtIiwrdnaFdeuLbrzMj9EefPc1K0ZooCtEqotvqSGGrIWbXHEbBzkdAJtcbndRFkDz9rkoFig0uhIAE281xSR9IuUvuah0njI+unXEaprVLQupuUJ85j0n0P5LGVvkLliCxl+4/oyty9oLz8WRPOv3EWjuwVz1LPrHNnSoASOnWwPTcTnF/igDx/+PJRlzSVe1i3cdSsepgxVww0OvJHvUVB82zn5MHLgUTZ/orLppkB1penKmXcX3A6ZrKBOaqxzZYND6Ri75gRm9vIYl2eZoPrvITNznEBfjUh2PdtldU9RY1EVM/l+soaT6qzJoehrXaWYGgRkzE15m7Hw2oqHceFYRjVVKq1XiSZPAhJTX/wbq8zIwLqLKPqIrSqWxuWUK9gA1YaS87iOCc1BpFGw+F1lh4L/52ysVQvb6iJ/I/MtXKvZs10OfSH73sTGJkC+ZcqpGLBFIjYJnzIdOPYoj7cg55J/WmgBBf2jVEPUtWmULT/gfWXC7LW/HJDYWWs1OKcXE0hfLo1VUMy1dyg3+awn5rULkdVXL/IV+5LDIOyOS+vKW8TM5/9EeoPqp/20sMoiBfhoIRJ2DsrSRkVz+JneJrENvBNvdIYpPCTFvtiwFon4O8HOOFhwJbiWCmqHaDbx57Bq2O6EmLtOVEdfkyTjfyY4nz9qrBCimSTw/0phjvv0ezMOTIbEfwOJRhZzlZrmTzeEnp53ASo/Qe8QnYY4ThFCcxfHNfQ4coGdmVocDbiHBrdc5MKjxNB6ToSMktgslsfvePGOv6XMecZ8P/X+i7+3ufHtI+Mw4DLhvxR9MzlhzkksqrmjEIxf8TGGTY zYRrgWND kBmoXcpAo32ztCDgRMsOESAqGM5bjnExgtQFZevEwMyxgfupLvO3ab4mNwkir/gACMqkUd/LVJBiDJdf3svurYaf6fzDUc+l2EUu/nWb2vVZjj1SC9VoArf74VPmzKZhwt/XW4BN0xqyQ7R3f4e7tD6TAFntKAWU1odgd2CHKAFgPr/kR7EPecXSumCPZZ5wvliouOa2iLZpfgxRTvb6NPD4r35iIYPBMxzDeZiaa47MRYlc= 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, Oct 15, 2024 at 01:07:30AM GMT, Yosry Ahmed wrote: > On Wed, Oct 9, 2024 at 5:36 PM Shakeel Butt wrote: > > > > The memcg stats are maintained in rstat infrastructure which provides > > very fast updates side and reasonable read side. However memcg added > > plethora of stats and made the read side, which is cgroup rstat flush, > > very slow. To solve that, threshold was added in the memcg stats read > > side i.e. no need to flush the stats if updates are within the > > threshold. > > > > This threshold based improvement worked for sometime but more stats were > > added to memcg and also the read codepath was getting triggered in the > > performance sensitive paths which made threshold based ratelimiting > > ineffective. We need more visibility into the hot and cold stats i.e. > > stats with a lot of updates. Let's add trace to get that visibility. > > > > Signed-off-by: Shakeel Butt > > One question below, otherwise: > > Reviewed-by: Yosry Ahmed > > > --- > > include/trace/events/memcg.h | 59 ++++++++++++++++++++++++++++++++++++ > > mm/memcontrol.c | 13 ++++++-- > > 2 files changed, 70 insertions(+), 2 deletions(-) > > create mode 100644 include/trace/events/memcg.h > > > > diff --git a/include/trace/events/memcg.h b/include/trace/events/memcg.h > > new file mode 100644 > > index 000000000000..913db9aba580 > > --- /dev/null > > +++ b/include/trace/events/memcg.h > > @@ -0,0 +1,59 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#undef TRACE_SYSTEM > > +#define TRACE_SYSTEM memcg > > + > > +#if !defined(_TRACE_MEMCG_H) || defined(TRACE_HEADER_MULTI_READ) > > +#define _TRACE_MEMCG_H > > + > > +#include > > +#include > > + > > + > > +DECLARE_EVENT_CLASS(memcg_rstat, > > + > > + TP_PROTO(struct mem_cgroup *memcg, int item, int val), > > + > > + TP_ARGS(memcg, item, val), > > + > > + TP_STRUCT__entry( > > + __field(u64, id) > > + __field(int, item) > > + __field(int, val) > > + ), > > + > > + TP_fast_assign( > > + __entry->id = cgroup_id(memcg->css.cgroup); > > + __entry->item = item; > > + __entry->val = val; > > + ), > > + > > + TP_printk("memcg_id=%llu item=%d val=%d", > > + __entry->id, __entry->item, __entry->val) > > +); > > + > > +DEFINE_EVENT(memcg_rstat, mod_memcg_state, > > + > > + TP_PROTO(struct mem_cgroup *memcg, int item, int val), > > + > > + TP_ARGS(memcg, item, val) > > +); > > + > > +DEFINE_EVENT(memcg_rstat, mod_memcg_lruvec_state, > > + > > + TP_PROTO(struct mem_cgroup *memcg, int item, int val), > > + > > + TP_ARGS(memcg, item, val) > > +); > > + > > +DEFINE_EVENT(memcg_rstat, count_memcg_events, > > + > > + TP_PROTO(struct mem_cgroup *memcg, int item, int val), > > + > > + TP_ARGS(memcg, item, val) > > +); > > + > > + > > +#endif /* _TRACE_MEMCG_H */ > > + > > +/* This part must be outside protection */ > > +#include > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index c098fd7f5c5e..17af08367c68 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -71,6 +71,10 @@ > > > > #include > > > > +#define CREATE_TRACE_POINTS > > +#include > > +#undef CREATE_TRACE_POINTS > > + > > #include > > > > struct cgroup_subsys memory_cgrp_subsys __read_mostly; > > @@ -682,7 +686,9 @@ void __mod_memcg_state(struct mem_cgroup *memcg, enum memcg_stat_item idx, > > return; > > > > __this_cpu_add(memcg->vmstats_percpu->state[i], val); > > - memcg_rstat_updated(memcg, memcg_state_val_in_pages(idx, val)); > > + val = memcg_state_val_in_pages(idx, val); > > + memcg_rstat_updated(memcg, val); > > + trace_mod_memcg_state(memcg, idx, val); > > } > > > > /* idx can be of type enum memcg_stat_item or node_stat_item. */ > > @@ -741,7 +747,9 @@ static void __mod_memcg_lruvec_state(struct lruvec *lruvec, > > /* Update lruvec */ > > __this_cpu_add(pn->lruvec_stats_percpu->state[i], val); > > > > - memcg_rstat_updated(memcg, memcg_state_val_in_pages(idx, val)); > > + val = memcg_state_val_in_pages(idx, val); > > + memcg_rstat_updated(memcg, val); > > + trace_mod_memcg_lruvec_state(memcg, idx, val); > > memcg_stats_unlock(); > > } > > > > @@ -832,6 +840,7 @@ void __count_memcg_events(struct mem_cgroup *memcg, enum vm_event_item idx, > > memcg_stats_lock(); > > __this_cpu_add(memcg->vmstats_percpu->events[i], count); > > memcg_rstat_updated(memcg, count); > > + trace_count_memcg_events(memcg, idx, count); > > count here is an unsigned long, and we are casting it to int, right? > > Would it be slightly better if the tracepoint uses a long instead of > int? It's still not ideal but probably better than int. > Do you mean something line the following? If this looks good to you then we can ask Andrew to squash this in the patch. diff --git a/include/trace/events/memcg.h b/include/trace/events/memcg.h index 913db9aba580..37812900acce 100644 --- a/include/trace/events/memcg.h +++ b/include/trace/events/memcg.h @@ -11,14 +11,14 @@ DECLARE_EVENT_CLASS(memcg_rstat, - TP_PROTO(struct mem_cgroup *memcg, int item, int val), + TP_PROTO(struct mem_cgroup *memcg, int item, long val), TP_ARGS(memcg, item, val), TP_STRUCT__entry( __field(u64, id) __field(int, item) - __field(int, val) + __field(long, val) ), TP_fast_assign( @@ -33,21 +33,21 @@ DECLARE_EVENT_CLASS(memcg_rstat, DEFINE_EVENT(memcg_rstat, mod_memcg_state, - TP_PROTO(struct mem_cgroup *memcg, int item, int val), + TP_PROTO(struct mem_cgroup *memcg, int item, long val), TP_ARGS(memcg, item, val) ); DEFINE_EVENT(memcg_rstat, mod_memcg_lruvec_state, - TP_PROTO(struct mem_cgroup *memcg, int item, int val), + TP_PROTO(struct mem_cgroup *memcg, int item, long val), TP_ARGS(memcg, item, val) ); DEFINE_EVENT(memcg_rstat, count_memcg_events, - TP_PROTO(struct mem_cgroup *memcg, int item, int val), + TP_PROTO(struct mem_cgroup *memcg, int item, long val), TP_ARGS(memcg, item, val) );