All of lore.kernel.org
 help / color / mirror / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Dan Magenheimer <dan.magenheimer@oracle.com>
Cc: Konrad Wilk <konrad.wilk@oracle.com>,
	Nitin Gupta <ngupta@vflare.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Seth Jennings <sjenning@linux.vnet.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH 3/4] zsmalloc use zs_handle instead of void *
Date: Fri, 11 May 2012 09:14:46 +0900	[thread overview]
Message-ID: <4FAC59F6.4080503@kernel.org> (raw)
In-Reply-To: <8473859b-42f3-4354-b5ba-fd5b8cbac22f@default>

Hi Dan,

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

>> From: Minchan Kim [mailto:minchan@kernel.org]
>>
>> 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@kvack.org.  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@kvack.org"> email@kvack.org </a>
> 



-- 
Kind regards,
Minchan Kim

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Minchan Kim <minchan@kernel.org>
To: Dan Magenheimer <dan.magenheimer@oracle.com>
Cc: Konrad Wilk <konrad.wilk@oracle.com>,
	Nitin Gupta <ngupta@vflare.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Seth Jennings <sjenning@linux.vnet.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH 3/4] zsmalloc use zs_handle instead of void *
Date: Fri, 11 May 2012 09:14:46 +0900	[thread overview]
Message-ID: <4FAC59F6.4080503@kernel.org> (raw)
In-Reply-To: <8473859b-42f3-4354-b5ba-fd5b8cbac22f@default>

Hi Dan,

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

>> From: Minchan Kim [mailto:minchan@kernel.org]
>>
>> 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@kvack.org.  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@kvack.org"> email@kvack.org </a>
> 



-- 
Kind regards,
Minchan Kim

  reply	other threads:[~2012-05-11  0:14 UTC|newest]

