All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] mm, vmalloc: cleanup for vmap block
@ 2013-06-07  9:51 ` Zhang Yanfei
  0 siblings, 0 replies; 12+ messages in thread
From: Zhang Yanfei @ 2013-06-07  9:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: chanho.min, Johannes Weiner, iamjoonsoo.kim, linux-kernel,
	Linux MM, Mel Gorman

This patchset is a cleanup for vmap block. And similar/same
patches has been submitted before:
- Johannes Weiner's patch: https://lkml.org/lkml/2011/4/14/619
- Chanho Min's patch: https://lkml.org/lkml/2013/2/6/810

In Johannes's thread, Mel suggested to figure out if this
bitmap was not supposed to be doing something useful and depending
on that implement recycling of partially used vmap blocks.

Anyway, just as Johannes said, we shouldn't leave these dead/unused
code as is, because it really is a waste of time for cpus and readers
of the code. And this cleanup doesn't prevent anyone from improving
the algorithm later on.

Based on the two patches before, I split the cleanup into three
small pieces that may be more clear.

Zhang Yanfei (3):
  mm, vmalloc: Remove dead code in vb_alloc
  mm, vmalloc: Remove unused purge_fragmented_blocks_thiscpu
  mm, vmalloc: Remove alloc_map from vmap_block

 mm/vmalloc.c |   24 +-----------------------
 1 files changed, 1 insertions(+), 23 deletions(-)

^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH 0/3] mm, vmalloc: cleanup for vmap block
@ 2013-06-07  9:51 ` Zhang Yanfei
  0 siblings, 0 replies; 12+ messages in thread
From: Zhang Yanfei @ 2013-06-07  9:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: chanho.min, Johannes Weiner, iamjoonsoo.kim, linux-kernel,
	Linux MM, Mel Gorman

This patchset is a cleanup for vmap block. And similar/same
patches has been submitted before:
- Johannes Weiner's patch: https://lkml.org/lkml/2011/4/14/619
- Chanho Min's patch: https://lkml.org/lkml/2013/2/6/810

In Johannes's thread, Mel suggested to figure out if this
bitmap was not supposed to be doing something useful and depending
on that implement recycling of partially used vmap blocks.

Anyway, just as Johannes said, we shouldn't leave these dead/unused
code as is, because it really is a waste of time for cpus and readers
of the code. And this cleanup doesn't prevent anyone from improving
the algorithm later on.

Based on the two patches before, I split the cleanup into three
small pieces that may be more clear.

Zhang Yanfei (3):
  mm, vmalloc: Remove dead code in vb_alloc
  mm, vmalloc: Remove unused purge_fragmented_blocks_thiscpu
  mm, vmalloc: Remove alloc_map from vmap_block

 mm/vmalloc.c |   24 +-----------------------
 1 files changed, 1 insertions(+), 23 deletions(-)

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH 1/3] mm, vmalloc: Remove dead code in vb_alloc
  2013-06-07  9:51 ` Zhang Yanfei
@ 2013-06-07  9:53   ` Zhang Yanfei
  -1 siblings, 0 replies; 12+ messages in thread
From: Zhang Yanfei @ 2013-06-07  9:53 UTC (permalink / raw)
  To: Andrew Morton
  Cc: chanho.min, Johannes Weiner, iamjoonsoo.kim, linux-kernel,
	Linux MM, Mel Gorman

Space in a vmap block that was once allocated is considered dirty and
not made available for allocation again before the whole block is
recycled. The result is that free space within a vmap block is always
contiguous.

So if a vmap block has enough free space for allocation, the allocation
is impossible to fail. Thus, the fragmented block purging was never invoked
from vb_alloc(). So remove this dead code.

Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
 mm/vmalloc.c |   16 +---------------
 1 files changed, 1 insertions(+), 15 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index d365724..b8abcba 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -910,7 +910,6 @@ static void *vb_alloc(unsigned long size, gfp_t gfp_mask)
 	struct vmap_block *vb;
 	unsigned long addr = 0;
 	unsigned int order;
-	int purge = 0;
 
 	BUG_ON(size & ~PAGE_MASK);
 	BUG_ON(size > PAGE_SIZE*VMAP_MAX_ALLOC);
@@ -934,17 +933,7 @@ again:
 		if (vb->free < 1UL << order)
 			goto next;
 
-		i = bitmap_find_free_region(vb->alloc_map,
-						VMAP_BBMAP_BITS, order);
-
-		if (i < 0) {
-			if (vb->free + vb->dirty == VMAP_BBMAP_BITS) {
-				/* fragmented and no outstanding allocations */
-				BUG_ON(vb->dirty != VMAP_BBMAP_BITS);
-				purge = 1;
-			}
-			goto next;
-		}
+		i = VMAP_BBMAP_BITS - vb->free;
 		addr = vb->va->va_start + (i << PAGE_SHIFT);
 		BUG_ON(addr_to_vb_idx(addr) !=
 				addr_to_vb_idx(vb->va->va_start));
@@ -960,9 +949,6 @@ next:
 		spin_unlock(&vb->lock);
 	}
 
-	if (purge)
-		purge_fragmented_blocks_thiscpu();
-
 	put_cpu_var(vmap_block_queue);
 	rcu_read_unlock();
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 1/3] mm, vmalloc: Remove dead code in vb_alloc
@ 2013-06-07  9:53   ` Zhang Yanfei
  0 siblings, 0 replies; 12+ messages in thread
From: Zhang Yanfei @ 2013-06-07  9:53 UTC (permalink / raw)
  To: Andrew Morton
  Cc: chanho.min, Johannes Weiner, iamjoonsoo.kim, linux-kernel,
	Linux MM, Mel Gorman

Space in a vmap block that was once allocated is considered dirty and
not made available for allocation again before the whole block is
recycled. The result is that free space within a vmap block is always
contiguous.

So if a vmap block has enough free space for allocation, the allocation
is impossible to fail. Thus, the fragmented block purging was never invoked
from vb_alloc(). So remove this dead code.

Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
 mm/vmalloc.c |   16 +---------------
 1 files changed, 1 insertions(+), 15 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index d365724..b8abcba 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -910,7 +910,6 @@ static void *vb_alloc(unsigned long size, gfp_t gfp_mask)
 	struct vmap_block *vb;
 	unsigned long addr = 0;
 	unsigned int order;
-	int purge = 0;
 
 	BUG_ON(size & ~PAGE_MASK);
 	BUG_ON(size > PAGE_SIZE*VMAP_MAX_ALLOC);
@@ -934,17 +933,7 @@ again:
 		if (vb->free < 1UL << order)
 			goto next;
 
-		i = bitmap_find_free_region(vb->alloc_map,
-						VMAP_BBMAP_BITS, order);
-
-		if (i < 0) {
-			if (vb->free + vb->dirty == VMAP_BBMAP_BITS) {
-				/* fragmented and no outstanding allocations */
-				BUG_ON(vb->dirty != VMAP_BBMAP_BITS);
-				purge = 1;
-			}
-			goto next;
-		}
+		i = VMAP_BBMAP_BITS - vb->free;
 		addr = vb->va->va_start + (i << PAGE_SHIFT);
 		BUG_ON(addr_to_vb_idx(addr) !=
 				addr_to_vb_idx(vb->va->va_start));
@@ -960,9 +949,6 @@ next:
 		spin_unlock(&vb->lock);
 	}
 
-	if (purge)
-		purge_fragmented_blocks_thiscpu();
-
 	put_cpu_var(vmap_block_queue);
 	rcu_read_unlock();
 
-- 
1.7.1

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 2/3] mm, vmalloc: Remove unused purge_fragmented_blocks_thiscpu
  2013-06-07  9:51 ` Zhang Yanfei
