Messages in this thread |  | | Subject | Re: [PATCH] mm: page_alloc: dump migrate-failed pages only at -EBUSY | From | David Hildenbrand <> | Date | Sun, 23 May 2021 14:06:20 +0200 |
| |
On 21.05.21 19:39, Minchan Kim wrote: > On Fri, May 21, 2021 at 10:08:15AM +0200, David Hildenbrand wrote: >> On 20.05.21 22:51, Minchan Kim wrote: >>> On Thu, May 20, 2021 at 09:28:09PM +0200, David Hildenbrand wrote: >>>> Minchan Kim <minchan@kernel.org> schrieb am Do. 20. Mai 2021 um 21:20: >>>> >>>>> On Wed, May 19, 2021 at 02:33:41PM -0700, Minchan Kim wrote: >>>>>> alloc_contig_dump_pages aims for helping debugging page migration >>>>>> failure by page refcount mismatch or something else of page itself >>>>>> from migration handler function. However, in -ENOMEM case, there is >>>>>> nothing to get clue from page descriptor information so just >>>>>> dump pages only when -EBUSY happens. >>>>>> >>>>>> Signed-off-by: Minchan Kim <minchan@kernel.org> >>>>>> --- >>>>>> mm/page_alloc.c | 3 ++- >>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>>>>> index 3100fcb08500..c0a2971dc755 100644 >>>>>> --- a/mm/page_alloc.c >>>>>> +++ b/mm/page_alloc.c >>>>>> @@ -8760,7 +8760,8 @@ static int __alloc_contig_migrate_range(struct >>>>> compact_control *cc, >>>>>> >>>>>> lru_cache_enable(); >>>>>> if (ret < 0) { >>>>>> - alloc_contig_dump_pages(&cc->migratepages); >>>>>> + if (ret == -EBUSY) >>>>>> + alloc_contig_dump_pages(&cc->migratepages); >>>>>> putback_movable_pages(&cc->migratepages); >>>>>> return ret; >>>>>> } >>>>>> -- >>>>>> 2.31.1.751.gd2f1c929bd-goog >>>>>> >>>>> >>>>> Resend with a little modifying description. >>>>> >>>>> From c5a2fea291cf46079b87cc9ac9a25fc7f819d0fd Mon Sep 17 00:00:00 2001 >>>>> From: Minchan Kim <minchan@kernel.org> >>>>> Date: Wed, 19 May 2021 14:22:18 -0700 >>>>> Subject: [PATCH] mm: page_alloc: dump migrate-failed pages only at -EBUSY >>>>> >>>>> alloc_contig_dump_pages aims for helping debugging page migration >>>>> failure by elevated page refcount compared to expected_count. >>>>> (for the detail, please look at migrate_page_move_mapping) >>>>> >>>>> However, -ENOMEM is just the case that system is under memory >>>>> pressure state, not relevant with page refcount at all. Thus, >>>>> the dumping page list is not helpful for the debugging point of view. >>>>> >>>> >>>> what about -ENOMEM when migrating empty/free huge pages? I think there is >>>> value in having the pages dumped to identify something like that. And it >>>> doesn‘t require heavy memory pressure to fail allocating a huge page. >>>> >>> >>> -ENOMEM means there is no memory to alloate destination page. >>> How could it help dumping source pages in those case from dump_page >>> content point of view? >> >> You would spot a huge page in the source list (usually at first position) >> without any obvious migration blockers I assume? > > It was not a huge page case. > >> >> I'm wondering, did you actually run into this being suboptimal? If it's a >> real problem dumping too many stuff when running into -ENOMEM, fine with me. >> If it's a theoretical issue, I'd prefer to just keep it simple as is. > > That's exactly what I encountered. With -ENOMEM, it dumped bunch of > pages on migratepages list. It was just useless with just consuming > logbuffer since there are nothing much to investigate with dumping > source pages. >
Fine with me, then
Reviewed-by: David Hildenbrand <david@redhat.com>
Thanks!
-- Thanks,
David / dhildenb
|  |