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=-14.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no 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 D146BC04EBE for ; Thu, 8 Oct 2020 07:40:21 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 54AB62184D for ; Thu, 8 Oct 2020 07:40:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="q5USTEPG" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 54AB62184D Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 8CC106B0062; Thu, 8 Oct 2020 03:40:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 855D16B0068; Thu, 8 Oct 2020 03:40:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 743226B006C; Thu, 8 Oct 2020 03:40:20 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0163.hostedemail.com [216.40.44.163]) by kanga.kvack.org (Postfix) with ESMTP id 43ABB6B0062 for ; Thu, 8 Oct 2020 03:40:20 -0400 (EDT) Received: from smtpin16.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id CEF76362A for ; Thu, 8 Oct 2020 07:40:19 +0000 (UTC) X-FDA: 77347960158.16.magic57_0b0cf79271d6 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin16.hostedemail.com (Postfix) with ESMTP id AEA3310188A99 for ; Thu, 8 Oct 2020 07:40:19 +0000 (UTC) X-HE-Tag: magic57_0b0cf79271d6 X-Filterd-Recvd-Size: 5773 Received: from mail-yb1-f196.google.com (mail-yb1-f196.google.com [209.85.219.196]) by imf34.hostedemail.com (Postfix) with ESMTP for ; Thu, 8 Oct 2020 07:40:19 +0000 (UTC) Received: by mail-yb1-f196.google.com with SMTP id b142so3830252ybg.9 for ; Thu, 08 Oct 2020 00:40:19 -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=7Rp3My2L/roRnIDEi9kisQtYgeX2FwvYUO7ZV7SrOvU=; b=q5USTEPGaKtcH62zrMADCGgthfmfGu4Q0LxLhnuNjym9+D4SMw3X36j3KU7PUzr5Dl O3SJ/s06qAC48u4lXvaYATZ9of9OC8WEoI/ZlA3aPa7c1M/57tckvnl0uaHIHe5Xr2xi otnnoT8jMz+Jcjwqv/oBEMWHDvMowmhOOh+e12Fg2AUsu0gYGfWyU0zIy7TLanaRGLqM 7D8tix2Bp8rO+xDRzZ3Jy541nYCFqdZsX3CNKZR+CkLyWwIHhRAiAE7w9HIlGJTvQEfT FNPcT1nKGBB2ie1x2KIjwAWWw9jrptHbuJOeQaGqo/n0tT+9ScOR7Zx3WNGnS5qhSeRT srBQ== 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=7Rp3My2L/roRnIDEi9kisQtYgeX2FwvYUO7ZV7SrOvU=; b=CqaYdFsFkSWqoCgB9oFjJGPoAlIg6bj0PnU4WixYMVDGAMg207AuP4b8+6bxtFvXus 0lmzh1G5p/YYw07Ja8T1UFnStiEJ5GeiuO02nq6tNkY3fLXB2vtE0ehuphAiaG1N4p+5 hcMmUw6OoBV7HSiVkBT7Gw64jrsR03x1vqL6BptUuHyZI6Ok0KjijBGUEoQlyHfYS2NM bti28yWgjKPkszbwyH4l+y5FeWwfGJoUhq8kPkpEbphZRAGRsE9vvFy22TH7RcHuU1UU 5DnfwwoBxBuNalV8b1O0b+ZDlFyyR06I9+avh3eL+b+at+y9BqXAcI7TlU4wTbaCaFtL uPMA== X-Gm-Message-State: AOAM53135hs4D3dkKPhTGt+lbI0BSczCutMT+x9p8GqaEql4+lHGaDQV hdWjlgHLRXYHaZlVaWZv/xew2EERmhKxUJ73I+u+0Q== X-Google-Smtp-Source: ABdhPJz8G0UzuxWD3kadRsINe7B0llIGw01vL0xbE8O4KhdvJJiDtB4CFJz7q0izUDi5LxJ7jREm6oqkxc9v//V3yp0= X-Received: by 2002:a25:840d:: with SMTP id u13mr9231276ybk.7.1602142818150; Thu, 08 Oct 2020 00:40:18 -0700 (PDT) MIME-Version: 1.0 References: <20201007184403.1902111-1-axelrasmussen@google.com> <20201007184403.1902111-3-axelrasmussen@google.com> In-Reply-To: <20201007184403.1902111-3-axelrasmussen@google.com> From: Michel Lespinasse Date: Thu, 8 Oct 2020 00:40:05 -0700 Message-ID: Subject: Re: [PATCH v2 2/2] mmap_lock: add tracepoints around lock acquisition To: Axel Rasmussen Cc: Steven Rostedt , Ingo Molnar , Andrew Morton , Vlastimil Babka , Daniel Jordan , Laurent Dufour , Jann Horn , Chinwen Chang , Yafang Shao , LKML , linux-mm Content-Type: text/plain; charset="UTF-8" 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 Wed, Oct 7, 2020 at 11:44 AM Axel Rasmussen wrote: > The goal of these tracepoints is to be able to debug lock contention > issues. This 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. > > We trace just before we start acquisition, when the acquisition returns > (whether it succeeded or not), and when the lock is released (or > downgraded). The events are broken out by lock type (read / write). > > The events are also broken out by memcg path. For container-based > workloads, users often think of several processes in a memcg as a single > logical "task", so collecting statistics at this level is useful. > > The end goal is to get latency information. This isn't directly included > in the trace events. Instead, users are expected to compute the time > between "start locking" and "acquire returned", using e.g. synthetic > events or BPF. The benefit we get from this is simpler code. > > Because we use tracepoint_enabled() to decide whether or not to trace, > this patch has effectively no overhead unless tracepoints are enabled at > runtime. If tracepoints are enabled, there is a performance impact, but > how much depends on exactly what e.g. the BPF program does. > > Signed-off-by: Axel Rasmussen Thanks for working on this. I like that there is no overhead unless CONFIG_TRACING is set. However, I think the __mmap_lock_traced_lock and similar functions are the wrong level of abstraction, especially considering that we are considering to switch away from using rwsem as the underlying lock implementation. Would you consider something along the following lines instead for include/linux/mmap_lock.h ? #ifdef CONFIG_TRACING DECLARE_TRACEPOINT(...); void __mmap_lock_do_trace_start_locking(struct mm_struct *mm, bool write); static inline void mmap_lock_trace_start_locking(struct mm_struct *mm, bool write) { if (tracepoint_enabled(mmap_lock_start_locking)) __mmap_lock_do_trace_start_locking(mm, write); } #else static inline void mmap_lock_trace_start_locking(struct mm_struct *mm, bool write) {} #endif static inline void mmap_write_lock(struct mm_struct *mm) { mmap_lock_trace_start_locking(mm, true); down_write(&mm->mmap_lock); mmap_lock_trace_acquire_returned(mm, true, true); } I think this is more straightforward, and also the mmap_lock_trace_start_locking and similar functions don't depend on the underlying lock implementation. The changes to the other files look fine to me. -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies.