All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Minchan Kim <minchan@kernel.org>
Cc: linux-mm <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	John Dias <joaodias@google.com>, Michal Hocko <mhocko@suse.com>,
	David Hildenbrand <david@redhat.com>,
	Jason Baron <jbaron@akamai.com>
Subject: Re: [PATCH v2] mm: page_alloc: dump migrate-failed pages
Date: Mon, 8 Mar 2021 16:21:28 -0800	[thread overview]
Message-ID: <20210308162128.9b4a7d4c1576a72fd4878bdb@linux-foundation.org> (raw)
In-Reply-To: <20210308202047.1903802-1-minchan@kernel.org>

On Mon,  8 Mar 2021 12:20:47 -0800 Minchan Kim <minchan@kernel.org> wrote:

> alloc_contig_range is usually used on cma area or movable zone.
> It's critical if the page migration fails on those areas so
> dump more debugging message.
> 
> 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.
> 
> Admin could enable the dump like this(by default, disabled)
> 
> 	echo "func dump_migrate_failure_pages +p" > control
> 
> Admin could disable it.
> 
> 	echo "func dump_migrate_failure_pages =_" > control
> 
> ...
>
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -8453,6 +8453,34 @@ static unsigned long pfn_max_align_up(unsigned long pfn)
>  				pageblock_nr_pages));
>  }
>  
> +#if defined(CONFIG_DYNAMIC_DEBUG) || \
> +	(defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE))
> +static DEFINE_RATELIMIT_STATE(alloc_contig_ratelimit_state,
> +		DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST);
> +int alloc_contig_ratelimit(void)
> +{
> +	return __ratelimit(&alloc_contig_ratelimit_state);
> +}

Wow, that's an eyesore.  We're missing helpers in the ratelimit code. 
Can we do something like

/* description goes here */
#define RATELIMIT2(interval, burst)
({
	static DEFINE_RATELIMIT_STATE(_rs, interval, burst);

	__ratelimit(_rs);
})

#define RATELIMIT()
	RATELIMIT2(DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST)

> +void dump_migrate_failure_pages(struct list_head *page_list)
> +{
> +	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor,
> +			"migrate failure");
> +	if (DYNAMIC_DEBUG_BRANCH(descriptor) &&
> +			alloc_contig_ratelimit()) {
> +		struct page *page;
> +
> +		WARN(1, "failed callstack");
> +		list_for_each_entry(page, page_list, lru)
> +			dump_page(page, "migration failure");
> +	}
> +}

Then we can simply do

	if (DYNAMIC_DEBUG_BRANCH(descriptor) && RATELIMIT())



  parent reply	other threads:[~2021-03-09  0:22 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-08 20:20 [PATCH v2] mm: page_alloc: dump migrate-failed pages Minchan Kim
2021-03-08 21:29 ` kernel test robot
2021-03-08 21:29   ` kernel test robot
2021-03-08 22:29   ` Minchan Kim
2021-03-08 22:29     ` Minchan Kim
2021-03-09  0:41     ` [kbuild-all] " Rong Chen
2021-03-09  0:41       ` Rong Chen
2021-03-09  2:23       ` [kbuild-all] " Minchan Kim
2021-03-09  2:23         ` Minchan Kim
2021-03-09  0:21 ` Andrew Morton [this message]
2021-03-09  2:21   ` Minchan Kim
2021-03-09  0:30 ` kernel test robot
2021-03-09  0:30   ` kernel test robot
2021-03-09  0:30 ` [RFC PATCH] mm: page_alloc: alloc_contig_ratelimit() can be static kernel test robot
2021-03-09  0:30   ` kernel test robot
2021-03-09  9:32 ` [PATCH v2] mm: page_alloc: dump migrate-failed pages Michal Hocko
2021-03-09 16:15   ` Minchan Kim
2021-03-09 16:32     ` Michal Hocko
2021-03-09 17:27       ` Minchan Kim
2021-03-10 13:04         ` Michal Hocko
2021-03-10 15:59           ` Minchan Kim
2021-03-10  7:42     ` Minchan Kim
2021-03-10  8:20       ` David Hildenbrand
2021-03-10 15:45         ` Minchan Kim
2021-03-10 13:07       ` Michal Hocko
2021-03-10 16:05         ` Minchan Kim
2021-03-10 16:46           ` Michal Hocko
2021-03-10 17:06             ` Minchan Kim
2021-03-10 18:07               ` Minchan Kim
2021-03-10 13:26   ` Matthew Wilcox
2021-03-10 13:54     ` Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210308162128.9b4a7d4c1576a72fd4878bdb@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=jbaron@akamai.com \
    --cc=joaodias@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=minchan@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.