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;
Sender: Minchan Kim <minchan.kim@gmail.com>
Date:   Wed, 10 Mar 2021 09:06:48 -0800
From:   Minchan Kim <minchan@kernel.org>
To:     Michal Hocko <mhocko@suse.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        linux-mm <linux-mm@kvack.org>,
        LKML <linux-kernel@vger.kernel.org>,
        John Dias <joaodias@google.com>,
        David Hildenbrand <david@redhat.com>,
        Jason Baron <jbaron@akamai.com>
Subject: Re: [PATCH v2] mm: page_alloc: dump migrate-failed pages
Message-ID: <YEj8qACYFmbckWk7@google.com>
References: <20210308202047.1903802-1-minchan@kernel.org>
 <YEdAw6gnp9XxoWUQ@dhcp22.suse.cz>
 <YEefLYiX6rF3Uk4E@google.com>
 <YEh4doXvyuRl5BDB@google.com>
 <YEjEefQpBHV5eBXj@dhcp22.suse.cz>
 <YEjuUHBDKu2uX4EO@google.com>
 <YEj4AGuBn/Q4CIuY@dhcp22.suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YEj4AGuBn/Q4CIuY@dhcp22.suse.cz>
Precedence: bulk

On Wed, Mar 10, 2021 at 05:46:56PM +0100, Michal Hocko wrote:
> On Wed 10-03-21 08:05:36, Minchan Kim wrote:
> > On Wed, Mar 10, 2021 at 02:07:05PM +0100, Michal Hocko wrote:
> [...]
> > > The is a lot of churn indeed. Have you considered adding $FOO_lglvl
> > > variants for those so that you can use them for your particular case
> > > without affecting most of existing users? Something similar we have
> > > discussed in other email thread regarding lru_add_drain_all?
> > 
> > I thought that way but didn't try since it couldn't make them
> > atomic(For example, other printk place in other context will
> > affect by the $FOO_lglvl).
> 
> I do not follow. I meant something like the following (likely incomplete
> but you should get an idea).

Oh, I thought you wanted to override loglevel temporally.

old_lvl = save_printk_lvl(new level);
dump_page();
restore_printk_lvl(old_lvl);

> 
> diff --git a/include/linux/page_owner.h b/include/linux/page_owner.h
> index 3468794f83d2..71b402eb8f78 100644
> --- a/include/linux/page_owner.h
> +++ b/include/linux/page_owner.h
> @@ -14,7 +14,7 @@ extern void __set_page_owner(struct page *page,
>  extern void __split_page_owner(struct page *page, unsigned int nr);
>  extern void __copy_page_owner(struct page *oldpage, struct page *newpage);
>  extern void __set_page_owner_migrate_reason(struct page *page, int reason);
> -extern void __dump_page_owner(struct page *page);
> +extern void __dump_page_owner(struct page *page, const char *loglvl);
>  extern void pagetypeinfo_showmixedcount_print(struct seq_file *m,
>  					pg_data_t *pgdat, struct zone *zone);
>  
> @@ -46,10 +46,10 @@ static inline void set_page_owner_migrate_reason(struct page *page, int reason)
>  	if (static_branch_unlikely(&page_owner_inited))
>  		__set_page_owner_migrate_reason(page, reason);
>  }
> -static inline void dump_page_owner(struct page *page)
> +static inline void dump_page_owner(struct page *page, const char *loglvl)
>  {
>  	if (static_branch_unlikely(&page_owner_inited))
> -		__dump_page_owner(page);
> +		__dump_page_owner(page, loglvl);
>  }
>  #else
>  static inline void reset_page_owner(struct page *page, unsigned int order)
> @@ -69,7 +69,7 @@ static inline void copy_page_owner(struct page *oldpage, struct page *newpage)
>  static inline void set_page_owner_migrate_reason(struct page *page, int reason)
>  {
>  }
> -static inline void dump_page_owner(struct page *page)
> +static inline void dump_page_owner(struct page *page, const char *loglvl)
>  {
>  }
>  #endif /* CONFIG_PAGE_OWNER */
> diff --git a/kernel/stacktrace.c b/kernel/stacktrace.c
> index 9f8117c7cfdd..1b13135d9916 100644
> --- a/kernel/stacktrace.c
> +++ b/kernel/stacktrace.c
> @@ -14,6 +14,18 @@
>  #include <linux/kallsyms.h>
>  #include <linux/stacktrace.h>
>  
> +void __stack_trace_print(const unsigned long *entries, unsigned int nr_entries,
> +		       int spacesconst, char *loglvl)
> +{
> +	unsigned int i;
> +
> +	if (WARN_ON(!entries))
> +		return;
> +
> +	for (i = 0; i < nr_entries; i++)
> +		printk("%s%*c%pS\n", loglvl, 1 + spaces, ' ', (void *)entries[i]);
> +}

That's exactly I did with introducing pr_loglevel. I wanted to address
*all places* to use dump_page and stack_trace_print since some folks
might ask me to fix all the broken place all at once. I'm getting tired
with such hassle.

void dump_page(const char *log_lvl, struct page *page, const char *reason)
{
        __dump_page(log_lvl, page, reason);
        dump_page_owner(log_lvl, page);
}
EXPORT_SYMBOL(dump_page);

/**
 * pr_loglevel - Print an loglevel message
 * @level: loglevel
 * @fmt: format string
 * @...: arguments for the format string
 *
 * This macro expands to a printk with @loglevel. It uses pr_fmt() to
 * generate the format string.
 */
#define pr_loglevel(level, fmt, ...) \
        printk("%s" pr_fmt(fmt), level, ##__VA_ARGS__)

void __dump_page(const char *log_lvl, struct page *page, const char *reason)
{
..
..
        if (page_poisoned) {
                pr_loglevel(log_lvl, "page:%px is uninitialized and poisoned", page);
                goto hex_only;
        }
..
}

static inline void dump_page_owner(const char *log_lvl, struct page *page)
{
        if (static_branch_unlikely(&page_owner_inited))
                __dump_page_owner(log_lvl, page);
}

void stack_trace_print(const char *log_lvl, const unsigned long *entries,
                unsigned int nr_entries, int spaces)
{
        unsigned int i;

        if (WARN_ON(!entries))
                return;

        for (i = 0; i < nr_entries; i++)
                pr_loglevel(log_lvl, "%*c%pS\n", 1 + spaces, ' ', (void *)entries[i]);
}
EXPORT_SYMBOL_GPL(stack_trace_print);