@ 2013-06-07  9:54   ` Zhang Yanfei
  -1 siblings, 0 replies; 12+ messages in thread
From: Zhang Yanfei @ 2013-06-07  9:54 UTC (permalink / raw)
  To: Andrew Morton
  Cc: chanho.min, Johannes Weiner, iamjoonsoo.kim, linux-kernel,
	Linux MM, Mel Gorman

This function is nowhere used now, so remove it.

Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
 mm/vmalloc.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index b8abcba..5c037b9 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -891,11 +891,6 @@ static void purge_fragmented_blocks(int cpu)
 	}
 }
 
-static void purge_fragmented_blocks_thiscpu(void)
-{
-	purge_fragmented_blocks(smp_processor_id());
-}
-
 static void purge_fragmented_blocks_allcpus(void)
 {
 	int cpu;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 2/3] mm, vmalloc: Remove unused purge_fragmented_blocks_thiscpu
@ 2013-06-07  9:54   ` Zhang Yanfei
  0 siblings, 0 replies; 12+ messages in thread
From: Zhang Yanfei @ 2013-06-07  9:54 UTC (permalink / raw)
  To: Andrew Morton
  Cc: chanho.min, Johannes Weiner, iamjoonsoo.kim, linux-kernel,
	Linux MM, Mel Gorman

This function is nowhere used now, so remove it.

Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
 mm/vmalloc.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index b8abcba..5c037b9 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -891,11 +891,6 @@ static void purge_fragmented_blocks(int cpu)
 	}
 }
 
-static void purge_fragmented_blocks_thiscpu(void)
-{
-	purge_fragmented_blocks(smp_processor_id());
-}
-
 static void purge_fragmented_blocks_allcpus(void)
 {
 	int cpu;
-- 
1.7.1

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 3/3] mm, vmalloc: Remove alloc_map from vmap_block
  2013-06-07  9:51 ` Zhang Yanfei
@ 2013-06-07  9:55   ` Zhang Yanfei
  -1 siblings, 0 replies; 12+ messages in thread
From: Zhang Yanfei @ 2013-06-07  9:55 UTC (permalink / raw)
  To: Andrew Morton
  Cc: chanho.min, Johannes Weiner, iamjoonsoo.kim, linux-kernel,
	Linux MM, Mel Gorman

