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=-15.1 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham 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 D682FC433E0 for ; Thu, 11 Mar 2021 18:53:10 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3DA5964FE0 for ; Thu, 11 Mar 2021 18:53:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3DA5964FE0 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B027F8D02E1; Thu, 11 Mar 2021 13:53:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id AD8858D02B2; Thu, 11 Mar 2021 13:53:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 979CE8D02E1; Thu, 11 Mar 2021 13:53:09 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0075.hostedemail.com [216.40.44.75]) by kanga.kvack.org (Postfix) with ESMTP id 7A00E8D02B2 for ; Thu, 11 Mar 2021 13:53:09 -0500 (EST) Received: from smtpin29.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 3E99D824999B for ; Thu, 11 Mar 2021 18:53:09 +0000 (UTC) X-FDA: 77908490898.29.2831B52 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf03.hostedemail.com (Postfix) with ESMTP id 1A166C0007D2 for ; Thu, 11 Mar 2021 18:53:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1615488788; h=from:from: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; bh=fZ+OkiReHiMwmMnVJ4gBg5WJotQNpULPKr0L4tI8odM=; b=iNs0UjFIbyKP6aTGd5KrFh0MRTu5Rh1Nd3HnFcP8sY8a70iYeoZUpim+pYi2tuWXXMuEUF RVkvleEMCy2650SnvyjY9/3wzbn4Jqd1apqvdTFDlNui0xNO/TpEttTu/hYujJKZSOfx4b A1ayWcVlNYai1WM2UFmulvS7xAheZWo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-86-YdhZSmjgMZSkln9fb6UdSg-1; Thu, 11 Mar 2021 13:53:05 -0500 X-MC-Unique: YdhZSmjgMZSkln9fb6UdSg-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 75788801596; Thu, 11 Mar 2021 18:53:04 +0000 (UTC) Received: from [10.36.115.26] (ovpn-115-26.ams2.redhat.com [10.36.115.26]) by smtp.corp.redhat.com (Postfix) with ESMTP id D17FB5C1C5; Thu, 11 Mar 2021 18:53:02 +0000 (UTC) To: Minchan Kim , Andrew Morton Cc: linux-mm , LKML , John Dias , Michal Hocko , Jason Baron References: <20210311183047.805891-1-minchan@kernel.org> From: David Hildenbrand Organization: Red Hat GmbH Subject: Re: [PATCH v4] mm: page_alloc: dump migrate-failed pages Message-ID: Date: Thu, 11 Mar 2021 19:53:01 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: <20210311183047.805891-1-minchan@kernel.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Stat-Signature: 4k8jb11ctrysquu1jxfejpugq6jy8dqh X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 1A166C0007D2 Received-SPF: none (redhat.com>: No applicable sender policy available) receiver=imf03; identity=mailfrom; envelope-from=""; helo=us-smtp-delivery-124.mimecast.com; client-ip=170.10.133.124 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1615488785-490408 Content-Transfer-Encoding: quoted-printable 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 11.03.21 19:30, Minchan Kim wrote: > Currently, debugging CMA allocation failures is quite limited. > The most commong source of these failures seems to be page s/commong/common/ > migration which doesn't provide any useful information on the > reason of the failure by itself. alloc_contig_range can report > those failures as it holds a list of migrate-failed pages. >=20 > page refcount, mapcount with page flags on dump_page are > helpful information to deduce the culprit. Furthermore, > dump_page_owner was super helpful to find long term pinner > who initiated the page allocation. Maybe simply "The information logged by dump_page() has already proven=20 helpful for debugging allocation issues, like identifying long-term=20 pinnings on ZONE_MOVABLE or MIGRATE_CMA." >=20 > The reason it approach with dynamic debug is the debug message > could emit lots of noises as alloc_contig_range calls more > frequently since it's a best effort allocator. "Let's use the dynamic debugging infrastructure, such that we avoid flooding the logs and creating a lot of noise on frequent=20 alloc_contig_range() calls. This information is helpful for debugging onl= y." >>> >=20 > There are two ifdefery conditions to support common dyndbg options: >=20 > - CONFIG_DYNAMIC_DEBUG_CORE && DYNAMIC_DEBUG_MODULE > It aims for supporting the feature with only specific file > with adding ccflags. >=20 > - CONFIG_DYNAMIC_DEBUG > It aims for supporting the feature with system wide globally. >=20 > A simple example to enable the feature: >=20 > Admin could enable the dump like this(by default, disabled) >=20 > echo "func alloc_contig_dump_pages +p" > control >=20 > Admin could disable it. >=20 > echo "func alloc_contig_dump_pages =3D_" > control >=20 > Detail goes Documentation/admin-guide/dynamic-debug-howto.rst <<< I'd drop that completely and only mention: "For details on dynamic debugging, see=20 Documentation/admin-guide/dynamic-debug-howto.rst." As you have usage in the code itself, I think you don't have to be=20 repetitive. The ifdeffery seems to be common (e.g.,=20 include/linux/netdevice.) for dynamic debugging users, so I don't see=20 the need to describe that in detail. >>> >=20 > A concern is utility functions in dump_page uses inconsistent > loglevels. >=20 > __dump_page: KERN_WARNING > __dump_page_owner: KERN_ALERT > stack_trace_print: KERN_DEFAULT >=20 > There are bunch of places to use the inconsistent loglevel > utility functions(e.g., just grep dump_page/strace_trace_print). > It's unfortunate but here we are. It could be addressed > different patchset. <<< I'd drop that completely and mention "In the future, we might want to make the loglevels used inside=20 dump_page() consistent and eventually rework the way we log the=20 information here. See [1]" Where [1] is a link to the discussion. >=20 > Signed-off-by: Minchan Kim > --- > * from v3 - https://lore.kernel.org/linux-mm/20210310180104.517886-1-mi= nchan@kernel.org > * add dyndgb usage comment - akpm > * use dumpstack instead of warn_on - david >=20 > * from v2 - https://lore.kernel.org/linux-mm/20210308202047.1903802-1-m= inchan@kernel.org/ > * remove ratelimit - mhocko >=20 > * from v1 - https://lore.kernel.org/linux-mm/20210217163603.429062-1-mi= nchan@kernel.org/ > * use dynamic debugging with system wide instead of per-call site - = mhocko >=20 > mm/page_alloc.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) >=20 Minor nits: > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 3e4b29ee2b1e..76fc202cb105 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -8453,6 +8453,36 @@ static unsigned long pfn_max_align_up(unsigned l= ong pfn) > pageblock_nr_pages)); > } > =20 > +#if defined(CONFIG_DYNAMIC_DEBUG) || \ > + (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE)) > +/* > + * usage) "usage)" looks wrong here. Did you mean "Usage:" > + * dyndbg_dir=3D"/sys/kernel/debug/dynamic_debug" > + * To enable: > + * echo "func alloc_contig_dump_pages +p" > $dyndbg_dir/control > + * To disable: > + * echo "func alloc_contig_dump_pages =3D_" > $dyndbg_dir/control > + * For detail, read dynamic-debug-howto.rst Maybe simply "See admin-guide/dynamic-debug-howto.rst" > + */ > +static void alloc_contig_dump_pages(struct list_head *page_list) > +{ > + DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, > + "migrate failure"); You can fit that into a single line. > + > + if (DYNAMIC_DEBUG_BRANCH(descriptor)) { > + struct page *page; > + > + dump_stack(); > + list_for_each_entry(page, page_list, lru) > + dump_page(page, "migration failure"); > + } > +} > +#else > +static inline void alloc_contig_dump_pages(struct list_head *page_list= ) > +{ > +} > +#endif > + > /* [start, end) must belong to a single zone. */ > static int __alloc_contig_migrate_range(struct compact_control *cc, > unsigned long start, unsigned long end) > @@ -8496,6 +8526,7 @@ static int __alloc_contig_migrate_range(struct co= mpact_control *cc, > NULL, (unsigned long)&mtc, cc->mode, MR_CONTIG_RANGE); > } > if (ret < 0) { > + alloc_contig_dump_pages(&cc->migratepages); > putback_movable_pages(&cc->migratepages); > return ret; > } >=20 As I said, for my taste good enough for now. I would certainly preferred=20 what Michal suggested (e.g., doing it via debug loglevels), but this=20 gets the job done and is not too ugly. Reviewed-by: David Hildenbrand --=20 Thanks, David / dhildenb