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

Groups > linux.kernel > #1601894 > unrolled thread

[PATCH 1/4] mm/zsmalloc: always set movable/highmem flag to the zspage

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/4] mm/zsmalloc: always set movable/highmem flag to the zspage js1304@gmail.com - 2017-03-16 03:50 +0100
    Re: [PATCH 1/4] mm/zsmalloc: always set movable/highmem flag to the  zspage Minchan Kim <minchan@kernel.org> - 2017-03-21 12:20 +0100
      Re: [PATCH 1/4] mm/zsmalloc: always set movable/highmem flag to the  zspage Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2017-03-23 03:10 +0100
        Re: [PATCH 1/4] mm/zsmalloc: always set movable/highmem flag to the  zspage Minchan Kim <minchan@kernel.org> - 2017-03-24 02:00 +0100

#1601894 — [PATCH 1/4] mm/zsmalloc: always set movable/highmem flag to the zspage

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

Zspage is always movable and is used through zs_map_object() function
which returns directly accessible pointer that contains content of
zspage. It is independent on the user's allocation flag.
Therefore, it's better to always set movable/highmem flag to the zspage.
After that, we don't need __GFP_MOVABLE/__GFP_HIGHMEM clearing in
cache_alloc_handle()/cache_alloc_zspage() since there is no zs_malloc
caller who specifies __GFP_MOVABLE/__GFP_HIGHMEM.

Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
 drivers/block/zram/zram_drv.c |  9 ++-------
 mm/zsmalloc.c                 | 10 ++++------
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 0194441..f65dcd1 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -684,19 +684,14 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
 	 */
 	if (!handle)
 		handle = zs_malloc(meta->mem_pool, clen,
-				__GFP_KSWAPD_RECLAIM |
-				__GFP_NOWARN |
-				__GFP_HIGHMEM |
-				__GFP_MOVABLE);
+				__GFP_KSWAPD_RECLAIM | __GFP_NOWARN);
 	if (!handle) {
 		zcomp_stream_put(zram->comp);
 		zstrm = NULL;
 
 		atomic64_inc(&zram->stats.writestall);
 
-		handle = zs_malloc(meta->mem_pool, clen,
-				GFP_NOIO | __GFP_HIGHMEM |
-				__GFP_MOVABLE);
+		handle = zs_malloc(meta->mem_pool, clen, GFP_NOIO);
 		if (handle)
 			goto compress_again;
 
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index b7ee9c3..fada232 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -347,8 +347,7 @@ static void destroy_cache(struct zs_pool *pool)
 
 static unsigned long cache_alloc_handle(struct zs_pool *pool, gfp_t gfp)
 {
-	return (unsigned long)kmem_cache_alloc(pool->handle_cachep,
-			gfp & ~(__GFP_HIGHMEM|__GFP_MOVABLE));
+	return (unsigned long)kmem_cache_alloc(pool->handle_cachep, gfp);
 }
 
 static void cache_free_handle(struct zs_pool *pool, unsigned long handle)
@@ -358,9 +357,8 @@ static void cache_free_handle(struct zs_pool *pool, unsigned long handle)
 
 static struct zspage *cache_alloc_zspage(struct zs_pool *pool, gfp_t flags)
 {
-	return kmem_cache_alloc(pool->zspage_cachep,
-			flags & ~(__GFP_HIGHMEM|__GFP_MOVABLE));
-}
+	return kmem_cache_alloc(pool->zspage_cachep, flags);
+};
 
 static void cache_free_zspage(struct zs_pool *pool, struct zspage *zspage)
 {
@@ -1120,7 +1118,7 @@ static struct zspage *alloc_zspage(struct zs_pool *pool,
 	for (i = 0; i < class->pages_per_zspage; i++) {
 		struct page *page;
 
-		page = alloc_page(gfp);
+		page = alloc_page(gfp | __GFP_MOVABLE | __GFP_HIGHMEM);
 		if (!page) {
 			while (--i >= 0) {
 				dec_zone_page_state(pages[i], NR_ZSPAGES);
-- 
1.9.1

[toc] | [next] | [standalone]

#1605507 — Re: [PATCH 1/4] mm/zsmalloc: always set movable/highmem flag to the zspage

Hi Joonsoo,

On Thu, Mar 16, 2017 at 11:46:35AM +0900, js1304@gmail.com wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> 
> Zspage is always movable and is used through zs_map_object() function
> which returns directly accessible pointer that contains content of
> zspage. It is independent on the user's allocation flag.
> Therefore, it's better to always set movable/highmem flag to the zspage.
> After that, we don't need __GFP_MOVABLE/__GFP_HIGHMEM clearing in
> cache_alloc_handle()/cache_alloc_zspage() since there is no zs_malloc
> caller who specifies __GFP_MOVABLE/__GFP_HIGHMEM.

Hmm, I wanted this when you pointed out to me firstly but when I think
again, I don't see it's improvement. Sorry for that.
The zs_malloc is exported symbol and it has gfp_t argument so user can
do whatever he want with any zone modifiers flags. IOW, if someuser want
to allocate pages from {normal|dma} zone by whatever reason, he can
omit __GFP_HIGHMEM from the gfp flag to fullfill the goal.

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

#1607120 — Re: [PATCH 1/4] mm/zsmalloc: always set movable/highmem flag to the zspage

On Tue, Mar 21, 2017 at 08:10:05PM +0900, Minchan Kim wrote:
> Hi Joonsoo,
> 
> On Thu, Mar 16, 2017 at 11:46:35AM +0900, js1304@gmail.com wrote:
> > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> > 
> > Zspage is always movable and is used through zs_map_object() function
> > which returns directly accessible pointer that contains content of
> > zspage. It is independent on the user's allocation flag.
> > Therefore, it's better to always set movable/highmem flag to the zspage.
> > After that, we don't need __GFP_MOVABLE/__GFP_HIGHMEM clearing in
> > cache_alloc_handle()/cache_alloc_zspage() since there is no zs_malloc
> > caller who specifies __GFP_MOVABLE/__GFP_HIGHMEM.
> 
> Hmm, I wanted this when you pointed out to me firstly but when I think
> again, I don't see it's improvement. Sorry for that.
> The zs_malloc is exported symbol and it has gfp_t argument so user can
> do whatever he want with any zone modifiers flags. IOW, if someuser want
> to allocate pages from {normal|dma} zone by whatever reason, he can
> omit __GFP_HIGHMEM from the gfp flag to fullfill the goal.

Hello,

I don't think that such flexibility makes things better. User cannot
fully understand what flags are the best since it highly depends on
implementation detail. For example, __GFP_MOVABLE is needed to
optimize memory fragmentation and user cannot know it and there is no
reason that user need to know it. __GFP_HIGHMEM is the similar case.
He cannot know that he can pass __GFP_HIGHMEM without knowing the
implementation detail and he cannot know the impact of __GFP_HIGHMEM
here. So, I think that adding these flags in zsmalloc can be justified.

Anyway, this patch isn't so important for this series so if you don't
like it, I will drop it.

Thanks.

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

#1608041 — Re: [PATCH 1/4] mm/zsmalloc: always set movable/highmem flag to the zspage

Hi Joonsoo,

On Thu, Mar 23, 2017 at 11:10:23AM +0900, Joonsoo Kim wrote:
> On Tue, Mar 21, 2017 at 08:10:05PM +0900, Minchan Kim wrote:
> > Hi Joonsoo,
> > 
> > On Thu, Mar 16, 2017 at 11:46:35AM +0900, js1304@gmail.com wrote:
> > > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> > > 
> > > Zspage is always movable and is used through zs_map_object() function
> > > which returns directly accessible pointer that contains content of
> > > zspage. It is independent on the user's allocation flag.
> > > Therefore, it's better to always set movable/highmem flag to the zspage.
> > > After that, we don't need __GFP_MOVABLE/__GFP_HIGHMEM clearing in
> > > cache_alloc_handle()/cache_alloc_zspage() since there is no zs_malloc
> > > caller who specifies __GFP_MOVABLE/__GFP_HIGHMEM.
> > 
> > Hmm, I wanted this when you pointed out to me firstly but when I think
> > again, I don't see it's improvement. Sorry for that.
> > The zs_malloc is exported symbol and it has gfp_t argument so user can
> > do whatever he want with any zone modifiers flags. IOW, if someuser want
> > to allocate pages from {normal|dma} zone by whatever reason, he can
> > omit __GFP_HIGHMEM from the gfp flag to fullfill the goal.
> 
> Hello,
> 
> I don't think that such flexibility makes things better. User cannot
> fully understand what flags are the best since it highly depends on
> implementation detail. For example, __GFP_MOVABLE is needed to

zone modifier(GFP_DMA|DMA32|HIGHMEM|MOVABLE|potentially CMA):
User can select one of zone for his goal by S/W|H/W constraint.

> optimize memory fragmentation and user cannot know it and there is no
> reason that user need to know it. __GFP_HIGHMEM is the similar case.
> He cannot know that he can pass __GFP_HIGHMEM without knowing the
> implementation detail and he cannot know the impact of __GFP_HIGHMEM
> here. So, I think that adding these flags in zsmalloc can be justified.
> 
> Anyway, this patch isn't so important for this series so if you don't
> like it, I will drop it.

Yes, I don't feel strongly we need it at this moment.

Thanks.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web