linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm/mlock: use page_zone() instead of page_zone_id()
@ 2017-08-24  7:20 js1304
  2017-08-24 11:05 ` Vlastimil Babka
  0 siblings, 1 reply; 4+ messages in thread
From: js1304 @ 2017-08-24  7:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Vlastimil Babka, linux-mm, linux-kernel, Joonsoo Kim

From: Joonsoo Kim <iamjoonsoo.kim@lge.com>

page_zone_id() is a specialized function to compare the zone for the pages
that are within the section range. If the section of the pages are
different, page_zone_id() can be different even if their zone is the same.
This wrong usage doesn't cause any actual problem since
__munlock_pagevec_fill() would be called again with failed index. However,
it's better to use more appropriate function here.

This patch is also preparation for futher change about page_zone_id().

Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
 mm/mlock.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/mm/mlock.c b/mm/mlock.c
index b562b55..dfc6f19 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -365,8 +365,8 @@ static void __munlock_pagevec(struct pagevec *pvec, struct zone *zone)
  * @start + PAGE_SIZE when no page could be added by the pte walk.
  */
 static unsigned long __munlock_pagevec_fill(struct pagevec *pvec,
-		struct vm_area_struct *vma, int zoneid,	unsigned long start,
-		unsigned long end)
+			struct vm_area_struct *vma, struct zone *zone,
+			unsigned long start, unsigned long end)
 {
 	pte_t *pte;
 	spinlock_t *ptl;
@@ -394,7 +394,7 @@ static unsigned long __munlock_pagevec_fill(struct pagevec *pvec,
 		 * Break if page could not be obtained or the page's node+zone does not
 		 * match
 		 */
-		if (!page || page_zone_id(page) != zoneid)
+		if (!page || page_zone(page) != zone)
 			break;
 
 		/*
@@ -446,7 +446,6 @@ void munlock_vma_pages_range(struct vm_area_struct *vma,
 		unsigned long page_increm;
 		struct pagevec pvec;
 		struct zone *zone;
-		int zoneid;
 
 		pagevec_init(&pvec, 0);
 		/*
@@ -481,7 +480,6 @@ void munlock_vma_pages_range(struct vm_area_struct *vma,
 				 */
 				pagevec_add(&pvec, page);
 				zone = page_zone(page);
-				zoneid = page_zone_id(page);
 
 				/*
 				 * Try to fill the rest of pagevec using fast
@@ -490,7 +488,7 @@ void munlock_vma_pages_range(struct vm_area_struct *vma,
 				 * pagevec.
 				 */
 				start = __munlock_pagevec_fill(&pvec, vma,
-						zoneid, start, end);
+						zone, start, end);
 				__munlock_pagevec(&pvec, zone);
 				goto next;
 			}
-- 
2.7.4

--
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>

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/mlock: use page_zone() instead of page_zone_id()
  2017-08-24  7:20 [PATCH] mm/mlock: use page_zone() instead of page_zone_id() js1304
@ 2017-08-24 11:05 ` Vlastimil Babka
  2017-08-24 23:59   ` Joonsoo Kim
  0 siblings, 1 reply; 4+ messages in thread
From: Vlastimil Babka @ 2017-08-24 11:05 UTC (permalink / raw)
  To: js1304, Andrew Morton
  Cc: Minchan Kim, linux-mm, linux-kernel, Joonsoo Kim, Mel Gorman

+CC Mel

On 08/24/2017 09:20 AM, js1304@gmail.com wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> 
> page_zone_id() is a specialized function to compare the zone for the pages
> that are within the section range. If the section of the pages are
> different, page_zone_id() can be different even if their zone is the same.
> This wrong usage doesn't cause any actual problem since
> __munlock_pagevec_fill() would be called again with failed index. However,
> it's better to use more appropriate function here.

Hmm using zone id was part of the series making munlock faster. Too bad
it's doing the wrong thing on some memory models. Looks like it wasn't
evaluated in isolation, but only as part of the pagevec usage (commit
7a8010cd36273) but most likely it wasn't contributing too much to the
14% speedup.

> This patch is also preparation for futher change about page_zone_id().

Out of curiosity, what kind of change?

Vlastimil

> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> ---
>  mm/mlock.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/mlock.c b/mm/mlock.c
> index b562b55..dfc6f19 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -365,8 +365,8 @@ static void __munlock_pagevec(struct pagevec *pvec, struct zone *zone)
>   * @start + PAGE_SIZE when no page could be added by the pte walk.
>   */
>  static unsigned long __munlock_pagevec_fill(struct pagevec *pvec,
> -		struct vm_area_struct *vma, int zoneid,	unsigned long start,
> -		unsigned long end)
> +			struct vm_area_struct *vma, struct zone *zone,
> +			unsigned long start, unsigned long end)
>  {
>  	pte_t *pte;
>  	spinlock_t *ptl;
> @@ -394,7 +394,7 @@ static unsigned long __munlock_pagevec_fill(struct pagevec *pvec,
>  		 * Break if page could not be obtained or the page's node+zone does not
>  		 * match
>  		 */
> -		if (!page || page_zone_id(page) != zoneid)
> +		if (!page || page_zone(page) != zone)
>  			break;
>  
>  		/*
> @@ -446,7 +446,6 @@ void munlock_vma_pages_range(struct vm_area_struct *vma,
>  		unsigned long page_increm;
>  		struct pagevec pvec;
>  		struct zone *zone;
> -		int zoneid;
>  
>  		pagevec_init(&pvec, 0);
>  		/*
> @@ -481,7 +480,6 @@ void munlock_vma_pages_range(struct vm_area_struct *vma,
>  				 */
>  				pagevec_add(&pvec, page);
>  				zone = page_zone(page);
> -				zoneid = page_zone_id(page);
>  
>  				/*
>  				 * Try to fill the rest of pagevec using fast
> @@ -490,7 +488,7 @@ void munlock_vma_pages_range(struct vm_area_struct *vma,
>  				 * pagevec.
>  				 */
>  				start = __munlock_pagevec_fill(&pvec, vma,
> -						zoneid, start, end);
> +						zone, start, end);
>  				__munlock_pagevec(&pvec, zone);
>  				goto next;
>  			}
> 

--
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>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/mlock: use page_zone() instead of page_zone_id()
  2017-08-24 11:05 ` Vlastimil Babka
