linux-kernel - Re: [PATCH] zsmalloc: drop unused member 'mapping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56C3E91B.1030101@hisilicon.com>
Date:	Wed, 17 Feb 2016 11:29:31 +0800
From:	xuyiping <xuyiping@...ilicon.com>
To:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
	YiPing Xu <xuyiping@...wei.com>
CC:	<minchan@...nel.org>, <ngupta@...are.org>, <linux-mm@...ck.org>,
	<linux-kernel@...r.kernel.org>, <suzhuangluan@...ilicon.com>,
	<puck.chen@...ilicon.com>, <dan.zhao@...ilicon.com>
Subject: Re: [PATCH] zsmalloc: drop unused member 'mapping_area->huge'

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