Thread overview: 104+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-03  6:40 [PATCH 1/4] zsmalloc: rename zspage_order with zspage_pages Minchan Kim
2012-05-03  6:40 ` Minchan Kim
2012-05-03  6:40 ` [PATCH 2/4] zsmalloc: add/fix function comment Minchan Kim
2012-05-03  6:40   ` Minchan Kim
2012-05-03 13:19   ` Nitin Gupta
2012-05-03 13:19     ` Nitin Gupta
2012-05-03  6:40 ` [PATCH 3/4] zsmalloc use zs_handle instead of void * Minchan Kim
2012-05-03  6:40   ` Minchan Kim
2012-05-03 13:32   ` Nitin Gupta
2012-05-03 13:32     ` Nitin Gupta
2012-05-03 15:23     ` Seth Jennings
2012-05-03 15:23       ` Seth Jennings
2012-05-04  2:24       ` Minchan Kim
2012-05-04  2:24         ` Minchan Kim
2012-05-07 15:01         ` Seth Jennings
2012-05-07 15:01           ` Seth Jennings
2012-05-09 20:19         ` Greg Kroah-Hartman
2012-05-09 20:19           ` Greg Kroah-Hartman
2012-05-10  2:03           ` Minchan Kim
2012-05-10  2:03             ` Minchan Kim
2012-05-10 14:02             ` Konrad Rzeszutek Wilk
2012-05-10 14:02               ` Konrad Rzeszutek Wilk
2012-05-10 14:12               ` Greg Kroah-Hartman
2012-05-10 14:12                 ` Greg Kroah-Hartman
2012-05-10 14:47               ` Nitin Gupta
2012-05-10 14:47                 ` Nitin Gupta
2012-05-10 15:00                 ` Minchan Kim
2012-05-10 15:00                   ` Minchan Kim
2012-05-10 15:11                 ` Seth Jennings
2012-05-10 15:11                   ` Seth Jennings
2012-05-10 15:19                   ` Minchan Kim
2012-05-10 15:19                     ` Minchan Kim
2012-05-10 15:19                   ` Greg Kroah-Hartman
2012-05-10 15:19                     ` Greg Kroah-Hartman
2012-05-10 16:29                     ` Nitin Gupta
2012-05-10 16:29                       ` Nitin Gupta
2012-05-10 16:44                       ` Greg Kroah-Hartman
2012-05-10 16:44                         ` Greg Kroah-Hartman
2012-05-10 17:24                         ` Nitin Gupta
2012-05-10 17:24                           ` Nitin Gupta
2012-05-10 17:33                           ` Konrad Rzeszutek Wilk
2012-05-10 17:33                             ` Konrad Rzeszutek Wilk
2012-05-10 23:24                             ` Minchan Kim
2012-05-10 23:24                               ` Minchan Kim
2012-05-10 23:50                               ` Dan Magenheimer
2012-05-10 23:50                                 ` Dan Magenheimer
2012-05-11  0:14                                 ` Minchan Kim [this message]
2012-05-11  0:14                                   ` Minchan Kim
2012-05-11 16:31                                   ` Dan Magenheimer
2012-05-11 16:31                                     ` Dan Magenheimer
2012-05-11 19:29                                   ` Konrad Rzeszutek Wilk
2012-05-11 19:29                                     ` Konrad Rzeszutek Wilk
2012-05-11 21:49                                     ` Seth Jennings
2012-05-11 21:49                                       ` Seth Jennings
2012-05-14  2:26                                       ` Minchan Kim
2012-05-14  2:26                                         ` Minchan Kim
2012-05-11 19:28                               ` Konrad Rzeszutek Wilk
2012-05-11 19:28                                 ` Konrad Rzeszutek Wilk
2012-05-14  2:18                                 ` Minchan Kim
2012-05-14  2:18                                   ` Minchan Kim
2012-05-15  1:57                                   ` Nitin Gupta
2012-05-15  1:57                                     ` Nitin Gupta
2012-05-15  2:21                                     ` Minchan Kim
2012-05-15  2:21                                       ` Minchan Kim
2012-05-15 15:04                                   ` Konrad Rzeszutek Wilk
2012-05-15 15:04                                     ` Konrad Rzeszutek Wilk
2012-05-16  1:36                                     ` Minchan Kim
2012-05-16  1:36                                       ` Minchan Kim
2012-05-16 11:06                                       ` Konrad Rzeszutek Wilk
2012-05-16 11:06                                         ` Konrad Rzeszutek Wilk
2012-05-03  6:40 ` [PATCH 4/4] zsmalloc: zsmalloc: align cache line size Minchan Kim
2012-05-03  6:40   ` Minchan Kim
2012-05-03 13:58   ` Nitin Gupta
2012-05-03 13:58     ` Nitin Gupta
2012-05-04  2:27     ` Minchan Kim
2012-05-04  2:27       ` Minchan Kim
2012-05-07  7:41       ` Pekka Enberg
2012-05-07  7:41         ` Pekka Enberg
2012-05-07 12:40         ` Nitin Gupta
2012-05-07 12:40           ` Nitin Gupta
2012-05-08  1:34           ` Minchan Kim
2012-05-08  1:34             ` Minchan Kim
2012-05-08 14:00             ` Dan Magenheimer
2012-05-08 14:00               ` Dan Magenheimer
2012-05-09  0:58               ` Minchan Kim
2012-05-09  0:58                 ` Minchan Kim
2012-05-09  3:08                 ` Dan Magenheimer
2012-05-09  3:08                   ` Dan Magenheimer
2012-05-09  4:07                   ` Minchan Kim
2012-05-09  4:07                     ` Minchan Kim
2012-05-11  0:03                 ` Dan Magenheimer
2012-05-11  0:03                   ` Dan Magenheimer
2012-05-11  0:25                   ` Minchan Kim
2012-05-11  0:25                     ` Minchan Kim
2012-05-11 19:06                     ` Konrad Rzeszutek Wilk
2012-05-11 19:06                       ` Konrad Rzeszutek Wilk
2012-05-14  1:55                       ` Minchan Kim
2012-05-14  1:55                         ` Minchan Kim
2012-05-15 15:18                         ` Konrad Rzeszutek Wilk
2012-05-15 15:18                           ` Konrad Rzeszutek Wilk
2012-05-16  1:44                           ` Minchan Kim
2012-05-16  1:44                             ` Minchan Kim
2012-05-03 13:18 ` [PATCH 1/4] zsmalloc: rename zspage_order with zspage_pages Nitin Gupta
2012-05-03 13:18   ` Nitin Gupta

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=4FAC59F6.4080503@kernel.org \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=dan.magenheimer@oracle.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ngupta@vflare.org \
    --cc=sjenning@linux.vnet.ibm.com \
    /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: link
Be 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.