Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1335997 > unrolled thread

[PATCH] zsmalloc: drop unused member 'mapping_area->huge'

Back to article view | Back to linux.kernel

Contents

  [PATCH] zsmalloc: drop unused member 'mapping_area->huge' YiPing Xu <xuyiping@huawei.com> - 2016-02-17 03:00 +0100
    Re: [PATCH] zsmalloc: drop unused member 'mapping_area->huge' Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-02-17 03:30 +0100
      Re: [PATCH] zsmalloc: drop unused member 'mapping_area->huge' xuyiping <xuyiping@hisilicon.com> - 2016-02-17 04:40 +0100
        Re: [PATCH] zsmalloc: drop unused member 'mapping_area->huge' Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-02-17 05:00 +0100
    Re: [PATCH] zsmalloc: drop unused member 'mapping_area->huge' Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-02-17 06:20 +0100
    Re: [PATCH] zsmalloc: drop unused member 'mapping_area->huge' Minchan Kim <minchan@kernel.org> - 2016-02-17 16:40 +0100

#1335997 — [PATCH] zsmalloc: drop unused member 'mapping_area->huge'

When unmapping a huge class page in zs_unmap_object, the page will
be unmapped by kmap_atomic. the "!area->huge" branch in
__zs_unmap_object is alway true, and no code set "area->huge" now,
so we can drop it.

Signed-off-by: YiPing Xu <xuyiping@huawei.com>
---
 mm/zsmalloc.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 2d7c4c1..43e4cbc 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -281,7 +281,6 @@ struct mapping_area {
 #endif
 	char *vm_addr; /* address of kmap_atomic()'ed pages */
 	enum zs_mapmode vm_mm; /* mapping mode */
-	bool huge;
 };
 
 static int create_handle_cache(struct zs_pool *pool)
