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 C772CECAAD3 for ; Sat, 10 Sep 2022 22:33:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C6D566B0071; Sat, 10 Sep 2022 18:33:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C1BD26B0072; Sat, 10 Sep 2022 18:33:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ABD228D0001; Sat, 10 Sep 2022 18:33:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 999416B0071 for ; Sat, 10 Sep 2022 18:33:15 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 54D9416035D for ; Sat, 10 Sep 2022 22:33:15 +0000 (UTC) X-FDA: 79897627950.02.247E807 Received: from mail-qt1-f178.google.com (mail-qt1-f178.google.com [209.85.160.178]) by imf10.hostedemail.com (Postfix) with ESMTP id 0F2DBC007D for ; Sat, 10 Sep 2022 22:33:14 +0000 (UTC) Received: by mail-qt1-f178.google.com with SMTP id cb8so3849427qtb.0 for ; Sat, 10 Sep 2022 15:33:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=rkBnb2K/p/J8hLmaT99r7CzCPZNFhjTvaKUvRHmeOhU=; b=i5yv2hh6Fa+6IoYmAkb8iQ4y7spBmLCby+cfpYHSZ95RM0n25zR3hbcu1XNCFCHKGs voOQS5W4q25ZULvtDeBuxOdfKA3YFo5iQPQPMP25D+/qDhrh/UxGeVUfKxuOgzZ5WtYb BI1qjjzMe5/y6pV8/2QvKSe0E6fqQfLNK1RzE9AJWxUsnIesSnmh5NrH6sJQpIXzlY/R gMVTIVpOCRhnI8sSGaN0C1WilTWpacfeGXxIPUhbxUDqJzKqWPlPUSfUoqxc/cxpyIDF tbHsWL1g6BuuETXBOo68pEXGwlWLr0s0k/VGgZybINwMWpcVa4+nTEhs/m9b0CBh2ZSP p9Vg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=rkBnb2K/p/J8hLmaT99r7CzCPZNFhjTvaKUvRHmeOhU=; b=YDp/0fslhdvLtzuaj+IcC8CfCwEn3Kopo4qw+AHgN/UQbjkAjTNpBMLpYOjSILfTTP NoRMyO6tTD7vbRoXqEIjKXJPZV97VztT7S7yXkTPjMI76fXlde+I6x2/ZybkwFWxscMp g4/LDWGbcW2uOsMND5MVWHrvrRSfrecnL+lpvUMaiXlC0eZnsjIuazrxDNQJnJRxNkPR xs3baUy//184uxr0qlcO/Bu1uX1MTJU0o3Fu7Zqrd68MiiEnLW5dth4PUaLtDWzjxbGR vYaK+5E6tUvijpfB+bRhFewn4lM5/TNvajKVYiUJPa+fVG1gELXPQkRb2u2jVXeDUwon Ioig== X-Gm-Message-State: ACgBeo1cKzpkgtj5xbz5qqvkL4iHXJRxeLWANhtzX6bKZvPVB5fL7oru JRI0j2jaTkjPQZzca4dJnb9HDpjrllD5ImRMjHM= X-Google-Smtp-Source: AA6agR5eevjcxDiCy+zzWvZj0jIteNlsUHkvQd/bGmyXqLn5cv4oqgVEwYP+KRrK85MA54RklmDHxAXGRBo4blZzE2g= X-Received: by 2002:a05:622a:11cf:b0:35b:a369:cc3 with SMTP id n15-20020a05622a11cf00b0035ba3690cc3mr7027882qtk.11.1662849194344; Sat, 10 Sep 2022 15:33:14 -0700 (PDT) MIME-Version: 1.0 References: <20220905031012.4450-1-osalvador@suse.de> <20220905031012.4450-2-osalvador@suse.de> In-Reply-To: From: Andrey Konovalov Date: Sun, 11 Sep 2022 00:33:03 +0200 Message-ID: Subject: Re: [PATCH v2 1/3] lib/stackdepot: Add a refcount field in stack_record To: Oscar Salvador Cc: Andrew Morton , LKML , Linux Memory Management List , Michal Hocko , Vlastimil Babka , Eric Dumazet , Waiman Long , Suren Baghdasaryan , Marco Elver , Alexander Potapenko Content-Type: text/plain; charset="UTF-8" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1662849195; a=rsa-sha256; cv=none; b=ghagYSyBZO+3d1t7YgHsVwy7ngDoCvSQmckSabbfA37x4LXepwsd3EZQnCb34Yocvwlw4N bBcpo/7aeLdq7327/SyoUKAI+huHVq0BvCuDbp8x4DYKLEV3Tgb6z4S4UYFqsn1/4VD/DU tBX7vd5Mmb5VEGM4uMfTW2KLFsR1FOk= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=i5yv2hh6; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf10.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.160.178 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1662849195; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=rkBnb2K/p/J8hLmaT99r7CzCPZNFhjTvaKUvRHmeOhU=; b=PEUaguo5qskgdDiYVXgcPs4T/osu30I+poWXbmJFfRrpE1044jrvH+aOjFWx33nycTQ/p3 mSIwvPXo3TyNT9e0vb6+1aJIselShBhqZy2M6QmI/l9R+eNe9Wd1MypJ+ydNZ9erS7/MsX 0/JgPQ9f3/E1nFy2mQvPHSGGrwaVSPI= Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=i5yv2hh6; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf10.hostedemail.com: domain of andreyknvl@gmail.com designates 209.85.160.178 as permitted sender) smtp.mailfrom=andreyknvl@gmail.com X-Rspam-User: X-Rspamd-Queue-Id: 0F2DBC007D X-Rspamd-Server: rspam11 X-Stat-Signature: 9jdquoz4sc7gpz4zeaacsxm179o3kum3 X-HE-Tag: 1662849194-411643 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 Tue, Sep 6, 2022 at 5:54 AM Oscar Salvador wrote: > > On Mon, Sep 05, 2022 at 10:57:20PM +0200, Andrey Konovalov wrote: > > On Mon, Sep 5, 2022 at 5:10 AM Oscar Salvador wrote: > > > +enum stack_depot_action { > > > + STACK_DEPOT_ACTION_NONE, > > > + STACK_DEPOT_ACTION_COUNT, > > > +}; > > > > Hi Oscar, > > Hi Andrey > > > Why do we need these actions? Why not just increment the refcount on > > each stack trace save? > > Let me try to explain it. > > Back in RFC, there were no actions and the refcount > was incremented/decremented in __set_page_ownwer() > and __reset_page_owner() functions. > > This lead to a performance "problem", where you would > look for the stack twice, one when save it > and one when increment it. I don't get this. If you increment the refcount at the same moment when you save a stack trace, why do you need to do the lookup twice? > We figured we could do better and, at least, for the __set_page_owner() > we could look just once for the stacktrace when calling __stack_depot_save, > and increment it directly there. Exactly. > We cannot do that for __reset_page_owner(), because the stack we are > saving is the freeing stacktrace, and we are not interested in that. > That is why __reset_page_owner() does: > > <--- > depot_stack_handle_t alloc_handle; > > ... > alloc_handle = page_owner->handle; > handle = save_stack(GFP_NOWAIT | __GFP_NOWARN, STACK_DEPOT_ACTION_NONE); > page_owner->free_handle = handle > stack_depot_dec_count(alloc_handle); > ---> > > alloc_handle contains the handle for the allocation stacktrace, which was set > in __set_page_owner(), and page_owner->free handle contains the handle for the > freeing stacktrace. > But we are only interested in the allocation stack and we only want to increment/decrement > that on allocation/free. But what is the problem with incrementing the refcount for free stacktrace in __reset_page_owner? You save the stack there anyway. Or is this because you don't want to see free stack traces when filtering for bigger refcounts? I would argue that this is not a thing stack depot should care about. If there's a refcount, it should reflect the true number of references. Perhaps, what you need is some kind of per-stack trace user metadata. And the details of what format this metadata takes shouldn't be present in the stack depot code. > > Could you split out the stack depot and the page_owner changes into > > separate patches? > > I could, I am not sure if it would make the review any easier though, > as you could not match stackdepot <-> page_owner changes. > > And we should be adding a bunch of code that would not be used till later on. > But I can try it out if there is a strong opinion. Yes, splitting would be quite useful. It allows backporting stack depot changes without having to backport any page_owner code. As long as there are not breaking interface changes, of course. Thanks!