On Sun, Jan 18, 2015 at 6:19 PM, Vlastimil Babka <vbabka@xxxxxxx> wrote: > On 18.1.2015 10:17, Hui Zhu wrote: >> >> From: Hui Zhu <zhuhui@xxxxxxxxxx> >> >> To test the patch [1], I use KGTP and a script [2] to show >> NR_FREE_CMA_PAGES >> and gross of cma_nr_free. The values are always not same. >> I check the code of pages alloc and free and found that race conditions >> on getting migratetype in buffered_rmqueue. > > > Can you elaborate? What does this races with, are you dynamically changing > the size of CMA area, or what? The migratetype here is based on which free > list the page was found on. Was it misplaced then? Wasn't Joonsoo's recent > series supposed to eliminate this? My bad. I thought move_freepages has race condition with this part. But I missed it will check PageBuddy before set_freepage_migratetype. Sorry for that. I will do more work around this one and [1]. Thanks for your review. Best, Hui > >> Then I add move the code of getting migratetype inside the zone->lock >> protection part. > > > Not just that, you are also reading migratetype from pageblock bitmap > instead of the one embedded in the free page. Which is more expensive > and we already do that more often than we would like to because of CMA. > And it appears to be a wrong fix for a possible misplacement bug. If there's > such misplacement, the wrong stats are not the only problem. > >> >> Because this issue will affect system even if the Linux kernel does't >> have [1]. So I post this patch separately. > > > But we can't test that without [1], right? Maybe the issue is introduced by > [1]? > > >> >> This patchset is based on fc7f0dd381720ea5ee5818645f7d0e9dece41cb0. >> >> [1] https://lkml.org/lkml/2015/1/18/28 >> [2] https://github.com/teawater/kgtp/blob/dev/add-ons/cma_free.py >> >> Signed-off-by: Hui Zhu <zhuhui@xxxxxxxxxx> >> --- >> mm/page_alloc.c | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index 7633c50..f3d6922 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -1694,11 +1694,12 @@ again: >> } >> spin_lock_irqsave(&zone->lock, flags); >> page = __rmqueue(zone, order, migratetype); >> + if (page) >> + migratetype = get_pageblock_migratetype(page); >> + else >> + goto failed_unlock; >> spin_unlock(&zone->lock); >> - if (!page) >> - goto failed; >> - __mod_zone_freepage_state(zone, -(1 << order), >> - get_freepage_migratetype(page)); >> + __mod_zone_freepage_state(zone, -(1 << order), >> migratetype); >> } >> __mod_zone_page_state(zone, NR_ALLOC_BATCH, -(1 << order)); >> @@ -1715,6 +1716,8 @@ again: >> goto again; >> return page; >> +failed_unlock: >> + spin_unlock(&zone->lock); >> failed: >> local_irq_restore(flags); >> return NULL; > > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>