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 20C76C48297 for ; Fri, 9 Feb 2024 08:01:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 83F1A6B0074; Fri, 9 Feb 2024 03:01:20 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7C76D6B0075; Fri, 9 Feb 2024 03:01:20 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6694A6B0078; Fri, 9 Feb 2024 03:01:20 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 51F1C6B0074 for ; Fri, 9 Feb 2024 03:01:20 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 14A61140F37 for ; Fri, 9 Feb 2024 08:01:20 +0000 (UTC) X-FDA: 81771520320.26.9D356C7 Received: from mail-vk1-f177.google.com (mail-vk1-f177.google.com [209.85.221.177]) by imf25.hostedemail.com (Postfix) with ESMTP id 4CD3DA0013 for ; Fri, 9 Feb 2024 08:01:18 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=geu26HZK; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf25.hostedemail.com: domain of elver@google.com designates 209.85.221.177 as permitted sender) smtp.mailfrom=elver@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707465678; 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=KsiZ+BblRr7NCTYXiTpbTyVjjF/y9cawt6P+Aca2g38=; b=y2Z6jJCY4pIK7EuTsIYHzgVHXiUVu8xCL3IFn7+JhsCcY46wv27c4plmlU/5N2iJpHoBza VWDRAm8nOoQcZyw7dhR7NvxHQzPPspErOW3r0LS4/qwevpbAJJisKyj+wRC53gVPJIQbIG GKjI2Ddene1kGWOOeN0K6fCmGxZjfPY= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=geu26HZK; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf25.hostedemail.com: domain of elver@google.com designates 209.85.221.177 as permitted sender) smtp.mailfrom=elver@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707465678; a=rsa-sha256; cv=none; b=P7lRIPmDZy4z7rFnox5peCw24BTwsD7rKKkq3G7R3Sr2jerVbUWPlvXvGTbg+z8kUfKMlw 4aOa2ll0Ps02/aF02W96GhbAO0MT/SWaMb9cIQv5HPca5tyJWJqxduRYFhJwC0KkeDYrVP PoTYtewq9Od28uSAYHdbPn5rNM92dU0= Received: by mail-vk1-f177.google.com with SMTP id 71dfb90a1353d-4c01076b9faso297154e0c.2 for ; Fri, 09 Feb 2024 00:01:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1707465677; x=1708070477; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=KsiZ+BblRr7NCTYXiTpbTyVjjF/y9cawt6P+Aca2g38=; b=geu26HZKNI5trEokl1pMBxq1Pl43YfnkeDLpwZAIPh18Tg5w98GeIzswdSWY8KSgNB zt4zpvNsvzTj4hYAw2vJjszWQ/LFGenXoAXdsJC/Re1nqr1Q7LJbhcKh9kXZNSuC5/Xt 4H3YV+lLWir5+sVtug6Z+TB/+39e3D4vR+HfsLI9tsLSmblffYpSOM/hT/oo5FyNTefP PSmNy1zm/JSx0PStqFI8xbZQftb3BcBCVNy+QyUHgqNqh88YWDI42C2SZ5/kgvLvD4ji SweZVlamQkCMPH/CpfPqkJ5AATmA99ucQqunxC71gLJU7yNFO/10CUufpHNLAMuzqskx UdAQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707465677; x=1708070477; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=KsiZ+BblRr7NCTYXiTpbTyVjjF/y9cawt6P+Aca2g38=; b=MvnpfjVpJfX7nPp3nTDyIOX2fw0iaD1/yvdShm3HFG+8hKPpwZDj81trZ/rw0/t63g cU0xrtpDRge3w6Vtc2+u6A3FpOqerjJqhEC719EINn0r2lhKkqgu9JwoA03X1MjUM8Mw BSWizvqcMwbA3Wis5TirU+zBErKR7rDP1ilVoYvL++MxWhvhlEKp8Rt4nX+yIK3dB7BB 1A9Awv7RxLCgyFPwcT9AguUG5sV4LlFq+eIGLqyoPHGd3gElRaoxxWrAoLlVjpbKjAu4 HfTWHpcXhoXSmm7gm/fCWLP7KoXql+vyqAXpwDgezGUzoQsRvYgvv+/4AgJ+FKPVgkfG 2FsA== X-Gm-Message-State: AOJu0YwUUhp7nGZFISaodRm3HPl4K0RcVcC0+/Pm3deH0TTQvNKOgAWS kY1W+wy8DbgGzTV59tE19O1l04C0AtTgdFcMUSqvBk5b8PFcvBt4UbVyNj8dMJ6sC6nUeurafee +WYT09vdoinx7Am/PNh2Tg+jef68Vw/3JtV02 X-Google-Smtp-Source: AGHT+IFXjPjL6iDCeDuIk+TxXSQFdvefy0JvGxD2k7S2flDvVMhe4qrAnu3UCoLe3q+ZZX8j8IKJ2Cw+rrf91bvTYzw= X-Received: by 2002:a1f:e6c1:0:b0:4c0:1a5c:9185 with SMTP id d184-20020a1fe6c1000000b004c01a5c9185mr864523vkh.4.1707465677142; Fri, 09 Feb 2024 00:01:17 -0800 (PST) MIME-Version: 1.0 References: <20240208234539.19113-1-osalvador@suse.de> <20240208234539.19113-4-osalvador@suse.de> In-Reply-To: <20240208234539.19113-4-osalvador@suse.de> From: Marco Elver Date: Fri, 9 Feb 2024 09:00:40 +0100 Message-ID: Subject: Re: [PATCH v7 3/4] mm,page_owner: Display all stacks and their count To: Oscar Salvador Cc: Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Michal Hocko , Vlastimil Babka , Andrey Konovalov , Alexander Potapenko Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 4CD3DA0013 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: ydr5jdptajow6bui6qc39dfd84ch7ebr X-HE-Tag: 1707465678-472434 X-HE-Meta: U2FsdGVkX19eSdgrR6G3D87S4UJkwzCi30djmKVu0+bk7ogA9VY11LPLQypAcyUKo2pNZUOr6uu1EZxxF7DHlU7jeOFgB2UYJKkoslbJz6pEt4S8bY5Gox44FKTEx6srxNPn5tNN6LBtGYZcxd01GW0lMyepPDd1XnA+Srz+QPIrFFRReb9F6HlbwytlAU9G+qeDLGuzSKKskjaHRd7yz1/vtvRB+wGUkykWVYiqT3RcyTinlWOZYGJ6JFtreg0mDZy5sCST2jQK3Q3hDeVtDmlny5GnYx85YQzJ/rvvczu2hZf5QL2bG50D9hjo92L3RuD9nhR9xHsitnV6Um1KmmpmQxOuuIQ6S/zvt2E3psU09i3m6v/NEovVIl2t+/UNfANjNgjGhHnjlbCPCt3OiSTI2zE+46z8TTKpPdcwEZVcvYwRjFls0BJ7IscpDFxraKYj/7zbFjZMa533cn6iPr/ETwVsbIdTJ75LqB4ZLthbDl0g6plg5HsCJvi7hSXbWqUJybfFkAltSjmQ91H1QHGrAJ0xhaxCbu4EgozCc9eZZ91BJtSbBvDedJUVxJX/sQhBxUb/jIKAu4560bSALhunzQGxCkJsM7zF/vxA9QsdAtwelW+33dIaK9lR9QwJbU1Vhc7Psj30V1CCGZlysWhOREzcTOrODAfDNxGAVhEGRNp1/7OXc2hhJjG4efxzmJw7RH7H7P9aYAgxeJvHtNroaGlPeZZgw0bbBQp2e9Zln5TZ8PG28l6f3/siX9Fyd5yU/Wl7jk/F34gItDRzh/gEHldeESjUf4/tHGKfUqVu8GZDyyJ/ILkjW3B6tIGJldzZkxM8stuJMXnRqzCmH2RNFWROIIi6ktCSmVxbLUfwSqw1AcMY7qujG4dO4pK4HVBlKoItyDFLRrnf/cXCLNAB79bn3GbJuYJNKGCCkR3uB2zmtUNJl7UFQPKSSOj/rrOx6sW3zSEbVXEqQYK YFovanJ+ JMimIT8Dr3fQUmp34YHHKdjGtH++QqvFo/LBhRQGap5LMNNamM0s7rLPJ/vBcuHGJ+++V/HIbAet6U58G3ZH2Roc/zMqnMTwNpd361B8TjI0+9Vpw3KDEb0qYvCFtMxCkf7rh13HPPt7qE0mVS2TlE3C9Y0g5laR2PNTqC3eyE3DwrQcqIyk+fKOZTPsVA6nah9jERyi3p6OtbOvypHuWMPCyS+2OKt0zzlz+d9De3LOcu/FENPfE0dY8BI9HAgz/LwJrBoTtM62KKOgJ0H5cbNqYcA== 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: List-Subscribe: List-Unsubscribe: On Fri, 9 Feb 2024 at 00:45, Oscar Salvador wrote: > > This patch adds a new file called 'page_owner_stacks', which > will show all stacks that were added by page_owner followed by > their counting, giving us a clear overview of stack <-> count > relationship. > > E.g: > > prep_new_page+0xa9/0x120 > get_page_from_freelist+0x801/0x2210 > __alloc_pages+0x18b/0x350 > alloc_pages_mpol+0x91/0x1f0 > folio_alloc+0x14/0x50 > filemap_alloc_folio+0xb2/0x100 > __filemap_get_folio+0x14a/0x490 > ext4_write_begin+0xbd/0x4b0 [ext4] > generic_perform_write+0xc1/0x1e0 > ext4_buffered_write_iter+0x68/0xe0 [ext4] > ext4_file_write_iter+0x70/0x740 [ext4] > vfs_write+0x33d/0x420 > ksys_write+0xa5/0xe0 > do_syscall_64+0x80/0x160 > entry_SYSCALL_64_after_hwframe+0x6e/0x76 > stack_count: 4578 > > In order to show all the stacks, we implement stack_depot_get_next_stack(), > which walks all buckets while retrieving the stacks stored in them. > stack_depot_get_next_stack() will return all stacks, one at a time, > by first finding a non-empty bucket, and then retrieving all the stacks > stored in that bucket. > Once we have completely gone through it, we get the next non-empty bucket > and repeat the same steps, and so on until we have completely checked all > buckets. > > Signed-off-by: Oscar Salvador > --- > include/linux/stackdepot.h | 20 +++++++++ > lib/stackdepot.c | 46 +++++++++++++++++++++ > mm/page_owner.c | 85 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 151 insertions(+) > > diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h > index ac62de4d4999..d851ec821e6f 100644 > --- a/include/linux/stackdepot.h > +++ b/include/linux/stackdepot.h > @@ -183,6 +183,26 @@ depot_stack_handle_t stack_depot_save(unsigned long *entries, > */ > struct stack_record *stack_depot_get_stack(depot_stack_handle_t handle); > > +/** > + * stack_depot_get_next_stack - Returns all stacks, one at a time "Returns all stack_records" to be clear that this is returning the struct. > + * > + * @table: Current table we are checking > + * @bucket: Current bucket we are checking > + * @last_found: Last stack that was found > + * > + * This function finds first a non-empty bucket and returns the first stack > + * stored in it. On consequent calls, it walks the bucket to see whether > + * it contains more stacks. > + * Once we have walked all the stacks in a bucket, we check > + * the next one, and we repeat the same steps until we have checked all of them I think for this function it's important to say that no entry returned from this function can be evicted. I.e. the easiest way to ensure this is that the caller makes sure the entries returned are never passed to stack_depot_put() - which is certainly the case for your usecase because you do not use stack_depot_put(). > + * Return: A pointer a to stack_record struct, or NULL when we have walked all > + * buckets. > + */ > +struct stack_record *stack_depot_get_next_stack(unsigned long *table, To keep consistent, I'd also call this __stack_depot_get_next_stack_record(), so that we're clear this is more of an internal function not for general usage. > + struct list_head **bucket, > + struct stack_record **last_found); > + > /** > * stack_depot_fetch - Fetch a stack trace from stack depot > * > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > index 197c355601f9..107bd0174cd6 100644 > --- a/lib/stackdepot.c > +++ b/lib/stackdepot.c > @@ -782,6 +782,52 @@ unsigned int stack_depot_get_extra_bits(depot_stack_handle_t handle) > } > EXPORT_SYMBOL(stack_depot_get_extra_bits); > > +struct stack_record *stack_depot_get_next_stack(unsigned long *table, > + struct list_head **curr_bucket, > + struct stack_record **last_found) > +{ > + struct list_head *bucket = *curr_bucket; > + unsigned long nr_table = *table; > + struct stack_record *found = NULL; > + unsigned long stack_table_entries = stack_hash_mask + 1; > + > + rcu_read_lock_sched_notrace(); We are returning pointers to stack_records out of the RCU-read critical section, which are then later used to continue the iteration. list_for_each_entry_continue_rcu() says this is fine if "... you held some sort of non-RCU reference (such as a reference count) ...". Updating the function's documentation to say none of these entries can be evicted via a stack_depot_put() is required. > + if (!bucket) { > + /* > + * Find a non-empty bucket. Once we have found it, > + * we will use list_for_each_entry_continue_rcu() on the next > + * call to keep walking the bucket. > + */ > +new_table: > + bucket = &stack_table[nr_table]; > + list_for_each_entry_rcu(found, bucket, hash_list) { > + goto out; > + } > + } else { > + /* Check whether we have more stacks in this bucket */ > + found = *last_found; > + list_for_each_entry_continue_rcu(found, bucket, hash_list) { > + goto out; > + } > + } > + > + /* No more stacks in this bucket, check the next one */ > + nr_table++; > + if (nr_table < stack_table_entries) > + goto new_table; > + > + /* We are done walking all buckets */ > + found = NULL; > + > +out: > + *table = nr_table; > + *curr_bucket = bucket; > + *last_found = found; > + rcu_read_unlock_sched_notrace(); > + > + return found; > +} > + > static int stats_show(struct seq_file *seq, void *v) > { > /* > diff --git a/mm/page_owner.c b/mm/page_owner.c > index 0adf41702b9d..aea212734557 100644 > --- a/mm/page_owner.c > +++ b/mm/page_owner.c > @@ -749,6 +749,89 @@ static const struct file_operations proc_page_owner_operations = { > .llseek = lseek_page_owner, > }; > > +struct stack_iterator { > + unsigned long nr_table; > + struct list_head *bucket; > + struct stack_record *last_stack; > +}; > + > +static void *stack_start(struct seq_file *m, loff_t *ppos) > +{ > + struct stack_iterator *iter = m->private; > + > + if (*ppos == -1UL) > + return NULL; > + > + return stack_depot_get_next_stack(&iter->nr_table, > + &iter->bucket, > + &iter->last_stack); > +} > + > +static void *stack_next(struct seq_file *m, void *v, loff_t *ppos) > +{ > + struct stack_iterator *iter = m->private; > + struct stack_record *stack; > + > + stack = stack_depot_get_next_stack(&iter->nr_table, > + &iter->bucket, > + &iter->last_stack); > + *ppos = stack ? *ppos + 1 : -1UL; > + > + return stack; > +} > + > +static int stack_print(struct seq_file *m, void *v) > +{ > + char *buf; > + int ret = 0; > + struct stack_iterator *iter = m->private; > + struct stack_record *stack = iter->last_stack; > + > + if (!stack->size || stack->size < 0 || refcount_read(&stack->count) < 2) > + return 0; > + > + buf = kzalloc(PAGE_SIZE, GFP_KERNEL); > + > + ret += stack_trace_snprint(buf, PAGE_SIZE, stack->entries, stack->size, > + 0); > + if (!ret) > + goto out; > + > + scnprintf(buf + ret, PAGE_SIZE - ret, "stack_count: %d\n\n", > + refcount_read(&stack->count)); > + > + seq_printf(m, buf); > + seq_puts(m, "\n\n"); > +out: > + kfree(buf); > + > + return 0; > +} > + > +static void stack_stop(struct seq_file *m, void *v) > +{ > +} > + > +static const struct seq_operations page_owner_stack_op = { > + .start = stack_start, > + .next = stack_next, > + .stop = stack_stop, > + .show = stack_print > +}; > + > +static int page_owner_stack_open(struct inode *inode, struct file *file) > +{ > + return seq_open_private(file, &page_owner_stack_op, > + sizeof(struct stack_iterator)); > +} > + > +const struct file_operations page_owner_stack_operations = { > + .open = page_owner_stack_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = seq_release, > +}; > + > static int __init pageowner_init(void) > { > if (!static_branch_unlikely(&page_owner_inited)) { > @@ -758,6 +841,8 @@ static int __init pageowner_init(void) > > debugfs_create_file("page_owner", 0400, NULL, NULL, > &proc_page_owner_operations); > + debugfs_create_file("page_owner_stacks", 0400, NULL, NULL, > + &page_owner_stack_operations); > > return 0; > } > -- > 2.43.0 >