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 63C41C54EE9 for ; Mon, 19 Sep 2022 15:01:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C4A516B0072; Mon, 19 Sep 2022 11:01:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BF9656B0073; Mon, 19 Sep 2022 11:01:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A9ADB80007; Mon, 19 Sep 2022 11:01:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 9969C6B0072 for ; Mon, 19 Sep 2022 11:01:08 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 6E246141C09 for ; Mon, 19 Sep 2022 15:01:08 +0000 (UTC) X-FDA: 79929147816.04.1BA6C4F Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by imf02.hostedemail.com (Postfix) with ESMTP id E761480010 for ; Mon, 19 Sep 2022 15:01:07 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 8769922224; Mon, 19 Sep 2022 15:01:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1663599666; h=from:from:reply-to: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=KmT7O1nTC5ewzPsWTyUyQ6zAmAiduoQIoZYJtb3b+/8=; b=borvO9NJzYlT2QJ+/yKEWVnOBoh8qjAvRxZf3wIk+grVLI8RgZa7KQPL6vckViHXUbHu2j CT+UZ30lmgoOp4SvJlNyjZIJ1RAbWeSgtCobKq+iloGykzghoYAA9raV2gNFv49cNwuNWO Y1YwVUp9R9x3xftcRI7nceTQAUYHQis= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1663599666; h=from:from:reply-to: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=KmT7O1nTC5ewzPsWTyUyQ6zAmAiduoQIoZYJtb3b+/8=; b=5YECWEHBU2yXwwMziXEswMp2WSASf5Lx1SAGpANi9GlrdPioYwaWtoQPhsOSk4zJr+xJYY sNOcYE/GMjD0YsAg== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 4F24A13A96; Mon, 19 Sep 2022 15:01:06 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id wzOlEjKEKGOjbgAAMHmgww (envelope-from ); Mon, 19 Sep 2022 15:01:06 +0000 Message-ID: <1920ee84-994e-e4bf-63e4-167842edca72@suse.cz> Date: Mon, 19 Sep 2022 17:01:06 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH v2 1/3] lib/stackdepot: Add a refcount field in stack_record Content-Language: en-US To: Andrey Konovalov , Oscar Salvador Cc: Andrew Morton , LKML , Linux Memory Management List , Michal Hocko , Eric Dumazet , Waiman Long , Suren Baghdasaryan , Marco Elver , Alexander Potapenko References: <20220905031012.4450-1-osalvador@suse.de> <20220905031012.4450-2-osalvador@suse.de> From: Vlastimil Babka In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1663599668; 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=KmT7O1nTC5ewzPsWTyUyQ6zAmAiduoQIoZYJtb3b+/8=; b=5VGaXCgN1VycNDAzHkT11HR/cw4hb0MZRisBnYZi8674m29ZUjf/0bi12NXt8G+kVs1Fa/ 9w2Kxkgy4k8i/MT2w/Ayt8id/BOaTyL7myTTCegybJKsZEc59Vbft7xrASFdUOB5s7MH1b qnSFEnVnbN6Q0A9VCHRvhvpK97ajkBQ= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=borvO9NJ; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=5YECWEHB; dmarc=none; spf=pass (imf02.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.28 as permitted sender) smtp.mailfrom=vbabka@suse.cz ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1663599668; a=rsa-sha256; cv=none; b=y/tlk/JOnx1/+Kz+XpP9Tp7tS34xNhQbp1Y3VGXT1EFIOcJe2BK/Ajh0mRRHZnxdK5d/9O Cc5n1W3QiZ2tYHbVgxLoLQ8GjcHdiZSMt+dLtmp2N/gN1rF36DraH/TccAWhuv7UvoorlK 8HNy7SP3cWcRuZOj43ENY8mzlF/8eww= Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=borvO9NJ; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=5YECWEHB; dmarc=none; spf=pass (imf02.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.28 as permitted sender) smtp.mailfrom=vbabka@suse.cz X-Rspam-User: X-Stat-Signature: bp7en3odajr5nkaf8aqo1qf8q9tuj5zi X-Rspamd-Queue-Id: E761480010 X-Rspamd-Server: rspam12 X-HE-Tag: 1663599667-720592 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 9/11/22 00:33, Andrey Konovalov wrote: >> 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. But to keep this really a "true number" we would have to now decrement the counter when the page gets freed by a different stack trace, i.e. we replace the previously stored free_handle with another one. Which is possible, but seems like a waste of CPU? > 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. I was thinking ideally we might want fully separate stackdepot storages per-stack trace user, i.e. separate hashmaps and dynamically allocated handles instead of the static "slabs" array. Different users tend to have distinct stacks so that would make lookups more effective too. Only CONFIG_STACKDEPOT_ALWAYS_INIT users (KASAN) would keep using the static array due to their inherent limitations wrt dynamic allocations. Then these different stackdepot instances could be optionally refcounted or not, and if need be, have additional metadata as you suggest. >> > 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! >