Skip to contentSkip to conversation

mm, vmalloc: cleanup for vmap block

PATCH In review Zhang Yanfei 3 participants 7 messages Jun 7, 2013 – Jun 20, 2013

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover mm, vmalloc: cleanup for vmap block

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#1 lore raw

PATCH 1/3 mm, vmalloc: Remove dead code in vb_alloc

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.

---
 mm/vmalloc.c |   16 +---------------
 1 files changed, 1 insertions(+), 15 deletions(-)
mm/vmalloc.c +1 -15
index d365724..b8abcba 100644
@@ -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();
 
Signature · 7 lines
1.7.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#2 lore raw

PATCH 2/3 mm, vmalloc: Remove unused purge_fragmented_blocks_thiscpu

In reply to Zhang Yanfei

This function is nowhere used now, so remove it.

---
 mm/vmalloc.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)
mm/vmalloc.c +0 -5
index b8abcba..5c037b9 100644
@@ -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;
Signature · 7 lines
1.7.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#3 lore raw

PATCH 3/3 mm, vmalloc: Remove alloc_map from vmap_block

In reply to Zhang Yanfei

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.

---
 mm/vmalloc.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)
mm/vmalloc.c +0 -3
index 5c037b9..ad11d4f 100644
@@ -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);
Signature · 7 lines
1.7.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#4 lore raw
In reply to Zhang Yanfei
Quoted 4 lines

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.

Quoted 8 lines

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#5 lore raw
Quoted 7 lines · Chanho Min
Quoted 4 lines

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!

#6 lore raw
In reply to Zhang Yanfei
Quoted 4 lines

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.

Quoted 8 lines

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>