As we have removed the dead code in the vb_alloc, it seems there is
no place to use the alloc_map. So there is no reason to maintain
the alloc_map in vmap_block.

Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
 mm/vmalloc.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 5c037b9..ad11d4f 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -754,7 +754,6 @@ struct vmap_block {
 	struct vmap_area *va;
 	struct vmap_block_queue *vbq;
 	unsigned long free, dirty;
-	DECLARE_BITMAP(alloc_map, VMAP_BBMAP_BITS);
 	DECLARE_BITMAP(dirty_map, VMAP_BBMAP_BITS);
 	struct list_head free_list;
 	struct rcu_head rcu_head;
@@ -820,7 +819,6 @@ static struct vmap_block *new_vmap_block(gfp_t gfp_mask)
 	vb->va = va;
 	vb->free = VMAP_BBMAP_BITS;
 	vb->dirty = 0;
-	bitmap_zero(vb->alloc_map, VMAP_BBMAP_BITS);
 	bitmap_zero(vb->dirty_map, VMAP_BBMAP_BITS);
 	INIT_LIST_HEAD(&vb->free_list);
 
@@ -873,7 +871,6 @@ static void purge_fragmented_blocks(int cpu)
 		if (vb->free + vb->dirty == VMAP_BBMAP_BITS && vb->dirty != VMAP_BBMAP_BITS) {
 			vb->free = 0; /* prevent further allocs after releasing lock */
 			vb->dirty = VMAP_BBMAP_BITS; /* prevent purging it again */
-			bitmap_fill(vb->alloc_map, VMAP_BBMAP_BITS);
 			bitmap_fill(vb->dirty_map, VMAP_BBMAP_BITS);
 			spin_lock(&vbq->lock);
 			list_del_rcu(&vb->free_list);
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 3/3] mm, vmalloc: Remove alloc_map from vmap_block
@ 2013-06-07  9:55   ` Zhang Yanfei
  0 siblings, 0 replies; 12+ messages in thread
From: Zhang Yanfei @ 2013-06-07  9:55 UTC (permalink / raw)
  To: Andrew Morton
  Cc: chanho.min, Johannes Weiner, iamjoonsoo.kim, linux-kernel,
	Linux MM, Mel Gorman

As we have removed the dead code in the vb_alloc, it seems there is
no place to use the alloc_map. So there is no reason to maintain
the alloc_map in vmap_block.

Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
 mm/vmalloc.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 5c037b9..ad11d4f 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -754,7 +754,6 @@ struct vmap_block {
 	struct vmap_area *va;
 	struct vmap_block_queue *vbq;
 	unsigned long free, dirty;
-	DECLARE_BITMAP(alloc_map, VMAP_BBMAP_BITS);
 	DECLARE_BITMAP(dirty_map, VMAP_BBMAP_BITS);
 	struct list_head free_list;
 	struct rcu_head rcu_head;
@@ -820,7 +819,6 @@ static struct vmap_block *new_vmap_block(gfp_t gfp_mask)
 	vb->va = va;
 	vb->free = VMAP_BBMAP_BITS;
 	vb->dirty = 0;
-	bitmap_zero(vb->alloc_map, VMAP_BBMAP_BITS);
 	bitmap_zero(vb->dirty_map, VMAP_BBMAP_BITS);
 	INIT_LIST_HEAD(&vb->free_list);
 
@@ -873,7 +871,6 @@ static void purge_fragmented_blocks(int cpu)
 		if (vb->free + vb->dirty == VMAP_BBMAP_BITS && vb->dirty != VMAP_BBMAP_BITS) {
 			vb->free = 0; /* prevent further allocs after releasing lock */
 			vb->dirty = VMAP_BBMAP_BITS; /* prevent purging it again */
-			bitmap_fill(vb->alloc_map, VMAP_BBMAP_BITS);
 			bitmap_fill(vb->dirty_map, VMAP_BBMAP_BITS);
 			spin_lock(&vbq->lock);
 			list_del_rcu(&vb->free_list);
-- 
1.7.1

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* RE: [PATCH 0/3] mm, vmalloc: cleanup for vmap block
  2013-06-07  9:51 ` Zhang Yanfei
                   ` (4 preceding siblings ...)
  (?)
@ 2013-06-10  1:12 ` Chanho Min
  2013-06-20  6:38     ` Johannes Weiner
  -1 siblings, 1 reply; 12+ messages in thread
From: Chanho Min @ 2013-06-10  1:12 UTC (permalink / raw)
  To: 'Zhang Yanfei', 'Andrew Morton'
  Cc: 'Johannes Weiner',
	iamjoonsoo.kim, linux-kernel, 'Linux MM',
	'Mel Gorman'

> This patchset is a cleanup for vmap block. And similar/same
> patches has been submitted before:
> - Johannes Weiner's patch: https://lkml.org/lkml/2011/4/14/619
> - Chanho Min's patch: https://lkml.org/lkml/2013/2/6/810

This is exactly the same patch as mine. The previous two patches are
should be concluded.

> In Johannes's thread, Mel suggested to figure out if this
> bitmap was not supposed to be doing something useful and depending
> on that implement recycling of partially used vmap blocks.
> 
> Anyway, just as Johannes said, we shouldn't leave these dead/unused
> code as is, because it really is a waste of time for cpus and readers
> of the code. And this cleanup doesn't prevent anyone from improving
> the algorithm later on.

I agree. This unnecessarily bitmap operation can cause significant
overhead as https://lkml.org/lkml/2013/2/7/705.

Thanks
Chanho Min

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 12+ messages in thread
* RE: [PATCH 0/3] mm, vmalloc: cleanup for vmap block
  2013-06-07  9:51 ` Zhang Yanfei
                   ` (3 preceding siblings ...)
  (?)
@ 2013-06-10  1:12 ` Chanho Min
  -1 siblings, 0 replies; 12+ messages in thread
From: Chanho Min @ 2013-06-10  1:12 UTC (permalink / raw)
  To: 'Zhang Yanfei', 'Andrew Morton'
  Cc: 'Johannes Weiner',
	iamjoonsoo.kim, linux-kernel, 'Linux MM',
	'Mel Gorman'

> This patchset is a cleanup for vmap block. And similar/same
> patches has been submitted before:
> - Johannes Weiner's patch: https://lkml.org/lkml/2011/4/14/619
> - Chanho Min's patch: https://lkml.org/lkml/2013/2/6/810

This is exactly the same patch as mine. The previous two patches are
should be concluded.

> In Johannes's thread, Mel suggested to figure out if this
> bitmap was not supposed to be doing something useful and depending
> on that implement recycling of partially used vmap blocks.
> 
> Anyway, just as Johannes said, we shouldn't leave these dead/unused
> code as is, because it really is a waste of time for cpus and readers
> of the code. And this cleanup doesn't prevent anyone from improving
> the algorithm later on.

I agree. This unnecessarily bitmap operation can cause significant
overhead as https://lkml.org/lkml/2013/2/7/705.

Thanks
Chanho Min

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] mm, vmalloc: cleanup for vmap block
  2013-06-10  1:12 ` Chanho Min
@ 2013-06-20  6:38     ` Johannes Weiner
  0 siblings, 0 replies; 12+ messages in thread
