Received: by 2002:a05:6a10:9848:0:0:0:0 with SMTP id x8csp889939pxf; Thu, 1 Apr 2021 17:06:18 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzCBhc+aHxR683sU14yBIxRopomZETXbBqpsfhBPqXI9Ovp+1DaFBDcfW3BVcvfGE4yzmSd X-Received: by 2002:aa7:c815:: with SMTP id a21mr12793304edt.38.1617321978520; Thu, 01 Apr 2021 17:06:18 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1617321978; cv=none; d=google.com; s=arc-20160816; b=r0+M6ZvKUa4S/1WW2GXN5BVZCZj+s0IKvhnizm7JXKPkzPCJR7KwOX+6SSOTEsvpwq 0Z0HabSqY5yPnvKHX//s5ZcgSV4Gld+PZptSyMhgYICK1o2ptTo8mpvSPzpZqkT7kK1u qmdphjwh5Bv/uFUMb/skJISZeKw7T8HtiYr3Qe5CVGVWlWHK3ZFkIw4cJmYytgb2Y3RF SqLlahqUauAduUxwYSw4etGZPXzsWhQ632dc9v8joZCyuqDsUmSfn3t7zCvg2R8S9giS 9YaUwNyPMz6vAXjin/UAKHeBtc0I3xFkhu90jWlG5aIDp43vof6HRMxh/Hd+MjZM3Do0 ZCeA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:subject:cc:to:from:date :dkim-signature; bh=MFw0DJ7aBpmWrOXu412Veds3LJfrGqXf2jCNM9cFdx4=; b=ax1kUiyCSrN8mnlLb7Qa3iJb3Z22kTfa3OUC/uyySYYGLp1Zo5fGWZMx+JFiwQnmQj GGPDadxoP3fxuQ/RSOCcNDbDL15n2QJ7IdQ+G4HwQ86aTxH0L4sYII9drVL92gz8Mkzt 2+srP5LA1Dauoy1K2ytd2PTs9hkSP7MfBxsz7B08yYvVzGOTYT9N/584YdlS6uV3PmQl dIjvkbVlx7JKPao/nMpmKYo6z12Xz0s/MEMDJ/0uIsMlvDkqyJa5TYiT1Sxlqjh+H6zx 8gq1zCfGVtAHffhTQmfYCmVAZJ7l6EaJWLNCFLrc2rhyBtaQS/eyUj6REcCYlwYD1mnr 0r6g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux-foundation.org header.s=korg header.b="pN/xBbiu"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id g21si4925257eje.108.2021.04.01.17.05.55; Thu, 01 Apr 2021 17:06:18 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linux-foundation.org header.s=korg header.b="pN/xBbiu"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235296AbhDBAFY (ORCPT + 99 others); Thu, 1 Apr 2021 20:05:24 -0400 Received: from mail.kernel.org ([198.145.29.99]:55814 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233677AbhDBAFU (ORCPT ); Thu, 1 Apr 2021 20:05:20 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id C2F4F60FED; Fri, 2 Apr 2021 00:05:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1617321920; bh=6DTceYIxopB6FCcqzfOkPPslBYRaOV3hFwQSWjZGRzg=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=pN/xBbiuAx3DN06+F4R7VfLgYVMi6hoHcURfdtJSqHT1c4Fl3nRJhPAW4cV7BNcq2 H7iJf/vOIKAUGLliWJI0CPFbUqtrdhx6L+LIJ/mTyPbUXAvNnthFpADKeqDQBtGzeo fkats2jlgZJGMrM0al1pxjEaop2kEV1rxPeTWNtY= Date: Thu, 1 Apr 2021 17:05:19 -0700 From: Andrew Morton To: Sergei Trofimovich 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: <20210401170519.00824fbdf8ab60b720609422@linux-foundation.org> In-Reply-To: <20210401223010.3580480-1-slyfox@gentoo.org> References: <20210401223010.3580480-1-slyfox@gentoo.org> X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > + */ > + 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?). > --- 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.