| Message ID | 20160304043232.GC12036@js1304-P5Q-DELUXE (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2016/3/4 12:32, Joonsoo Kim wrote: > On Fri, Mar 04, 2016 at 11:02:33AM +0900, Joonsoo Kim wrote: >> On Thu, Mar 03, 2016 at 08:49:01PM +0800, Hanjun Guo wrote: >>> On 2016/3/3 15:42, Joonsoo Kim wrote: >>>> 2016-03-03 10:25 GMT+09:00 Laura Abbott <labbott@redhat.com>: >>>>> (cc -mm and Joonsoo Kim) >>>>> >>>>> >>>>> On 03/02/2016 05:52 AM, Hanjun Guo wrote: >>>>>> Hi, >>>>>> >>>>>> I came across a suspicious error for CMA stress test: >>>>>> >>>>>> Before the test, I got: >>>>>> -bash-4.3# cat /proc/meminfo | grep Cma >>>>>> CmaTotal: 204800 kB >>>>>> CmaFree: 195044 kB >>>>>> >>>>>> >>>>>> After running the test: >>>>>> -bash-4.3# cat /proc/meminfo | grep Cma >>>>>> CmaTotal: 204800 kB >>>>>> CmaFree: 6602584 kB >>>>>> >>>>>> So the freed CMA memory is more than total.. >>>>>> >>>>>> Also the the MemFree is more than mem total: >>>>>> >>>>>> -bash-4.3# cat /proc/meminfo >>>>>> MemTotal: 16342016 kB >>>>>> MemFree: 22367268 kB >>>>>> MemAvailable: 22370528 kB >>> [...] >>>>> I played with this a bit and can see the same problem. The sanity >>>>> check of CmaFree < CmaTotal generally triggers in >>>>> __move_zone_freepage_state in unset_migratetype_isolate. >>>>> This also seems to be present as far back as v4.0 which was the >>>>> first version to have the updated accounting from Joonsoo. >>>>> Were there known limitations with the new freepage accounting, >>>>> Joonsoo? >>>> I don't know. I also played with this and looks like there is >>>> accounting problem, however, for my case, number of free page is slightly less >>>> than total. I will take a look. >>>> >>>> Hanjun, could you tell me your malloc_size? I tested with 1 and it doesn't >>>> look like your case. >>> I tested with malloc_size with 2M, and it grows much bigger than 1M, also I >>> did some other test: >> Thanks! Now, I can re-generate erronous situation you mentioned. >> >>> - run with single thread with 100000 times, everything is fine. >>> >>> - I hack the cam_alloc() and free as below [1] to see if it's lock issue, with >>> the same test with 100 multi-thread, then I got: >> [1] would not be sufficient to close this race. >> >> Try following things [A]. And, for more accurate test, I changed code a bit more >> to prevent kernel page allocation from cma area [B]. This will prevent kernel >> page allocation from cma area completely so we can focus cma_alloc/release race. >> >> Although, this is not correct fix, it could help that we can guess >> where the problem is. > More correct fix is something like below. > Please test it. Hmm, this is not working: -bash-4.3# cat /proc/meminfo |grep Cma CmaTotal: 204800 kB CmaFree: 19388216 kB -bash-4.3# cat /proc/meminfo MemTotal: 16342016 kB MemFree: 35146212 kB MemAvailable: 35158008 kB Buffers: 4236 kB Cached: 45032 kB SwapCached: 0 kB Active: 19276 kB Inactive: 36492 kB Active(anon): 6724 kB Inactive(anon): 52 kB Active(file): 12552 kB Inactive(file): 36440 kB Unevictable: 0 kB Mlocked: 0 kB SwapTotal: 0 kB SwapFree: 0 kB Dirty: 0 kB Writeback: 0 kB AnonPages: 6524 kB Mapped: 24724 kB Shmem: 264 kB Slab: 26948 kB SReclaimable: 6260 kB SUnreclaim: 20688 kB KernelStack: 3296 kB PageTables: 400 kB NFS_Unstable: 0 kB Bounce: 0 kB WritebackTmp: 0 kB CommitLimit: 8171008 kB Committed_AS: 32764 kB VmallocTotal: 258998208 kB VmallocUsed: 0 kB VmallocChunk: 0 kB AnonHugePages: 0 kB CmaTotal: 204800 kB CmaFree: 19388216 kB HugePages_Total: 0 HugePages_Free: 0 HugePages_Rsvd: 0 HugePages_Surp: 0 Hugepagesize: 2048 kB Thanks Hanjun > > It checks problematic buddy merging and prevent it. > I will try to find another way that is less intrusive for freepath performance. > > Thanks. > > ---------------->8----------------------- > >From 855cb11368487a0f02a5ad5b3d9de375dfbb061c Mon Sep 17 00:00:00 2001 > From: Joonsoo Kim <iamjoonsoo.kim@lge.com> > Date: Fri, 4 Mar 2016 13:28:17 +0900 > Subject: [PATCH] mm/cma: fix race > > Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com> > --- > mm/page_alloc.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index c6c38ed..a01c3b5 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -620,8 +620,8 @@ static inline void rmv_page_order(struct page *page) > * > * For recording page's order, we use page_private(page). > */ > -static inline int page_is_buddy(struct page *page, struct page *buddy, > - unsigned int order) > +static inline int page_is_buddy(struct zone *zone, struct page *page, > + struct page *buddy, unsigned int order) > { > if (!pfn_valid_within(page_to_pfn(buddy))) > return 0; > @@ -644,6 +644,12 @@ static inline int page_is_buddy(struct page *page, struct page *buddy, > if (page_zone_id(page) != page_zone_id(buddy)) > return 0; > > + if (IS_ENABLED(CONFIG_CMA) && > + has_isolate_pageblock(zone) && > + order >= pageblock_order && > + is_migrate_isolate(get_pageblock_migratetype(buddy))) > + return 0; > + > VM_BUG_ON_PAGE(page_count(buddy) != 0, buddy); > > return 1; > @@ -711,7 +717,7 @@ static inline void __free_one_page(struct page *page, > while (order < max_order - 1) { > buddy_idx = __find_buddy_index(page_idx, order); > buddy = page + (buddy_idx - page_idx); > - if (!page_is_buddy(page, buddy, order)) > + if (!page_is_buddy(zone, page, buddy, order)) > break; > /* > * Our buddy is free or it is CONFIG_DEBUG_PAGEALLOC guard page, > @@ -745,7 +751,7 @@ static inline void __free_one_page(struct page *page, > higher_page = page + (combined_idx - page_idx); > buddy_idx = __find_buddy_index(combined_idx, order + 1); > higher_buddy = higher_page + (buddy_idx - combined_idx); > - if (page_is_buddy(higher_page, higher_buddy, order + 1)) { > + if (page_is_buddy(zone, higher_page, higher_buddy, order + 1)) { > list_add_tail(&page->lru, > &zone->free_area[order].free_list[migratetype]); > goto out;
On Fri, Mar 04, 2016 at 02:05:09PM +0800, Hanjun Guo wrote: > On 2016/3/4 12:32, Joonsoo Kim wrote: > > On Fri, Mar 04, 2016 at 11:02:33AM +0900, Joonsoo Kim wrote: > >> On Thu, Mar 03, 2016 at 08:49:01PM +0800, Hanjun Guo wrote: > >>> On 2016/3/3 15:42, Joonsoo Kim wrote: > >>>> 2016-03-03 10:25 GMT+09:00 Laura Abbott <labbott@redhat.com>: > >>>>> (cc -mm and Joonsoo Kim) > >>>>> > >>>>> > >>>>> On 03/02/2016 05:52 AM, Hanjun Guo wrote: > >>>>>> Hi, > >>>>>> > >>>>>> I came across a suspicious error for CMA stress test: > >>>>>> > >>>>>> Before the test, I got: > >>>>>> -bash-4.3# cat /proc/meminfo | grep Cma > >>>>>> CmaTotal: 204800 kB > >>>>>> CmaFree: 195044 kB > >>>>>> > >>>>>> > >>>>>> After running the test: > >>>>>> -bash-4.3# cat /proc/meminfo | grep Cma > >>>>>> CmaTotal: 204800 kB > >>>>>> CmaFree: 6602584 kB > >>>>>> > >>>>>> So the freed CMA memory is more than total.. > >>>>>> > >>>>>> Also the the MemFree is more than mem total: > >>>>>> > >>>>>> -bash-4.3# cat /proc/meminfo > >>>>>> MemTotal: 16342016 kB > >>>>>> MemFree: 22367268 kB > >>>>>> MemAvailable: 22370528 kB > >>> [...] > >>>>> I played with this a bit and can see the same problem. The sanity > >>>>> check of CmaFree < CmaTotal generally triggers in > >>>>> __move_zone_freepage_state in unset_migratetype_isolate. > >>>>> This also seems to be present as far back as v4.0 which was the > >>>>> first version to have the updated accounting from Joonsoo. > >>>>> Were there known limitations with the new freepage accounting, > >>>>> Joonsoo? > >>>> I don't know. I also played with this and looks like there is > >>>> accounting problem, however, for my case, number of free page is slightly less > >>>> than total. I will take a look. > >>>> > >>>> Hanjun, could you tell me your malloc_size? I tested with 1 and it doesn't > >>>> look like your case. > >>> I tested with malloc_size with 2M, and it grows much bigger than 1M, also I > >>> did some other test: > >> Thanks! Now, I can re-generate erronous situation you mentioned. > >> > >>> - run with single thread with 100000 times, everything is fine. > >>> > >>> - I hack the cam_alloc() and free as below [1] to see if it's lock issue, with > >>> the same test with 100 multi-thread, then I got: > >> [1] would not be sufficient to close this race. > >> > >> Try following things [A]. And, for more accurate test, I changed code a bit more > >> to prevent kernel page allocation from cma area [B]. This will prevent kernel > >> page allocation from cma area completely so we can focus cma_alloc/release race. > >> > >> Although, this is not correct fix, it could help that we can guess > >> where the problem is. > > More correct fix is something like below. > > Please test it. > > Hmm, this is not working: Sad to hear that. Could you tell me your system's MAX_ORDER and pageblock_order? Thanks.
On 2016/3/4 14:38, Joonsoo Kim wrote: > On Fri, Mar 04, 2016 at 02:05:09PM +0800, Hanjun Guo wrote: >> On 2016/3/4 12:32, Joonsoo Kim wrote: >>> On Fri, Mar 04, 2016 at 11:02:33AM +0900, Joonsoo Kim wrote: >>>> On Thu, Mar 03, 2016 at 08:49:01PM +0800, Hanjun Guo wrote: >>>>> On 2016/3/3 15:42, Joonsoo Kim wrote: >>>>>> 2016-03-03 10:25 GMT+09:00 Laura Abbott <labbott@redhat.com>: >>>>>>> (cc -mm and Joonsoo Kim) >>>>>>> >>>>>>> >>>>>>> On 03/02/2016 05:52 AM, Hanjun Guo wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> I came across a suspicious error for CMA stress test: >>>>>>>> >>>>>>>> Before the test, I got: >>>>>>>> -bash-4.3# cat /proc/meminfo | grep Cma >>>>>>>> CmaTotal: 204800 kB >>>>>>>> CmaFree: 195044 kB >>>>>>>> >>>>>>>> >>>>>>>> After running the test: >>>>>>>> -bash-4.3# cat /proc/meminfo | grep Cma >>>>>>>> CmaTotal: 204800 kB >>>>>>>> CmaFree: 6602584 kB >>>>>>>> >>>>>>>> So the freed CMA memory is more than total.. >>>>>>>> >>>>>>>> Also the the MemFree is more than mem total: >>>>>>>> >>>>>>>> -bash-4.3# cat /proc/meminfo >>>>>>>> MemTotal: 16342016 kB >>>>>>>> MemFree: 22367268 kB >>>>>>>> MemAvailable: 22370528 kB >>>>> [...] >>>>>>> I played with this a bit and can see the same problem. The sanity >>>>>>> check of CmaFree < CmaTotal generally triggers in >>>>>>> __move_zone_freepage_state in unset_migratetype_isolate. >>>>>>> This also seems to be present as far back as v4.0 which was the >>>>>>> first version to have the updated accounting from Joonsoo. >>>>>>> Were there known limitations with the new freepage accounting, >>>>>>> Joonsoo? >>>>>> I don't know. I also played with this and looks like there is >>>>>> accounting problem, however, for my case, number of free page is slightly less >>>>>> than total. I will take a look. >>>>>> >>>>>> Hanjun, could you tell me your malloc_size? I tested with 1 and it doesn't >>>>>> look like your case. >>>>> I tested with malloc_size with 2M, and it grows much bigger than 1M, also I >>>>> did some other test: >>>> Thanks! Now, I can re-generate erronous situation you mentioned. >>>> >>>>> - run with single thread with 100000 times, everything is fine. >>>>> >>>>> - I hack the cam_alloc() and free as below [1] to see if it's lock issue, with >>>>> the same test with 100 multi-thread, then I got: >>>> [1] would not be sufficient to close this race. >>>> >>>> Try following things [A]. And, for more accurate test, I changed code a bit more >>>> to prevent kernel page allocation from cma area [B]. This will prevent kernel >>>> page allocation from cma area completely so we can focus cma_alloc/release race. >>>> >>>> Although, this is not correct fix, it could help that we can guess >>>> where the problem is. >>> More correct fix is something like below. >>> Please test it. >> Hmm, this is not working: > Sad to hear that. > > Could you tell me your system's MAX_ORDER and pageblock_order? > MAX_ORDER is 11, pageblock_order is 9, thanks for your help! Hanjun
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index c6c38ed..a01c3b5 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -620,8 +620,8 @@ static inline void rmv_page_order(struct page *page) * * For recording page's order, we use page_private(page). */ -static inline int page_is_buddy(struct page *page, struct page *buddy, - unsigned int order) +static inline int page_is_buddy(struct zone *zone, struct page *page, + struct page *buddy, unsigned int order) { if (!pfn_valid_within(page_to_pfn(buddy))) return 0; @@ -644,6 +644,12 @@ static inline int page_is_buddy(struct page *page, struct page *buddy, if (page_zone_id(page) != page_zone_id(buddy)) return 0; + if (IS_ENABLED(CONFIG_CMA) && + has_isolate_pageblock(zone) && + order >= pageblock_order && + is_migrate_isolate(get_pageblock_migratetype(buddy))) + return 0; + VM_BUG_ON_PAGE(page_count(buddy) != 0, buddy); return 1; @@ -711,7 +717,7 @@ static inline void __free_one_page(struct page *page, while (order < max_order - 1) { buddy_idx = __find_buddy_index(page_idx, order); buddy = page + (buddy_idx - page_idx); - if (!page_is_buddy(page, buddy, order)) + if (!page_is_buddy(zone, page, buddy, order)) break; /* * Our buddy is free or it is CONFIG_DEBUG_PAGEALLOC guard page, @@ -745,7 +751,7 @@ static inline void __free_one_page(struct page *page, higher_page = page + (combined_idx - page_idx); buddy_idx = __find_buddy_index(combined_idx, order + 1); higher_buddy = higher_page + (buddy_idx - combined_idx); - if (page_is_buddy(higher_page, higher_buddy, order + 1)) { + if (page_is_buddy(zone, higher_page, higher_buddy, order + 1)) { list_add_tail(&page->lru, &zone->free_area[order].free_list[migratetype]); goto out;