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 X-Spam-Level: X-Spam-Status: No, score=-28.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 05610C4320E for ; Thu, 29 Jul 2021 17:34:20 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 8ECFE60E9B for ; Thu, 29 Jul 2021 17:34:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 8ECFE60E9B Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id EC4348D0003; Thu, 29 Jul 2021 13:34:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E741B8D0002; Thu, 29 Jul 2021 13:34:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D62B28D0003; Thu, 29 Jul 2021 13:34:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0037.hostedemail.com [216.40.44.37]) by kanga.kvack.org (Postfix) with ESMTP id B3EC28D0002 for ; Thu, 29 Jul 2021 13:34:18 -0400 (EDT) Received: from smtpin12.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 025B817AB0 for ; Thu, 29 Jul 2021 17:34:15 +0000 (UTC) X-FDA: 78416324112.12.A71556E Received: from mail-io1-f52.google.com (mail-io1-f52.google.com [209.85.166.52]) by imf08.hostedemail.com (Postfix) with ESMTP id 9E4BA300024B for ; Thu, 29 Jul 2021 17:34:15 +0000 (UTC) Received: by mail-io1-f52.google.com with SMTP id f11so8158585ioj.3 for ; Thu, 29 Jul 2021 10:34:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Y9Ewlgw+kgawQ6AHZLlkWuiBcJPxDcUOvpHKSWaDCcQ=; b=YTbgE128gsBTPnNOjQpj/EMQHtqnVoC34nMf+kBkBjQNWFLbbfr6EQFyrczUXcAnXq /jRtU2y7k/ZzzgWy6L5OfS3NpHRCVxtATAziFw8X6W4lF9EKn8ORcKYKO1Ez9Ay+a0km aVI5+SuNIGJafEqNkNH3p4nqlvHCVPrU97h49T3sp8QWlmnBxM/K55gtON/yII2WEexl zxtwbq9bJmqT0v8eayrUN398AKSvqNkyNi659AbP4VL0h5rVefFlhhPP2T0H5QXoVDz1 9kcfEgRwGGTjrb7AybsodS7wIxAK0nl/GxuoABuQkCilPnCTcXNaDxHWUJbkABuYVm2K cfIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Y9Ewlgw+kgawQ6AHZLlkWuiBcJPxDcUOvpHKSWaDCcQ=; b=LJxYCNScV520MbUde9TnvRErcvxbV8+lJN8je64LkZgUzIF1n4Vs3rnyW9Q/r9kHoi JBmrQkdtJX/dEDvpZpMhLEyKoMN2sM8QT3RCU2Eeo4aaheA5SBXjodKbLXQY9peJBnXR bsC9KdSSK+vbbZeuDZW4A/5/L4cZjSDliyNZze6/51UFWQQ7pdbhA28/d70HP2I4To+z 32DgtAwU9gLv+dKtR6SpVRX9vyXBlcObt28/MFbpuIS07wOUszyN35Itreup8rA+AVQX zctxxT/L0CRfbB4+/JQzJhBwSC8BOPFV5tiPsiSpNDPfQ+C34ymZ78cQZmrMKPzvSDps /nxw== X-Gm-Message-State: AOAM533PKKjKBstbjOk+tvv5rNLtaNGxGTmQ5gpCtxnl6k9XI7fa5f1s YznjFMHeSgsAMsP6X9aS3gZHQnuL8RZqZvPP81lljA== X-Google-Smtp-Source: ABdhPJz6ZYlm1UALlZLeni161G5bwtT+dSVL+gU7HdRPIcCo8gfRPcUoD4TxKOZVTrIovoLdnwd4m3f3w7vOkWaNTL0= X-Received: by 2002:a5e:8f0d:: with SMTP id c13mr4890425iok.57.1627580054841; Thu, 29 Jul 2021 10:34:14 -0700 (PDT) MIME-Version: 1.0 References: <20210729092853.38242-1-ligang.bdlg@bytedance.com> In-Reply-To: <20210729092853.38242-1-ligang.bdlg@bytedance.com> From: Axel Rasmussen Date: Thu, 29 Jul 2021 10:33:38 -0700 Message-ID: Subject: Re: [PATCH 3/3] mm: mmap_lock: add ip to mmap_lock tracepoints To: Gang Li Cc: Steven Rostedt , Ingo Molnar , Andrew Morton , Vlastimil Babka , LKML , Linux MM Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=YTbgE128; spf=pass (imf08.hostedemail.com: domain of axelrasmussen@google.com designates 209.85.166.52 as permitted sender) smtp.mailfrom=axelrasmussen@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspamd-Server: rspam02 X-Stat-Signature: fibnwn9bi8wqx1aoj93wtpo33rkhc1rn X-Rspamd-Queue-Id: 9E4BA300024B X-HE-Tag: 1627580055-612063 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: Not a strong objection, but I think this can be achieved already using either: - The "stacktrace" feature which histogram triggers support (https://www.kernel.org/doc/html/latest/trace/histogram.html) - bpftrace's kstack/ustack feature (https://github.com/iovisor/bpftrace/blob/master/docs/tutorial_one_liners.md#lesson-9-profile-on-cpu-kernel-stacks) I haven't tried it out myself, but I suspect you could construct a synthetic event (https://www.kernel.org/doc/html/latest/trace/histogram.html#synthetic-events) which adds in the stack trace, then it ought to function a lot like it would with this patch. Then again, it's not like this change is huge by any means. So, if you find this more convenient than those alternatives, you can take: Reviewed-by: Axel Rasmussen It's possible Steven or Tom have a more strong opinion on this though. ;) On Thu, Jul 29, 2021 at 2:29 AM Gang Li wrote: > > The mmap_lock is acquired on most (all?) mmap / munmap / page fault > operations, so a multi-threaded process which does a lot of these > can experience significant contention. Sometimes we want to know > where the lock is hold. And it's hard to locate without collecting ip. > > Here's an example: TP_printk("ip=%pS",ip) > Log looks like this: "ip=do_user_addr_fault+0x274/0x640" > > We can find out who cause the contention amd make some improvements > for it. > > Signed-off-by: Gang Li > --- > include/trace/events/mmap_lock.h | 27 +++++++++++++++++---------- > mm/mmap_lock.c | 6 +++--- > 2 files changed, 20 insertions(+), 13 deletions(-) > > diff --git a/include/trace/events/mmap_lock.h b/include/trace/events/mmap_lock.h > index b9dd66f9c226..8913a9f85894 100644 > --- a/include/trace/events/mmap_lock.h > +++ b/include/trace/events/mmap_lock.h > @@ -15,35 +15,39 @@ extern void trace_mmap_lock_unreg(void); > > DECLARE_EVENT_CLASS(mmap_lock, > > - TP_PROTO(struct mm_struct *mm, const char *memcg_path, bool write), > + TP_PROTO(struct mm_struct *mm, const char *memcg_path, bool write, > + unsigned long ip), > > - TP_ARGS(mm, memcg_path, write), > + TP_ARGS(mm, memcg_path, write, ip), > > TP_STRUCT__entry( > __field(struct mm_struct *, mm) > __string(memcg_path, memcg_path) > __field(bool, write) > + __field(void *, ip) > ), > > TP_fast_assign( > __entry->mm = mm; > __assign_str(memcg_path, memcg_path); > __entry->write = write; > + __entry->ip = (void *)ip; > ), > > TP_printk( > - "mm=%p memcg_path=%s write=%s", > + "mm=%p memcg_path=%s write=%s ip=%pS", > __entry->mm, > __get_str(memcg_path), > - __entry->write ? "true" : "false" > - ) > + __entry->write ? "true" : "false", > + __entry->ip > + ) > ); > > #define DEFINE_MMAP_LOCK_EVENT(name) \ > DEFINE_EVENT_FN(mmap_lock, name, \ > TP_PROTO(struct mm_struct *mm, const char *memcg_path, \ > - bool write), \ > - TP_ARGS(mm, memcg_path, write), \ > + bool write, unsigned long ip), \ > + TP_ARGS(mm, memcg_path, write, ip), \ > trace_mmap_lock_reg, trace_mmap_lock_unreg) > > DEFINE_MMAP_LOCK_EVENT(mmap_lock_start_locking); > @@ -52,14 +56,15 @@ DEFINE_MMAP_LOCK_EVENT(mmap_lock_released); > TRACE_EVENT_FN(mmap_lock_acquire_returned, > > TP_PROTO(struct mm_struct *mm, const char *memcg_path, bool write, > - bool success), > + unsigned long ip, bool success), > > - TP_ARGS(mm, memcg_path, write, success), > + TP_ARGS(mm, memcg_path, write, ip, success), > > TP_STRUCT__entry( > __field(struct mm_struct *, mm) > __string(memcg_path, memcg_path) > __field(bool, write) > + __field(void *, ip) > __field(bool, success) > ), > > @@ -67,14 +72,16 @@ TRACE_EVENT_FN(mmap_lock_acquire_returned, > __entry->mm = mm; > __assign_str(memcg_path, memcg_path); > __entry->write = write; > + __entry->ip = (void *)ip; > __entry->success = success; > ), > > TP_printk( > - "mm=%p memcg_path=%s write=%s success=%s", > + "mm=%p memcg_path=%s write=%s ip=%pS success=%s", > __entry->mm, > __get_str(memcg_path), > __entry->write ? "true" : "false", > + __entry->ip, > __entry->success ? "true" : "false" > ), > > diff --git a/mm/mmap_lock.c b/mm/mmap_lock.c > index 1854850b4b89..f1100eae6f2f 100644 > --- a/mm/mmap_lock.c > +++ b/mm/mmap_lock.c > @@ -227,20 +227,20 @@ static const char *get_mm_memcg_path(struct mm_struct *mm) > > void __mmap_lock_do_trace_start_locking(struct mm_struct *mm, bool write) > { > - TRACE_MMAP_LOCK_EVENT(start_locking, mm, write); > + TRACE_MMAP_LOCK_EVENT(start_locking, mm, write, _RET_IP_); > } > EXPORT_SYMBOL(__mmap_lock_do_trace_start_locking); > > void __mmap_lock_do_trace_acquire_returned(struct mm_struct *mm, bool write, > bool success) > { > - TRACE_MMAP_LOCK_EVENT(acquire_returned, mm, write, success); > + TRACE_MMAP_LOCK_EVENT(acquire_returned, mm, write, _RET_IP_, success); > } > EXPORT_SYMBOL(__mmap_lock_do_trace_acquire_returned); > > void __mmap_lock_do_trace_released(struct mm_struct *mm, bool write) > { > - TRACE_MMAP_LOCK_EVENT(released, mm, write); > + TRACE_MMAP_LOCK_EVENT(released, mm, write, _RET_IP_); > } > EXPORT_SYMBOL(__mmap_lock_do_trace_released); > #endif /* CONFIG_TRACING */ > -- > 2.20.1 >