Messages in this thread | | | Subject | Re: [PATCH v2] mm: page_alloc: dump migrate-failed pages | From | David Hildenbrand <> | Date | Wed, 10 Mar 2021 09:20:40 +0100 |
| |
On 10.03.21 08:42, Minchan Kim wrote: > On Tue, Mar 09, 2021 at 08:15:41AM -0800, Minchan Kim wrote: > > < snip > > >>> [...] >>>> +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"); >>>> + } >>> >>> Apart from the above, do we have to warn for something that is a >>> debugging aid? A similar concern wrt dump_page which uses pr_warn and >> >> Make sense. >> >>> page owner is using even pr_alert. >>> Would it make sense to add a loglevel parameter both into __dump_page >>> and dump_page_owner? >> >> Let me try it. > > I looked though them and made first draft to clean them up. > > It's bigger than my initial expectaion because there are many callsites > to use dump_page and stack_trace_print inconsistent loglevel. > Since it's not a specific problem for this work, I'd like to deal with > it as separate patchset since I don't want to be stuck on here for my > initial goal.
Why the need to rush regarding your series?
If it will clean up your patch significantly, then I think doing the cleanups first is the proper way to go.
I really don't get why this is a real problem.
-- Thanks,
David / dhildenb
| |