Re: [PATCH 3/4] zsmalloc use zs_handle instead of void *

[Minchan Kim]

Hi Dan,

On 05/11/2012 08:50 AM, Dan Magenheimer wrote:

>> From: Minchan Kim [mailto:minchan@xxxxxxxxxx]
>>
>> Okay. Now it works but zcache coupled with zsmalloc tightly.
>> User of zsmalloc should never know internal of zs_handle.
>>
>> 3)
>>
>> - zsmalloc.h
>> void *zs_handle_to_ptr(struct zs_handle handle)
>> {
>> 	return handle.hanle;
>> }
>>
>> static struct zv_hdr *zv_create(..)
>> {
>> 	struct zs_handle handle;
>> 	..
>> 	handle = zs_malloc(pool, size);
>> 	..
>> 	return zs_handle_to_ptr(handle);
>> }
>>
>> Why should zsmalloc support such interface?
>> It's a zcache problem so it's desriable to solve it in zcache internal.
>> And in future, if we can add/remove zs_handle's fields, we can't make
>> sure such API.
> 
> Hi Minchan --
> 
> I'm confused so maybe I am misunderstanding or you can
> explain further.  It seems like you are trying to redesign
> zsmalloc so that it can be a pure abstraction in a library.
> While I understand and value abstractions in software
> designs, the primary use now of zsmalloc is in zcache.  If

> there are other users that require a different interface
> or a more precise abstract API, zsmalloc could then
> evolve to meet the needs of multiple users.  But I think


At least, zram is also primary user and it also has such mess although it's not severe than zcache. zram->table[index].handle sometime has real (void*) handle, sometime (struct page*).
And I assume ramster you sent yesterday will be.

I think there are already many mess and I bet it will prevent going to mainline.
Especially, handle problem is severe because it a arguement of most functions exported in zsmalloc
So, we should clean up before late, IMHO.

> zcache is going to need more access to the internals
> of its allocator, not less.  Zsmalloc is currently missing
> some important functionality that (I believe) will be
> necessary to turn zcache into an enterprise-ready,


If you have such TODO list, could you post it?
It helps direction point of my stuff.

> always-on kernel feature.  If it evolves to add that
> functionality, then it may no longer be able to provide
> generic abstract access... in which case generic zsmalloc
> may then have zero users in the kernel.


Hmm, Do you want to make zsmalloc by zcache owned private allocator?

> 
> So I'd suggest we hold off on trying to make zsmalloc
> "pretty" until we better understand how it will be used
> by zcache (and ramster) and, if there are any, any future
> users.


zcache isn't urgent? I'm okay about zcache but at least, zram is when it merged into mainline, I think.
Many embedded system have a advantage with it so I hope we finish zsmalloc mess as soon as possble.

> 
> That's just my opinion...


Dan, Thanks for sharing your opinion.

> Dan
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxxx  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
> Don't email: <a href=ilto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>
> 



-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/