2013-06-07 09:54:29

by Zhang Yanfei

[permalink] [raw]
Subject: [PATCH 0/3] 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(-)


2013-06-07 09:56:22

by Zhang Yanfei

[permalink] [raw]
Subject: [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.

Signed-off-by: Zhang Yanfei <[email protected]>
---
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

2013-06-07 09:57:00

by Zhang Yanfei

[permalink] [raw]
Subject: [PATCH 2/3] mm, vmalloc: Remove unused purge_fragmented_blocks_thiscpu

This function is nowhere used now, so remove it.

Signed-off-by: Zhang Yanfei <[email protected]>
---
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

2013-06-07 09:57:55

by Zhang Yanfei

[permalink] [raw]
Subject: [PATCH 3/3] mm, vmalloc: Remove alloc_map from vmap_block

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 <[email protected]>
---
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

2013-06-20 06:38:29

by Johannes Weiner

[permalink] [raw]
Subject: Re: [PATCH 0/3] mm, vmalloc: cleanup for vmap block

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!