@@ -1127,11 +1126,9 @@ static void __zs_unmap_object(struct mapping_area *area,
 		goto out;
 
 	buf = area->vm_buf;
-	if (!area->huge) {
-		buf = buf + ZS_HANDLE_SIZE;
-		size -= ZS_HANDLE_SIZE;
-		off += ZS_HANDLE_SIZE;
-	}
+	buf = buf + ZS_HANDLE_SIZE;
+	size -= ZS_HANDLE_SIZE;
+	off += ZS_HANDLE_SIZE;
 
 	sizes[0] = PAGE_SIZE - off;
 	sizes[1] = size - sizes[0];
-- 
1.8.3.2

[toc] | [next] | [standalone]

#1336006

Hello,

On (02/17/16 09:56), YiPing Xu wrote:
>  static int create_handle_cache(struct zs_pool *pool)
> @@ -1127,11 +1126,9 @@ static void __zs_unmap_object(struct mapping_area *area,
>  		goto out;
>  
>  	buf = area->vm_buf;
> -	if (!area->huge) {
> -		buf = buf + ZS_HANDLE_SIZE;
> -		size -= ZS_HANDLE_SIZE;
> -		off += ZS_HANDLE_SIZE;
> -	}
> +	buf = buf + ZS_HANDLE_SIZE;
> +	size -= ZS_HANDLE_SIZE;
> +	off += ZS_HANDLE_SIZE;
>  
>  	sizes[0] = PAGE_SIZE - off;
>  	sizes[1] = size - sizes[0];


hm, indeed.

shouldn't it depend on class->huge?

void *zs_map_object()
{
	void *ret = __zs_map_object(area, pages, off, class->size);

	if (!class->huge)
		ret += ZS_HANDLE_SIZE;  /* area->vm_buf + ZS_HANDLE_SIZE */

	return ret;
}

static void __zs_unmap_object(struct mapping_area *area...)
{
	char *buf = area->vm_buf;

	/* handle is in page->private for class->huge */

	buf = buf + ZS_HANDLE_SIZE;
	size -= ZS_HANDLE_SIZE;
	off += ZS_HANDLE_SIZE;

	memcpy(..);
}

	-ss

[toc] | [prev] | [next] | [standalone]

#1336043

HI, Sergery

On 2016/2/17 10:26, Sergey Senozhatsky wrote:
> Hello,
>
> On (02/17/16 09:56), YiPing Xu wrote:
>>   static int create_handle_cache(struct zs_pool *pool)
>> @@ -1127,11 +1126,9 @@ static void __zs_unmap_object(struct mapping_area *area,
>>   		goto out;
>>
>>   	buf = area->vm_buf;
>> -	if (!area->huge) {
>> -		buf = buf + ZS_HANDLE_SIZE;
>> -		size -= ZS_HANDLE_SIZE;
>> -		off += ZS_HANDLE_SIZE;
>> -	}
>> +	buf = buf + ZS_HANDLE_SIZE;
>> +	size -= ZS_HANDLE_SIZE;
>> +	off += ZS_HANDLE_SIZE;
>>
>>   	sizes[0] = PAGE_SIZE - off;
>>   	sizes[1] = size - sizes[0];
>
>
> hm, indeed.
>
> shouldn't it depend on class->huge?
>
> void *zs_map_object()
> {

	if (off + class->size <= PAGE_SIZE) {

for huge object, the code will get into this branch, there is no more 
huge object process in __zs_map_object.

		/* this object is contained entirely within a page */
		area->vm_addr = kmap_atomic(page);
		ret = area->vm_addr + off;
		goto out;
	}


> 	void *ret = __zs_map_object(area, pages, off, class->size);
>
> 	if (!class->huge)
> 		ret += ZS_HANDLE_SIZE;  /* area->vm_buf + ZS_HANDLE_SIZE */
>
> 	return ret;
> }

void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
{
	..

	area = this_cpu_ptr(&zs_map_area);
	if (off + class->size <= PAGE_SIZE)

for huge object, the code will get into this branch, so, in 
__zs_unmap_object there is no depend on class->huge.

it is a little implicated here.

		kunmap_atomic(area->vm_addr);
	else {
		struct page *pages[2];

		pages[0] = page;
		pages[1] = get_next_page(page);
		BUG_ON(!pages[1]);

		__zs_unmap_object(area, pages, off, class->size);
	}

	..
}


> static void __zs_unmap_object(struct mapping_area *area...)
> {
> 	char *buf = area->vm_buf;
>
> 	/* handle is in page->private for class->huge */
>
> 	buf = buf + ZS_HANDLE_SIZE;
> 	size -= ZS_HANDLE_SIZE;
> 	off += ZS_HANDLE_SIZE;
>
> 	memcpy(..);
> }
>
> 	-ss
>
> .
>

[toc] | [prev] | [next] | [standalone]

#1336050

On (02/17/16 11:29), xuyiping wrote:
[..]
> 
> 	if (off + class->size <= PAGE_SIZE) {
> 
> for huge object, the code will get into this branch, there is no more huge
> object process in __zs_map_object.

correct, well, techically, it's not about huge objects, but objects that span
page boundaries. we can have objects of pretty small sizes being split between
pages, for example size:1536 and offset:3072, etc.

	-ss

[toc] | [prev] | [next] | [standalone]

#1336057

On (02/17/16 09:56), YiPing Xu wrote:
> When unmapping a huge class page in zs_unmap_object, the page will
> be unmapped by kmap_atomic. the "!area->huge" branch in
> __zs_unmap_object is alway true, and no code set "area->huge" now,
> so we can drop it.
> 

the patch looks good to me, thanks.
Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

	-ss

> Signed-off-by: YiPing Xu <xuyiping@huawei.com>
> ---
>  mm/zsmalloc.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 2d7c4c1..43e4cbc 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -281,7 +281,6 @@ struct mapping_area {
>  #endif
>  	char *vm_addr; /* address of kmap_atomic()'ed pages */
>  	enum zs_mapmode vm_mm; /* mapping mode */
> -	bool huge;
>  };
>  
>  static int create_handle_cache(struct zs_pool *pool)
> @@ -1127,11 +1126,9 @@ static void __zs_unmap_object(struct mapping_area *area,
>  		goto out;
>  
>  	buf = area->vm_buf;
> -	if (!area->huge) {
> -		buf = buf + ZS_HANDLE_SIZE;
> -		size -= ZS_HANDLE_SIZE;
> -		off += ZS_HANDLE_SIZE;
> -	}
> +	buf = buf + ZS_HANDLE_SIZE;
> +	size -= ZS_HANDLE_SIZE;
> +	off += ZS_HANDLE_SIZE;
>  
>  	sizes[0] = PAGE_SIZE - off;
>  	sizes[1] = size - sizes[0];
> -- 
> 1.8.3.2
> 

[toc] | [prev] | [next] | [standalone]

#1336524

On Wed, Feb 17, 2016 at 09:56:39AM +0800, YiPing Xu wrote:
> When unmapping a huge class page in zs_unmap_object, the page will
> be unmapped by kmap_atomic. the "!area->huge" branch in
> __zs_unmap_object is alway true, and no code set "area->huge" now,
> so we can drop it.
> 
> Signed-off-by: YiPing Xu <xuyiping@huawei.com>
Acked-by: Minchan Kim <minchan@kernel.org>

Thanks.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web