From: Johannes Weiner @ 2013-06-20  6:38 UTC (permalink / raw)
  To: Chanho Min
  Cc: 'Zhang Yanfei', 'Andrew Morton',
	iamjoonsoo.kim, linux-kernel, 'Linux MM',
	'Mel Gorman'

On Mon, Jun 10, 2013 at 10:12:57AM +0900, Chanho Min wrote:
> > This patchset is a cleanup for vmap block. And similar/same
> > patches has been submitted before:
> > - Johannes Weiner's patch: https://lkml.org/lkml/2011/4/14/619
> > - Chanho Min's patch: https://lkml.org/lkml/2013/2/6/810
> 
> This is exactly the same patch as mine. The previous two patches are
> should be concluded.

Ping.  This code is identical, it should credit Chanho as the original
author.  And I agree that the patches should be folded into one patch.

Thanks!

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] mm, vmalloc: cleanup for vmap block
@ 2013-06-20  6:38     ` Johannes Weiner
  0 siblings, 0 replies; 12+ messages in thread
From: Johannes Weiner @ 2013-06-20  6:38 UTC (permalink / raw)
  To: Chanho Min
  Cc: 'Zhang Yanfei', 'Andrew Morton',
	iamjoonsoo.kim, linux-kernel, 'Linux MM',
	'Mel Gorman'

On Mon, Jun 10, 2013 at 10:12:57AM +0900, Chanho Min wrote:
> > This patchset is a cleanup for vmap block. And similar/same
> > patches has been submitted before:
> > - Johannes Weiner's patch: https://lkml.org/lkml/2011/4/14/619
> > - Chanho Min's patch: https://lkml.org/lkml/2013/2/6/810
> 
> This is exactly the same patch as mine. The previous two patches are
> should be concluded.

Ping.  This code is identical, it should credit Chanho as the original
author.  And I agree that the patches should be folded into one patch.

Thanks!

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-06-20  6:38 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-07  9:51 [PATCH 0/3] mm, vmalloc: cleanup for vmap block Zhang Yanfei
2013-06-07  9:51 ` Zhang Yanfei
2013-06-07  9:53 ` [PATCH 1/3] mm, vmalloc: Remove dead code in vb_alloc Zhang Yanfei
2013-06-07  9:53   ` Zhang Yanfei
2013-06-07  9:54 ` [PATCH 2/3] mm, vmalloc: Remove unused purge_fragmented_blocks_thiscpu Zhang Yanfei
2013-06-07  9:54   ` Zhang Yanfei
2013-06-07  9:55 ` [PATCH 3/3] mm, vmalloc: Remove alloc_map from vmap_block Zhang Yanfei
2013-06-07  9:55   ` Zhang Yanfei
2013-06-10  1:12 ` [PATCH 0/3] mm, vmalloc: cleanup for vmap block Chanho Min
2013-06-10  1:12 ` Chanho Min
2013-06-20  6:38   ` Johannes Weiner
2013-06-20  6:38     ` Johannes Weiner

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.