From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-m32112.qiye.163.com (mail-m32112.qiye.163.com [220.197.32.112]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B3714337BA6 for ; Wed, 29 Apr 2026 08:24:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=220.197.32.112 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777451094; cv=none; b=htVRSH0/Nr9yXDQzajTMCqscs/zOch5VHraLcHBQmGYilzbzSJ3Tfzr/LTsqPHLtz5444MtQp9JZiq/Xge4NaLKoz9Re2zKGS1Z9CWLAaD2yo6fHImpYbPJFuGNuFAl3uOPbwyune0wtgRUcotL6MTF8MO5VgRETwiNQyYe75Qg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777451094; c=relaxed/simple; bh=Sd6lGMfFkt/4j+VaACyYOZ48+C/gwiPCr7LeeS9bTdo=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=rYAcAY/g+5mqke0PMAoLSmuX3C3BjJisq6JjYytxZSaKLPzChKWDQhLkcMnI8vVW5MQkGmdoEQUXevfp5ABnJwBkD9/pGkuLSOq6MddLe0Fq8sj1Fri32Fkx2XfGmVEUnR20rsa87XAK/egaPOe7qgGPSe4BZyjlU1tTeD4S2F8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=easystack.cn; spf=pass smtp.mailfrom=easystack.cn; arc=none smtp.client-ip=220.197.32.112 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=easystack.cn Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=easystack.cn Received: from [192.168.0.18] (unknown [218.94.118.90]) by smtp.qiye.163.com (Hmail) with ESMTP id 1989574e9; Wed, 29 Apr 2026 16:19:30 +0800 (GMT+08:00) Message-ID: <628b4e32-a004-421b-9c12-57892c16874e@easystack.cn> Date: Wed, 29 Apr 2026 16:19:29 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 2/4] mm/page_owner: add print_mode filter To: SeongJae Park Cc: akpm@linux-foundation.org, vbabka@kernel.org, surenb@google.com, mhocko@suse.com, jackmanb@google.com, hannes@cmpxchg.org, ziy@nvidia.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20260429005739.88517-1-sj@kernel.org> From: "zhen.ni" In-Reply-To: <20260429005739.88517-1-sj@kernel.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-HM-Tid: 0a9dd85260060229kunm78beb2c21ae281 X-HM-MType: 1 X-HM-Spam-Status: e1kfGhgUHx5ZQUpXWQgPGg8OCBgUHx5ZQUlOS1dZFg8aDwILHllBWSg2Ly tZV1koWUFJQjdXWRgWCB1ZQUpXWS1ZQUlXWQ8JGhUIEh9ZQVkZGh0ZVhhNShhMSUkfHhpPQlYVFA kWGhdVGRETFhoSFyQUDg9ZV1kYEgtZQVlJSkNVQk9VSkpDVUJLWVdZFhoPEhUdFFlBWU9LSFVCQk lOS1VKS0tVSkJLQlkG 在 2026/4/29 08:57, SeongJae Park 写道: > On Tue, 28 Apr 2026 15:11:10 +0800 Zhen Ni wrote: > >> Add print_mode functionality to reduce page_owner output size by >> printing only the stack handle instead of the full stack trace. >> >> Example output with print_mode enabled: >> Page allocated via order 0, mask 0x42800(GFP_NOWAIT|__GFP_COMP), >> pid 1, tgid 1 (systemd), ts 349667370 ns >> PFN 0xa00a2 type Unmovable Block 1280 type Unmovable >> Flags 0x33fffe0000004124(referenced|lru|active|private|node=3|zone=0| >> lastcpupid=0x1ffff) >> handle: 17432583 >> Charged to memcg / > > Looks nice to me. I have just trivial cosmetic comments below. > >> >> Print mode significantly reduces output size while preserving all >> other page allocation information. The correspondence between handles >> and stack traces can be obtained through the show_stacks_handles interface. > > I understand the mode was introduced for the stack handle mode, but it exists > for both full stack and stack handle mode? The wording makes me assume the > mode is only enabled vs disabled (boolean). It is true that there are only two > modes, but I feel like this commit message might better written. > The commit message could be clearer. >> >> Link: https://lore.kernel.org/linux-mm/20260417154638.22370-3-zhen.ni@easystack.cn/ > > Seems this is a link to v1 of this patch. I think adding the context or moving > this to the changelog [1] with the brief context explanation would be nice. > Good suggestion. >> Suggested-by: Zi Yan >> Signed-off-by: Zhen Ni >> --- >> >> Changes in v2: >> - Renamed from 'compact mode' to 'print_mode' for better clarity >> - Use enum values (0=full_stack, 1=stack_handle) instead of boolean >> - Update debugfs filename from 'compact' to 'print_mode' >> >> Changes in v3: >> - No code changes >> --- >> mm/page_owner.c | 28 +++++++++++++++++++++++++++- >> 1 file changed, 27 insertions(+), 1 deletion(-) >> >> diff --git a/mm/page_owner.c b/mm/page_owner.c >> index 5884d883837e..6d87b6948cfa 100644 >> --- a/mm/page_owner.c >> +++ b/mm/page_owner.c >> @@ -590,7 +590,13 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn, >> migratetype_names[pageblock_mt], >> &page->flags); >> >> - ret += stack_depot_snprint(handle, kbuf + ret, count - ret, 0); >> + /* Print mode: full stack or stack handle */ > > I think this comment should be useful if it was compact_mode. But because now > it is called print_mode and values have good names that self explaining what > the mode is, I'm not sure if this comment is really needed. > I'll remove the redundant comment >> + if (READ_ONCE(owner_filter.print_mode) == PAGE_OWNER_PRINT_STACK_HANDLE) { >> + ret += scnprintf(kbuf + ret, count - ret, >> + "handle: %d\n", handle); >> + } else { >> + ret += stack_depot_snprint(handle, kbuf + ret, count - ret, 0); >> + } > > Braces are not needed [2] here? > I'll remove the unnecessary braces Thanks for the detailed review >> if (ret >= count) >> goto err; >> >> @@ -985,6 +991,24 @@ static int page_owner_threshold_set(void *data, u64 val) >> DEFINE_SIMPLE_ATTRIBUTE(page_owner_threshold_fops, &page_owner_threshold_get, >> &page_owner_threshold_set, "%llu"); >> >> +static int page_owner_print_mode_get(void *data, u64 *val) >> +{ >> + *val = READ_ONCE(owner_filter.print_mode); >> + return 0; >> +} >> + >> +static int page_owner_print_mode_set(void *data, u64 val) >> +{ >> + if (val > PAGE_OWNER_PRINT_STACK_HANDLE) >> + return -EINVAL; >> + WRITE_ONCE(owner_filter.print_mode, val); >> + return 0; >> +} >> + >> +DEFINE_SIMPLE_ATTRIBUTE(page_owner_print_mode_fops, >> + &page_owner_print_mode_get, >> + &page_owner_print_mode_set, "%lld"); >> + >> >> static int __init pageowner_init(void) >> { >> @@ -998,6 +1022,8 @@ static int __init pageowner_init(void) >> debugfs_create_file("page_owner", 0400, NULL, NULL, &page_owner_fops); >> >> filter_dir = debugfs_create_dir("page_owner_filter", NULL); >> + debugfs_create_file("print_mode", 0600, filter_dir, NULL, >> + &page_owner_print_mode_fops); >> >> dir = debugfs_create_dir("page_owner_stacks", NULL); >> debugfs_create_file("show_stacks", 0400, dir, >> -- >> 2.20.1 > > [1] https://docs.kernel.org/process/submitting-patches.html#commentary > [2] https://docs.kernel.org/process/coding-style.html#placing-braces-and-spaces > > > Thanks, > SJ > > Best regards, Zhen Ni