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=-10.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_2 autolearn=unavailable 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 66FAAC43460 for ; Fri, 2 Apr 2021 11:50:52 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id D13CF61151 for ; Fri, 2 Apr 2021 11:50:51 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D13CF61151 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gentoo.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 2F0CC8D0003; Fri, 2 Apr 2021 07:50:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2C8378D0002; Fri, 2 Apr 2021 07:50:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1A2DB8D0003; Fri, 2 Apr 2021 07:50:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0054.hostedemail.com [216.40.44.54]) by kanga.kvack.org (Postfix) with ESMTP id F3EBD8D0002 for ; Fri, 2 Apr 2021 07:50:50 -0400 (EDT) Received: from smtpin27.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id B458E80604C0 for ; Fri, 2 Apr 2021 11:50:50 +0000 (UTC) X-FDA: 77987260260.27.7FE3747 Received: from smtp.gentoo.org (smtp.gentoo.org [140.211.166.183]) by imf18.hostedemail.com (Postfix) with ESMTP id 5C44A2000244 for ; Fri, 2 Apr 2021 11:50:47 +0000 (UTC) Date: Fri, 2 Apr 2021 12:50:39 +0100 From: Sergei Trofimovich To: Andrew Morton Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Daniel Bristot de Oliveira Subject: Re: [PATCH] mm: page_owner: detect page_owner recursion via task_struct Message-ID: <20210402125039.671f1f40@sf> In-Reply-To: <20210401170519.00824fbdf8ab60b720609422@linux-foundation.org> References: <20210401223010.3580480-1-slyfox@gentoo.org> <20210401170519.00824fbdf8ab60b720609422@linux-foundation.org> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 5C44A2000244 X-Stat-Signature: bw5nsoikm63xdbzibjwhedxiiwpnwryw Received-SPF: none (gentoo.org>: No applicable sender policy available) receiver=imf18; identity=mailfrom; envelope-from=""; helo=smtp.gentoo.org; client-ip=140.211.166.183 X-HE-DKIM-Result: none/none X-HE-Tag: 1617364247-50111 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 Thu, 1 Apr 2021 17:05:19 -0700 Andrew Morton wrote: > On Thu, 1 Apr 2021 23:30:10 +0100 Sergei Trofimovich wrote: > > > Before the change page_owner recursion was detected via fetching > > backtrace and inspecting it for current instruction pointer. > > It has a few problems: > > - it is slightly slow as it requires extra backtrace and a linear > > stack scan of the result > > - it is too late to check if backtrace fetching required memory > > allocation itself (ia64's unwinder requires it). > > > > To simplify recursion tracking let's use page_owner recursion depth > > as a counter in 'struct task_struct'. > > Seems like a better approach. > > > The change make page_owner=on work on ia64 bu avoiding infinite > > recursion in: > > kmalloc() > > -> __set_page_owner() > > -> save_stack() > > -> unwind() [ia64-specific] > > -> build_script() > > -> kmalloc() > > -> __set_page_owner() [we short-circuit here] > > -> save_stack() > > -> unwind() [recursion] > > > > ... > > > > --- a/include/linux/sched.h > > +++ b/include/linux/sched.h > > @@ -1371,6 +1371,15 @@ struct task_struct { > > struct llist_head kretprobe_instances; > > #endif > > > > +#ifdef CONFIG_PAGE_OWNER > > + /* > > + * Used by page_owner=on to detect recursion in page tracking. > > + * Is it fine to have non-atomic ops here if we ever access > > + * this variable via current->page_owner_depth? > > Yes, it is fine. This part of the comment can be removed. Cool! Will do. > > + */ > > + unsigned int page_owner_depth; > > +#endif > > Adding to the task_struct has a cost. But I don't expect that > PAGE_OWNER is commonly used in prodction builds (correct?). Yeah, PAGE_OWNER should not be enabled for production kernels. Not having extra memory overhead (or layout disruption) is a nice benefit though. I'll switch to "Unserialized, strictly 'current'" bitfield. > > --- a/init/init_task.c > > +++ b/init/init_task.c > > @@ -213,6 +213,9 @@ struct task_struct init_task > > #ifdef CONFIG_SECCOMP > > .seccomp = { .filter_count = ATOMIC_INIT(0) }, > > #endif > > +#ifdef CONFIG_PAGE_OWNER > > + .page_owner_depth = 0, > > +#endif > > }; > > EXPORT_SYMBOL(init_task); > > It will be initialized to zero by the compiler. We can omit this hunk > entirely. > > > --- a/mm/page_owner.c > > +++ b/mm/page_owner.c > > @@ -20,6 +20,16 @@ > > */ > > #define PAGE_OWNER_STACK_DEPTH (16) > > > > +/* > > + * How many reenters we allow to page_owner. > > + * > > + * Sometimes metadata allocation tracking requires more memory to be allocated: > > + * - when new stack trace is saved to stack depot > > + * - when backtrace itself is calculated (ia64) > > + * Instead of falling to infinite recursion give it a chance to recover. > > + */ > > +#define PAGE_OWNER_MAX_RECURSION_DEPTH (1) > > So this is presently a boolean. Is there any expectation that > PAGE_OWNER_MAX_RECURSION_DEPTH will ever be greater than 1? If not, we > could use a single bit in the task_struct. Add it to the > "Unserialized, strictly 'current'" bitfields. Could make it a 2-bit field if we want > to permit PAGE_OWNER_MAX_RECURSION_DEPTH=larger. Let's settle on depth=1. depth>1 is not trivial for other reasons I don't completely understand. Follow-up patch incoming. -- Sergei