From: Joonsoo Kim <iamjoonsoo.kim@lge.com> To: Vlastimil Babka <vbabka@suse.cz> Cc: Andrew Morton <akpm@linux-foundation.org>, Rik van Riel <riel@redhat.com>, Johannes Weiner <hannes@cmpxchg.org>, mgorman@techsingularity.net, Laura Abbott <lauraa@codeaurora.org>, Minchan Kim <minchan@kernel.org>, Marek Szyprowski <m.szyprowski@samsung.com>, Michal Nazarewicz <mina86@mina86.com>, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v6 3/6] mm/cma: populate ZONE_CMA Date: Tue, 18 Oct 2016 17:27:30 +0900 [thread overview] Message-ID: <20161018082730.GA20442@js1304-P5Q-DELUXE> (raw) In-Reply-To: <33f0a8f3-38d1-e527-f71f-839afe0b2ed9@suse.cz> On Tue, Oct 18, 2016 at 09:42:57AM +0200, Vlastimil Babka wrote: > On 10/14/2016 05:03 AM, js1304@gmail.com wrote: > >@@ -145,6 +145,35 @@ static int __init cma_activate_area(struct cma *cma) > > static int __init cma_init_reserved_areas(void) > > { > > int i; > >+ struct zone *zone; > >+ pg_data_t *pgdat; > >+ > >+ if (!cma_area_count) > >+ return 0; > >+ > >+ for_each_online_pgdat(pgdat) { > >+ unsigned long start_pfn = UINT_MAX, end_pfn = 0; > >+ > >+ for (i = 0; i < cma_area_count; i++) { > >+ if (pfn_to_nid(cma_areas[i].base_pfn) != > >+ pgdat->node_id) > >+ continue; > >+ > >+ start_pfn = min(start_pfn, cma_areas[i].base_pfn); > >+ end_pfn = max(end_pfn, cma_areas[i].base_pfn + > >+ cma_areas[i].count); > >+ } > >+ > >+ if (!end_pfn) > >+ continue; > >+ > >+ zone = &pgdat->node_zones[ZONE_CMA]; > >+ > >+ /* ZONE_CMA doesn't need to exceed CMA region */ > >+ zone->zone_start_pfn = max(zone->zone_start_pfn, start_pfn); > >+ zone->spanned_pages = min(zone_end_pfn(zone), end_pfn) - > >+ zone->zone_start_pfn; > > Hmm, do the max/min here work as intended? IIUC the initial Yeap. > zone_start_pfn is UINT_MAX and zone->spanned_pages is 1? So at least > the max/min should be swapped? No. CMA zone's start/end pfn are updated as node's start/end pfn. > Also the zone_end_pfn(zone) on the second line already sees the > changes to zone->zone_start_pfn in the first line, so it's kind of a > mess. You should probably cache zone_end_pfn() to a temporary > variable before changing zone_start_pfn. You're right although it doesn't cause any problem. I look at the code again and find that max/min isn't needed. Calculated start/end pfn should be inbetween node's start/end pfn so max(zone->zone_start_pfn, start_pfn) will return start_pfn and messed up min(zone_end_pfn(zone), end_pfn) will return end_pfn in all the cases. Anyway, I will fix it as following. zone->zone_start_pfn = start_pfn zone->spanned_pages = end_pfn - start_pfn Thanks.
WARNING: multiple messages have this Message-ID (diff)
From: Joonsoo Kim <iamjoonsoo.kim@lge.com> To: Vlastimil Babka <vbabka@suse.cz> Cc: Andrew Morton <akpm@linux-foundation.org>, Rik van Riel <riel@redhat.com>, Johannes Weiner <hannes@cmpxchg.org>, mgorman@techsingularity.net, Laura Abbott <lauraa@codeaurora.org>, Minchan Kim <minchan@kernel.org>, Marek Szyprowski <m.szyprowski@samsung.com>, Michal Nazarewicz <mina86@mina86.com>, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v6 3/6] mm/cma: populate ZONE_CMA Date: Tue, 18 Oct 2016 17:27:30 +0900 [thread overview] Message-ID: <20161018082730.GA20442@js1304-P5Q-DELUXE> (raw) In-Reply-To: <33f0a8f3-38d1-e527-f71f-839afe0b2ed9@suse.cz> On Tue, Oct 18, 2016 at 09:42:57AM +0200, Vlastimil Babka wrote: > On 10/14/2016 05:03 AM, js1304@gmail.com wrote: > >@@ -145,6 +145,35 @@ static int __init cma_activate_area(struct cma *cma) > > static int __init cma_init_reserved_areas(void) > > { > > int i; > >+ struct zone *zone; > >+ pg_data_t *pgdat; > >+ > >+ if (!cma_area_count) > >+ return 0; > >+ > >+ for_each_online_pgdat(pgdat) { > >+ unsigned long start_pfn = UINT_MAX, end_pfn = 0; > >+ > >+ for (i = 0; i < cma_area_count; i++) { > >+ if (pfn_to_nid(cma_areas[i].base_pfn) != > >+ pgdat->node_id) > >+ continue; > >+ > >+ start_pfn = min(start_pfn, cma_areas[i].base_pfn); > >+ end_pfn = max(end_pfn, cma_areas[i].base_pfn + > >+ cma_areas[i].count); > >+ } > >+ > >+ if (!end_pfn) > >+ continue; > >+ > >+ zone = &pgdat->node_zones[ZONE_CMA]; > >+ > >+ /* ZONE_CMA doesn't need to exceed CMA region */ > >+ zone->zone_start_pfn = max(zone->zone_start_pfn, start_pfn); > >+ zone->spanned_pages = min(zone_end_pfn(zone), end_pfn) - > >+ zone->zone_start_pfn; > > Hmm, do the max/min here work as intended? IIUC the initial Yeap. > zone_start_pfn is UINT_MAX and zone->spanned_pages is 1? So at least > the max/min should be swapped? No. CMA zone's start/end pfn are updated as node's start/end pfn. > Also the zone_end_pfn(zone) on the second line already sees the > changes to zone->zone_start_pfn in the first line, so it's kind of a > mess. You should probably cache zone_end_pfn() to a temporary > variable before changing zone_start_pfn. You're right although it doesn't cause any problem. I look at the code again and find that max/min isn't needed. Calculated start/end pfn should be inbetween node's start/end pfn so max(zone->zone_start_pfn, start_pfn) will return start_pfn and messed up min(zone_end_pfn(zone), end_pfn) will return end_pfn in all the cases. Anyway, I will fix it as following. zone->zone_start_pfn = start_pfn zone->spanned_pages = end_pfn - start_pfn Thanks. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2016-10-18 8:27 UTC|newest] Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-10-14 3:03 [PATCH v6 0/6] Introduce ZONE_CMA js1304 2016-10-14 3:03 ` js1304 2016-10-14 3:03 ` [PATCH v6 1/6] mm/page_alloc: don't reserve ZONE_HIGHMEM for ZONE_MOVABLE request js1304 2016-10-14 3:03 ` js1304 2016-10-14 3:03 ` [PATCH v6 2/6] mm/cma: introduce new zone, ZONE_CMA js1304 2016-10-14 3:03 ` js1304 2016-11-01 7:58 ` Chen Feng 2016-11-01 7:58 ` Chen Feng 2016-11-07 6:15 ` Joonsoo Kim 2016-11-07 6:15 ` Joonsoo Kim 2016-11-07 7:08 ` Chen Feng 2016-11-07 7:08 ` Chen Feng 2016-11-07 7:27 ` Joonsoo Kim 2016-11-07 7:27 ` Joonsoo Kim 2016-11-07 7:44 ` Chen Feng 2016-11-07 7:44 ` Chen Feng 2016-11-07 7:46 ` Chen Feng 2016-11-07 7:46 ` Chen Feng 2016-11-08 3:59 ` Joonsoo Kim 2016-11-08 3:59 ` Joonsoo Kim 2016-11-08 6:59 ` Chen Feng 2016-11-08 6:59 ` Chen Feng 2016-11-11 6:38 ` Joonsoo Kim 2016-11-11 6:38 ` Joonsoo Kim 2016-10-14 3:03 ` [PATCH v6 3/6] mm/cma: populate ZONE_CMA js1304 2016-10-14 3:03 ` js1304 2016-10-18 7:42 ` Vlastimil Babka 2016-10-18 7:42 ` Vlastimil Babka 2016-10-18 8:27 ` Joonsoo Kim [this message] 2016-10-18 8:27 ` Joonsoo Kim 2016-10-26 4:31 ` Joonsoo Kim 2016-10-26 4:31 ` Joonsoo Kim 2016-10-26 7:27 ` Vlastimil Babka 2016-10-26 7:27 ` Vlastimil Babka 2016-10-14 3:03 ` [PATCH v6 4/6] mm/cma: remove ALLOC_CMA js1304 2016-10-14 3:03 ` js1304 2016-10-14 3:03 ` [PATCH v6 5/6] mm/cma: remove MIGRATE_CMA js1304 2016-10-14 3:03 ` js1304 2016-10-14 3:03 ` [PATCH v6 6/6] mm/cma: remove per zone CMA stat js1304 2016-10-14 3:03 ` js1304 2016-11-07 6:25 ` [PATCH v6 0/6] Introduce ZONE_CMA Joonsoo Kim 2016-11-07 6:25 ` Joonsoo Kim 2016-11-28 7:41 ` Joonsoo Kim 2016-11-28 7:41 ` Joonsoo Kim
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20161018082730.GA20442@js1304-P5Q-DELUXE \ --to=iamjoonsoo.kim@lge.com \ --cc=akpm@linux-foundation.org \ --cc=aneesh.kumar@linux.vnet.ibm.com \ --cc=hannes@cmpxchg.org \ --cc=lauraa@codeaurora.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=m.szyprowski@samsung.com \ --cc=mgorman@techsingularity.net \ --cc=mina86@mina86.com \ --cc=minchan@kernel.org \ --cc=riel@redhat.com \ --cc=vbabka@suse.cz \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.