@ 2017-08-24 23:59   ` Joonsoo Kim
  2017-08-25  8:48     ` Vlastimil Babka
  0 siblings, 1 reply; 4+ messages in thread
From: Joonsoo Kim @ 2017-08-24 23:59 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Andrew Morton, Minchan Kim, linux-mm, linux-kernel, Mel Gorman

On Thu, Aug 24, 2017 at 01:05:15PM +0200, Vlastimil Babka wrote:
> +CC Mel
> 
> On 08/24/2017 09:20 AM, js1304@gmail.com wrote:
> > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> > 
> > page_zone_id() is a specialized function to compare the zone for the pages
> > that are within the section range. If the section of the pages are
> > different, page_zone_id() can be different even if their zone is the same.
> > This wrong usage doesn't cause any actual problem since
> > __munlock_pagevec_fill() would be called again with failed index. However,
> > it's better to use more appropriate function here.
> 
> Hmm using zone id was part of the series making munlock faster. Too bad
> it's doing the wrong thing on some memory models. Looks like it wasn't
> evaluated in isolation, but only as part of the pagevec usage (commit
> 7a8010cd36273) but most likely it wasn't contributing too much to the
> 14% speedup.

I roughly checked that patch and it seems that performance improvement
of that commit isn't related to page_zone_id() usage. With
page_zone(), we would have more chance that do a job as a batch.

> 
> > This patch is also preparation for futher change about page_zone_id().
> 
> Out of curiosity, what kind of change?
>

I prepared one more patch that prevent another user of page_zone_id()
since it is too tricky. However, I don't submit it. That description
should be removed. :/

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>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/mlock: use page_zone() instead of page_zone_id()
  2017-08-24 23:59   ` Joonsoo Kim
@ 2017-08-25  8:48     ` Vlastimil Babka
  0 siblings, 0 replies; 4+ messages in thread
From: Vlastimil Babka @ 2017-08-25  8:48 UTC (permalink / raw)
  To: Joonsoo Kim
  Cc: Andrew Morton, Minchan Kim, linux-mm, linux-kernel, Mel Gorman

On 08/25/2017 01:59 AM, Joonsoo Kim wrote:
> On Thu, Aug 24, 2017 at 01:05:15PM +0200, Vlastimil Babka wrote:
>> +CC Mel
>>
>> On 08/24/2017 09:20 AM, js1304@gmail.com wrote:
>>> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>>>
>>> page_zone_id() is a specialized function to compare the zone for the pages
>>> that are within the section range. If the section of the pages are
>>> different, page_zone_id() can be different even if their zone is the same.
>>> This wrong usage doesn't cause any actual problem since
>>> __munlock_pagevec_fill() would be called again with failed index. However,
>>> it's better to use more appropriate function here.
>>
>> Hmm using zone id was part of the series making munlock faster. Too bad
>> it's doing the wrong thing on some memory models. Looks like it wasn't
>> evaluated in isolation, but only as part of the pagevec usage (commit
>> 7a8010cd36273) but most likely it wasn't contributing too much to the
>> 14% speedup.
> 
> I roughly checked that patch and it seems that performance improvement
> of that commit isn't related to page_zone_id() usage. With
> page_zone(), we would have more chance that do a job as a batch.
> 
>>
>>> This patch is also preparation for futher change about page_zone_id().
>>
>> Out of curiosity, what kind of change?
>>
> 
> I prepared one more patch that prevent another user of page_zone_id()
> since it is too tricky. However, I don't submit it. That description
> should be removed. :/

OK. You can add

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> 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>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-08-25  8:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-24  7:20 [PATCH] mm/mlock: use page_zone() instead of page_zone_id() js1304
2017-08-24 11:05 ` Vlastimil Babka
2017-08-24 23:59   ` Joonsoo Kim
2017-08-25  8:48     ` Vlastimil Babka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).