Problem:
***
Stephen reported vread() will skip vm_map_ram areas when reading out
/proc/kcore with drgn utility. Please see below link to get more
details.
/proc/kcore reads 0's for vmap_block
https://lore.kernel.org/all/[email protected]/T/#u
Root cause:
***
The normal vmalloc API uses struct vmap_area to manage the virtual
kernel area allocated, and associate a vm_struct to store more
information and pass out. However, area reserved through vm_map_ram()
interface doesn't allocate vm_struct to associate with. So the current
code in vread() will skip the vm_map_ram area through 'if (!va->vm)'
conditional checking.
Solution:
***
To mark the area reserved through vm_map_ram() interface, add field 'flags'
into struct vmap_area. Bit 0 indicates this is vm_map_ram area created
through vm_map_ram() interface, bit 1 marks out the type of vm_map_ram area
which makes use of vmap_block to manage split regions via vb_alloc/free().
And also add bitmap field 'used_map' into struct vmap_block to mark those
further subdivided regions being used to differentiate with dirty and free
regions in vmap_block.
With the help of above vmap_area->flags and vmap_block->used_map, we can
recognize and handle vm_map_ram areas successfully. All these are done
in patch 1~3.
Meanwhile, do some improvement on areas related to vm_map_ram areas in
patch 4, 5. And also change area flag from VM_ALLOC to VM_IOREMAP in
patch 6, 7 because this will show them as 'ioremap' in /proc/vmallocinfo,
and exclude them from /proc/kcore.
Testing
***
Only did the basic testing on kvm guest, and run below command to
access kcore file to do statistics:
makedumpfile --mem-usage /proc/kcore
Changelog
***
v2->v3:
- Benefited from find_unlink_vmap_area() introduced by Uladzislau, do
not need to worry about the va->vm and va->flags reset during freeing.
- Change to identify vm_map_area with VMAP_RAM in vmap->flags in
vread().
- Rename the old vb_vread() to vmap_ram_vread().
- Handle two kinds of vm_map_area area reading in vmap_ram_vread()
respectively.
- Fix bug of the while loop code block in vmap_block reading, found by
Lorenzo.
- Improve conditional check related to vm_map_ram area, suggested by
Lorenzo.
v1->v2:
- Change alloc_vmap_area() to pass in va_flags so that we can pass and
set vmap_area->flags for vm_map_ram area. With this, no extra action
need be added to acquire vmap_area_lock when doing the vmap_area->flags
setting. Uladzislau reviewed v1 and pointed out the issue.
- Improve vb_vread() to cover the case where reading is started from a
dirty or free region.
RFC->v1:
- Add a new field flags in vmap_area to mark vm_map_ram area. It cold be
risky reusing the vm union in vmap_area in RFC. I will consider
reusing the union in vmap_area to save memory later. Now just take the
simpler way to let's focus on resolving the main problem.
- Add patch 4~7 for optimization.
Baoquan He (7):
mm/vmalloc.c: add used_map into vmap_block to track space of
vmap_block
mm/vmalloc.c: add flags to mark vm_map_ram area
mm/vmalloc.c: allow vread() to read out vm_map_ram areas
mm/vmalloc: explicitly identify vm_map_ram area when shown in
/proc/vmcoreinfo
mm/vmalloc: skip the uninitilized vmalloc areas
powerpc: mm: add VM_IOREMAP flag to the vmalloc area
sh: mm: set VM_IOREMAP flag to the vmalloc area
arch/powerpc/kernel/pci_64.c | 2 +-
arch/sh/kernel/cpu/sh4/sq.c | 2 +-
include/linux/vmalloc.h | 1 +
mm/vmalloc.c | 114 ++++++++++++++++++++++++++++++-----
4 files changed, 101 insertions(+), 18 deletions(-)
--
2.34.1
Through vmalloc API, a virtual kernel area is reserved for physical
address mapping. And vmap_area is used to track them, while vm_struct
is allocated to associate with the vmap_area to store more information
and passed out.
However, area reserved via vm_map_ram() is an exception. It doesn't have
vm_struct to associate with vmap_area. And we can't recognize the
vmap_area with '->vm == NULL' as a vm_map_ram() area because the normal
freeing path will set va->vm = NULL before unmapping, please see
function remove_vm_area().
Meanwhile, there are two kinds of handling for vm_map_ram area. One is
the whole vmap_area being reserved and mapped at one time through
vm_map_area() interface; the other is the whole vmap_area with
VMAP_BLOCK_SIZE size being reserved, while mapped into split regions
with smaller size via vb_alloc().
To mark the area reserved through vm_map_ram(), add flags field into
struct vmap_area. Bit 0 indicates this is vm_map_ram area created
through vm_map_ram() interface, while bit 1 marks out the type of
vm_map_ram area which makes use of vmap_block to manage split regions
via vb_alloc/free().
This is a preparatoin for later use.
Signed-off-by: Baoquan He <[email protected]>
---
include/linux/vmalloc.h | 1 +
mm/vmalloc.c | 16 ++++++++++++----
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 096d48aa3437..69250efa03d1 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -76,6 +76,7 @@ struct vmap_area {
unsigned long subtree_max_size; /* in "free" tree */
struct vm_struct *vm; /* in "busy" tree */
};
+ unsigned long flags; /* mark type of vm_map_ram area */
};
/* archs that select HAVE_ARCH_HUGE_VMAP should override one or more of these */
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index d6ff058ef4d0..ab4825050b5c 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1589,7 +1589,8 @@ preload_this_cpu_lock(spinlock_t *lock, gfp_t gfp_mask, int node)
static struct vmap_area *alloc_vmap_area(unsigned long size,
unsigned long align,
unsigned long vstart, unsigned long vend,
- int node, gfp_t gfp_mask)
+ int node, gfp_t gfp_mask,
+ unsigned long va_flags)
{
struct vmap_area *va;
unsigned long freed;
@@ -1635,6 +1636,7 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
va->va_start = addr;
va->va_end = addr + size;
va->vm = NULL;
+ va->flags = va_flags;
spin_lock(&vmap_area_lock);
insert_vmap_area(va, &vmap_area_root, &vmap_area_list);
@@ -1913,6 +1915,10 @@ static struct vmap_area *find_unlink_vmap_area(unsigned long addr)
#define VMAP_BLOCK_SIZE (VMAP_BBMAP_BITS * PAGE_SIZE)
+#define VMAP_RAM 0x1 /* indicates vm_map_ram area*/
+#define VMAP_BLOCK 0x2 /* mark out the vmap_block sub-type*/
+#define VMAP_FLAGS_MASK 0x3
+
struct vmap_block_queue {
spinlock_t lock;
struct list_head free;
@@ -1988,7 +1994,8 @@ static void *new_vmap_block(unsigned int order, gfp_t gfp_mask)
va = alloc_vmap_area(VMAP_BLOCK_SIZE, VMAP_BLOCK_SIZE,
VMALLOC_START, VMALLOC_END,
- node, gfp_mask);
+ node, gfp_mask,
+ VMAP_RAM|VMAP_BLOCK);
if (IS_ERR(va)) {
kfree(vb);
return ERR_CAST(va);
@@ -2297,7 +2304,8 @@ void *vm_map_ram(struct page **pages, unsigned int count, int node)
} else {
struct vmap_area *va;
va = alloc_vmap_area(size, PAGE_SIZE,
- VMALLOC_START, VMALLOC_END, node, GFP_KERNEL);
+ VMALLOC_START, VMALLOC_END,
+ node, GFP_KERNEL, VMAP_RAM);
if (IS_ERR(va))
return NULL;
@@ -2537,7 +2545,7 @@ static struct vm_struct *__get_vm_area_node(unsigned long size,
if (!(flags & VM_NO_GUARD))
size += PAGE_SIZE;
- va = alloc_vmap_area(size, align, start, end, node, gfp_mask);
+ va = alloc_vmap_area(size, align, start, end, node, gfp_mask, 0);
if (IS_ERR(va)) {
kfree(area);
return NULL;
--
2.34.1
In one vmap_block area, there could be three types of regions: region
being used which is allocated through vb_alloc(), dirty region which
is freed via vb_free() and free region. Among them, only used region
has available data. While there's no way to track those used regions
currently.
Here, add bitmap field used_map into vmap_block, and set/clear it during
allocation or freeing regions of vmap_block area.
This is a preparatoin for later use.
Signed-off-by: Baoquan He <[email protected]>
---
mm/vmalloc.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 428e0bee5c9c..d6ff058ef4d0 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1922,6 +1922,7 @@ struct vmap_block {
spinlock_t lock;
struct vmap_area *va;
unsigned long free, dirty;
+ DECLARE_BITMAP(used_map, VMAP_BBMAP_BITS);
unsigned long dirty_min, dirty_max; /*< dirty range */
struct list_head free_list;
struct rcu_head rcu_head;
@@ -1998,10 +1999,12 @@ static void *new_vmap_block(unsigned int order, gfp_t gfp_mask)
vb->va = va;
/* At least something should be left free */
BUG_ON(VMAP_BBMAP_BITS <= (1UL << order));
+ bitmap_zero(vb->used_map, VMAP_BBMAP_BITS);
vb->free = VMAP_BBMAP_BITS - (1UL << order);
vb->dirty = 0;
vb->dirty_min = VMAP_BBMAP_BITS;
vb->dirty_max = 0;
+ bitmap_set(vb->used_map, 0, (1UL << order));
INIT_LIST_HEAD(&vb->free_list);
vb_idx = addr_to_vb_idx(va->va_start);
@@ -2111,6 +2114,7 @@ static void *vb_alloc(unsigned long size, gfp_t gfp_mask)
pages_off = VMAP_BBMAP_BITS - vb->free;
vaddr = vmap_block_vaddr(vb->va->va_start, pages_off);
vb->free -= 1UL << order;
+ bitmap_set(vb->used_map, pages_off, (1UL << order));
if (vb->free == 0) {
spin_lock(&vbq->lock);
list_del_rcu(&vb->free_list);
@@ -2144,6 +2148,9 @@ static void vb_free(unsigned long addr, unsigned long size)
order = get_order(size);
offset = (addr & (VMAP_BLOCK_SIZE - 1)) >> PAGE_SHIFT;
vb = xa_load(&vmap_blocks, addr_to_vb_idx(addr));
+ spin_lock(&vb->lock);
+ bitmap_clear(vb->used_map, offset, (1UL << order));
+ spin_unlock(&vb->lock);
vunmap_range_noflush(addr, addr + size);
--
2.34.1
Currently, for vmalloc areas with flag VM_IOREMAP set, except of the
specific alignment clamping in __get_vm_area_node(), they will be
1) Shown as ioremap in /proc/vmallocinfo;
2) Ignored by /proc/kcore reading via vread()
So for the io mapping in ioremap_phb() of ppc, we should set VM_IOREMAP
in flag to make it handled correctly as above.
Signed-off-by: Baoquan He <[email protected]>
---
arch/powerpc/kernel/pci_64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 0c7cfb9fab04..fd42059ae2a5 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -132,7 +132,7 @@ void __iomem *ioremap_phb(phys_addr_t paddr, unsigned long size)
* address decoding but I'd rather not deal with those outside of the
* reserved 64K legacy region.
*/
- area = __get_vm_area_caller(size, 0, PHB_IO_BASE, PHB_IO_END,
+ area = __get_vm_area_caller(size, VM_IOREMAP, PHB_IO_BASE, PHB_IO_END,
__builtin_return_address(0));
if (!area)
return NULL;
--
2.34.1
On Fri, Jan 13, 2023 at 11:19:15AM +0800, Baoquan He wrote:
> In one vmap_block area, there could be three types of regions: region
> being used which is allocated through vb_alloc(), dirty region which
> is freed via vb_free() and free region. Among them, only used region
> has available data. While there's no way to track those used regions
> currently.
>
> Here, add bitmap field used_map into vmap_block, and set/clear it during
> allocation or freeing regions of vmap_block area.
>
> This is a preparatoin for later use.
>
> Signed-off-by: Baoquan He <[email protected]>
> ---
> mm/vmalloc.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 428e0bee5c9c..d6ff058ef4d0 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1922,6 +1922,7 @@ struct vmap_block {
> spinlock_t lock;
> struct vmap_area *va;
> unsigned long free, dirty;
> + DECLARE_BITMAP(used_map, VMAP_BBMAP_BITS);
> unsigned long dirty_min, dirty_max; /*< dirty range */
> struct list_head free_list;
> struct rcu_head rcu_head;
> @@ -1998,10 +1999,12 @@ static void *new_vmap_block(unsigned int order, gfp_t gfp_mask)
> vb->va = va;
> /* At least something should be left free */
> BUG_ON(VMAP_BBMAP_BITS <= (1UL << order));
> + bitmap_zero(vb->used_map, VMAP_BBMAP_BITS);
> vb->free = VMAP_BBMAP_BITS - (1UL << order);
> vb->dirty = 0;
> vb->dirty_min = VMAP_BBMAP_BITS;
> vb->dirty_max = 0;
> + bitmap_set(vb->used_map, 0, (1UL << order));
> INIT_LIST_HEAD(&vb->free_list);
>
> vb_idx = addr_to_vb_idx(va->va_start);
> @@ -2111,6 +2114,7 @@ static void *vb_alloc(unsigned long size, gfp_t gfp_mask)
> pages_off = VMAP_BBMAP_BITS - vb->free;
> vaddr = vmap_block_vaddr(vb->va->va_start, pages_off);
> vb->free -= 1UL << order;
> + bitmap_set(vb->used_map, pages_off, (1UL << order));
> if (vb->free == 0) {
> spin_lock(&vbq->lock);
> list_del_rcu(&vb->free_list);
> @@ -2144,6 +2148,9 @@ static void vb_free(unsigned long addr, unsigned long size)
> order = get_order(size);
> offset = (addr & (VMAP_BLOCK_SIZE - 1)) >> PAGE_SHIFT;
> vb = xa_load(&vmap_blocks, addr_to_vb_idx(addr));
> + spin_lock(&vb->lock);
> + bitmap_clear(vb->used_map, offset, (1UL << order));
> + spin_unlock(&vb->lock);
>
> vunmap_range_noflush(addr, addr + size);
>
> --
> 2.34.1
>
Reviewed-by: Uladzislau Rezki (Sony) <[email protected]>
--
Uladzislau Rezki
On Fri, Jan 13, 2023 at 11:19:20AM +0800, Baoquan He wrote:
> Currently, for vmalloc areas with flag VM_IOREMAP set, except of the
> specific alignment clamping in __get_vm_area_node(), they will be
> 1) Shown as ioremap in /proc/vmallocinfo;
> 2) Ignored by /proc/kcore reading via vread()
>
> So for the io mapping in ioremap_phb() of ppc, we should set VM_IOREMAP
> in flag to make it handled correctly as above.
>
> Signed-off-by: Baoquan He <[email protected]>
> ---
> arch/powerpc/kernel/pci_64.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index 0c7cfb9fab04..fd42059ae2a5 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -132,7 +132,7 @@ void __iomem *ioremap_phb(phys_addr_t paddr, unsigned long size)
> * address decoding but I'd rather not deal with those outside of the
> * reserved 64K legacy region.
> */
> - area = __get_vm_area_caller(size, 0, PHB_IO_BASE, PHB_IO_END,
> + area = __get_vm_area_caller(size, VM_IOREMAP, PHB_IO_BASE, PHB_IO_END,
> __builtin_return_address(0));
> if (!area)
> return NULL;
> --
> 2.34.1
>
Reviewed-by: Uladzislau Rezki (Sony) <[email protected]>
--
Uladzislau Rezki
On Fri, Jan 13, 2023 at 11:19:15AM +0800, Baoquan He wrote:
> In one vmap_block area, there could be three types of regions: region
> being used which is allocated through vb_alloc(), dirty region which
> is freed via vb_free() and free region. Among them, only used region
> has available data. While there's no way to track those used regions
> currently.
>
> Here, add bitmap field used_map into vmap_block, and set/clear it during
> allocation or freeing regions of vmap_block area.
>
> This is a preparatoin for later use.
>
> Signed-off-by: Baoquan He <[email protected]>
> ---
> mm/vmalloc.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 428e0bee5c9c..d6ff058ef4d0 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1922,6 +1922,7 @@ struct vmap_block {
> spinlock_t lock;
> struct vmap_area *va;
> unsigned long free, dirty;
> + DECLARE_BITMAP(used_map, VMAP_BBMAP_BITS);
> unsigned long dirty_min, dirty_max; /*< dirty range */
> struct list_head free_list;
> struct rcu_head rcu_head;
> @@ -1998,10 +1999,12 @@ static void *new_vmap_block(unsigned int order, gfp_t gfp_mask)
> vb->va = va;
> /* At least something should be left free */
> BUG_ON(VMAP_BBMAP_BITS <= (1UL << order));
> + bitmap_zero(vb->used_map, VMAP_BBMAP_BITS);
> vb->free = VMAP_BBMAP_BITS - (1UL << order);
> vb->dirty = 0;
> vb->dirty_min = VMAP_BBMAP_BITS;
> vb->dirty_max = 0;
> + bitmap_set(vb->used_map, 0, (1UL << order));
> INIT_LIST_HEAD(&vb->free_list);
>
> vb_idx = addr_to_vb_idx(va->va_start);
> @@ -2111,6 +2114,7 @@ static void *vb_alloc(unsigned long size, gfp_t gfp_mask)
> pages_off = VMAP_BBMAP_BITS - vb->free;
> vaddr = vmap_block_vaddr(vb->va->va_start, pages_off);
> vb->free -= 1UL << order;
> + bitmap_set(vb->used_map, pages_off, (1UL << order));
> if (vb->free == 0) {
> spin_lock(&vbq->lock);
> list_del_rcu(&vb->free_list);
> @@ -2144,6 +2148,9 @@ static void vb_free(unsigned long addr, unsigned long size)
> order = get_order(size);
> offset = (addr & (VMAP_BLOCK_SIZE - 1)) >> PAGE_SHIFT;
> vb = xa_load(&vmap_blocks, addr_to_vb_idx(addr));
> + spin_lock(&vb->lock);
> + bitmap_clear(vb->used_map, offset, (1UL << order));
> + spin_unlock(&vb->lock);
>
> vunmap_range_noflush(addr, addr + size);
>
> --
> 2.34.1
>
Reviewed-by: Lorenzo Stoakes <[email protected]>
On Fri, Jan 13, 2023 at 11:19:16AM +0800, Baoquan He wrote:
> Through vmalloc API, a virtual kernel area is reserved for physical
> address mapping. And vmap_area is used to track them, while vm_struct
> is allocated to associate with the vmap_area to store more information
> and passed out.
>
> However, area reserved via vm_map_ram() is an exception. It doesn't have
> vm_struct to associate with vmap_area. And we can't recognize the
> vmap_area with '->vm == NULL' as a vm_map_ram() area because the normal
> freeing path will set va->vm = NULL before unmapping, please see
> function remove_vm_area().
>
> Meanwhile, there are two kinds of handling for vm_map_ram area. One is
> the whole vmap_area being reserved and mapped at one time through
> vm_map_area() interface; the other is the whole vmap_area with
> VMAP_BLOCK_SIZE size being reserved, while mapped into split regions
> with smaller size via vb_alloc().
>
> To mark the area reserved through vm_map_ram(), add flags field into
> struct vmap_area. Bit 0 indicates this is vm_map_ram area created
> through vm_map_ram() interface, while bit 1 marks out the type of
> vm_map_ram area which makes use of vmap_block to manage split regions
> via vb_alloc/free().
>
> This is a preparatoin for later use.
>
> Signed-off-by: Baoquan He <[email protected]>
> ---
> include/linux/vmalloc.h | 1 +
> mm/vmalloc.c | 16 ++++++++++++----
> 2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 096d48aa3437..69250efa03d1 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -76,6 +76,7 @@ struct vmap_area {
> unsigned long subtree_max_size; /* in "free" tree */
> struct vm_struct *vm; /* in "busy" tree */
> };
> + unsigned long flags; /* mark type of vm_map_ram area */
> };
>
> /* archs that select HAVE_ARCH_HUGE_VMAP should override one or more of these */
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index d6ff058ef4d0..ab4825050b5c 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1589,7 +1589,8 @@ preload_this_cpu_lock(spinlock_t *lock, gfp_t gfp_mask, int node)
> static struct vmap_area *alloc_vmap_area(unsigned long size,
> unsigned long align,
> unsigned long vstart, unsigned long vend,
> - int node, gfp_t gfp_mask)
> + int node, gfp_t gfp_mask,
> + unsigned long va_flags)
> {
> struct vmap_area *va;
> unsigned long freed;
> @@ -1635,6 +1636,7 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
> va->va_start = addr;
> va->va_end = addr + size;
> va->vm = NULL;
> + va->flags = va_flags;
>
> spin_lock(&vmap_area_lock);
> insert_vmap_area(va, &vmap_area_root, &vmap_area_list);
> @@ -1913,6 +1915,10 @@ static struct vmap_area *find_unlink_vmap_area(unsigned long addr)
>
> #define VMAP_BLOCK_SIZE (VMAP_BBMAP_BITS * PAGE_SIZE)
>
> +#define VMAP_RAM 0x1 /* indicates vm_map_ram area*/
> +#define VMAP_BLOCK 0x2 /* mark out the vmap_block sub-type*/
> +#define VMAP_FLAGS_MASK 0x3
> +
> struct vmap_block_queue {
> spinlock_t lock;
> struct list_head free;
> @@ -1988,7 +1994,8 @@ static void *new_vmap_block(unsigned int order, gfp_t gfp_mask)
>
> va = alloc_vmap_area(VMAP_BLOCK_SIZE, VMAP_BLOCK_SIZE,
> VMALLOC_START, VMALLOC_END,
> - node, gfp_mask);
> + node, gfp_mask,
> + VMAP_RAM|VMAP_BLOCK);
> if (IS_ERR(va)) {
> kfree(vb);
> return ERR_CAST(va);
> @@ -2297,7 +2304,8 @@ void *vm_map_ram(struct page **pages, unsigned int count, int node)
> } else {
> struct vmap_area *va;
> va = alloc_vmap_area(size, PAGE_SIZE,
> - VMALLOC_START, VMALLOC_END, node, GFP_KERNEL);
> + VMALLOC_START, VMALLOC_END,
> + node, GFP_KERNEL, VMAP_RAM);
> if (IS_ERR(va))
> return NULL;
>
> @@ -2537,7 +2545,7 @@ static struct vm_struct *__get_vm_area_node(unsigned long size,
> if (!(flags & VM_NO_GUARD))
> size += PAGE_SIZE;
>
> - va = alloc_vmap_area(size, align, start, end, node, gfp_mask);
> + va = alloc_vmap_area(size, align, start, end, node, gfp_mask, 0);
> if (IS_ERR(va)) {
> kfree(area);
> return NULL;
> --
> 2.34.1
>
Reviewed-by: Uladzislau Rezki (Sony) <[email protected]>
--
Uladzislau Rezki
On Fri, Jan 13, 2023 at 11:19:16AM +0800, Baoquan He wrote:
> Through vmalloc API, a virtual kernel area is reserved for physical
> address mapping. And vmap_area is used to track them, while vm_struct
> is allocated to associate with the vmap_area to store more information
> and passed out.
>
> However, area reserved via vm_map_ram() is an exception. It doesn't have
> vm_struct to associate with vmap_area. And we can't recognize the
> vmap_area with '->vm == NULL' as a vm_map_ram() area because the normal
> freeing path will set va->vm = NULL before unmapping, please see
> function remove_vm_area().
>
> Meanwhile, there are two kinds of handling for vm_map_ram area. One is
> the whole vmap_area being reserved and mapped at one time through
> vm_map_area() interface; the other is the whole vmap_area with
> VMAP_BLOCK_SIZE size being reserved, while mapped into split regions
> with smaller size via vb_alloc().
>
> To mark the area reserved through vm_map_ram(), add flags field into
> struct vmap_area. Bit 0 indicates this is vm_map_ram area created
> through vm_map_ram() interface, while bit 1 marks out the type of
> vm_map_ram area which makes use of vmap_block to manage split regions
> via vb_alloc/free().
>
> This is a preparatoin for later use.
>
Small typo here :)
> Signed-off-by: Baoquan He <[email protected]>
> ---
> include/linux/vmalloc.h | 1 +
> mm/vmalloc.c | 16 ++++++++++++----
> 2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 096d48aa3437..69250efa03d1 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -76,6 +76,7 @@ struct vmap_area {
> unsigned long subtree_max_size; /* in "free" tree */
> struct vm_struct *vm; /* in "busy" tree */
> };
> + unsigned long flags; /* mark type of vm_map_ram area */
> };
>
> /* archs that select HAVE_ARCH_HUGE_VMAP should override one or more of these */
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index d6ff058ef4d0..ab4825050b5c 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1589,7 +1589,8 @@ preload_this_cpu_lock(spinlock_t *lock, gfp_t gfp_mask, int node)
> static struct vmap_area *alloc_vmap_area(unsigned long size,
> unsigned long align,
> unsigned long vstart, unsigned long vend,
> - int node, gfp_t gfp_mask)
> + int node, gfp_t gfp_mask,
> + unsigned long va_flags)
> {
> struct vmap_area *va;
> unsigned long freed;
> @@ -1635,6 +1636,7 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
> va->va_start = addr;
> va->va_end = addr + size;
> va->vm = NULL;
> + va->flags = va_flags;
>
> spin_lock(&vmap_area_lock);
> insert_vmap_area(va, &vmap_area_root, &vmap_area_list);
> @@ -1913,6 +1915,10 @@ static struct vmap_area *find_unlink_vmap_area(unsigned long addr)
>
> #define VMAP_BLOCK_SIZE (VMAP_BBMAP_BITS * PAGE_SIZE)
>
> +#define VMAP_RAM 0x1 /* indicates vm_map_ram area*/
> +#define VMAP_BLOCK 0x2 /* mark out the vmap_block sub-type*/
> +#define VMAP_FLAGS_MASK 0x3
> +
> struct vmap_block_queue {
> spinlock_t lock;
> struct list_head free;
> @@ -1988,7 +1994,8 @@ static void *new_vmap_block(unsigned int order, gfp_t gfp_mask)
>
> va = alloc_vmap_area(VMAP_BLOCK_SIZE, VMAP_BLOCK_SIZE,
> VMALLOC_START, VMALLOC_END,
> - node, gfp_mask);
> + node, gfp_mask,
> + VMAP_RAM|VMAP_BLOCK);
> if (IS_ERR(va)) {
> kfree(vb);
> return ERR_CAST(va);
> @@ -2297,7 +2304,8 @@ void *vm_map_ram(struct page **pages, unsigned int count, int node)
> } else {
> struct vmap_area *va;
> va = alloc_vmap_area(size, PAGE_SIZE,
> - VMALLOC_START, VMALLOC_END, node, GFP_KERNEL);
> + VMALLOC_START, VMALLOC_END,
> + node, GFP_KERNEL, VMAP_RAM);
> if (IS_ERR(va))
> return NULL;
>
> @@ -2537,7 +2545,7 @@ static struct vm_struct *__get_vm_area_node(unsigned long size,
> if (!(flags & VM_NO_GUARD))
> size += PAGE_SIZE;
>
> - va = alloc_vmap_area(size, align, start, end, node, gfp_mask);
> + va = alloc_vmap_area(size, align, start, end, node, gfp_mask, 0);
> if (IS_ERR(va)) {
> kfree(area);
> return NULL;
> --
> 2.34.1
>
Other than that typo,
Reviewed-by: Lorenzo Stoakes <[email protected]>
On Fri, Jan 13, 2023 at 11:19:20AM +0800, Baoquan He wrote:
> Currently, for vmalloc areas with flag VM_IOREMAP set, except of the
> specific alignment clamping in __get_vm_area_node(), they will be
> 1) Shown as ioremap in /proc/vmallocinfo;
> 2) Ignored by /proc/kcore reading via vread()
>
> So for the io mapping in ioremap_phb() of ppc, we should set VM_IOREMAP
> in flag to make it handled correctly as above.
>
> Signed-off-by: Baoquan He <[email protected]>
> ---
> arch/powerpc/kernel/pci_64.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index 0c7cfb9fab04..fd42059ae2a5 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -132,7 +132,7 @@ void __iomem *ioremap_phb(phys_addr_t paddr, unsigned long size)
> * address decoding but I'd rather not deal with those outside of the
> * reserved 64K legacy region.
> */
> - area = __get_vm_area_caller(size, 0, PHB_IO_BASE, PHB_IO_END,
> + area = __get_vm_area_caller(size, VM_IOREMAP, PHB_IO_BASE, PHB_IO_END,
> __builtin_return_address(0));
> if (!area)
> return NULL;
> --
> 2.34.1
>
Reviewed-by: Lorenzo Stoakes <[email protected]>
On 01/16/23 at 12:23pm, Lorenzo Stoakes wrote:
> On Fri, Jan 13, 2023 at 11:19:16AM +0800, Baoquan He wrote:
> > Through vmalloc API, a virtual kernel area is reserved for physical
> > address mapping. And vmap_area is used to track them, while vm_struct
> > is allocated to associate with the vmap_area to store more information
> > and passed out.
> >
> > However, area reserved via vm_map_ram() is an exception. It doesn't have
> > vm_struct to associate with vmap_area. And we can't recognize the
> > vmap_area with '->vm == NULL' as a vm_map_ram() area because the normal
> > freeing path will set va->vm = NULL before unmapping, please see
> > function remove_vm_area().
> >
> > Meanwhile, there are two kinds of handling for vm_map_ram area. One is
> > the whole vmap_area being reserved and mapped at one time through
> > vm_map_area() interface; the other is the whole vmap_area with
> > VMAP_BLOCK_SIZE size being reserved, while mapped into split regions
> > with smaller size via vb_alloc().
> >
> > To mark the area reserved through vm_map_ram(), add flags field into
> > struct vmap_area. Bit 0 indicates this is vm_map_ram area created
> > through vm_map_ram() interface, while bit 1 marks out the type of
> > vm_map_ram area which makes use of vmap_block to manage split regions
> > via vb_alloc/free().
> >
> > This is a preparatoin for later use.
> >
>
> Small typo here :)
Good catch, will fix it.
>
> > Signed-off-by: Baoquan He <[email protected]>
> > ---
> > include/linux/vmalloc.h | 1 +
> > mm/vmalloc.c | 16 ++++++++++++----
> > 2 files changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> > index 096d48aa3437..69250efa03d1 100644
> > --- a/include/linux/vmalloc.h
> > +++ b/include/linux/vmalloc.h
> > @@ -76,6 +76,7 @@ struct vmap_area {
> > unsigned long subtree_max_size; /* in "free" tree */
> > struct vm_struct *vm; /* in "busy" tree */
> > };
> > + unsigned long flags; /* mark type of vm_map_ram area */
> > };
> >
> > /* archs that select HAVE_ARCH_HUGE_VMAP should override one or more of these */
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index d6ff058ef4d0..ab4825050b5c 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -1589,7 +1589,8 @@ preload_this_cpu_lock(spinlock_t *lock, gfp_t gfp_mask, int node)
> > static struct vmap_area *alloc_vmap_area(unsigned long size,
> > unsigned long align,
> > unsigned long vstart, unsigned long vend,
> > - int node, gfp_t gfp_mask)
> > + int node, gfp_t gfp_mask,
> > + unsigned long va_flags)
> > {
> > struct vmap_area *va;
> > unsigned long freed;
> > @@ -1635,6 +1636,7 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
> > va->va_start = addr;
> > va->va_end = addr + size;
> > va->vm = NULL;
> > + va->flags = va_flags;
> >
> > spin_lock(&vmap_area_lock);
> > insert_vmap_area(va, &vmap_area_root, &vmap_area_list);
> > @@ -1913,6 +1915,10 @@ static struct vmap_area *find_unlink_vmap_area(unsigned long addr)
> >
> > #define VMAP_BLOCK_SIZE (VMAP_BBMAP_BITS * PAGE_SIZE)
> >
> > +#define VMAP_RAM 0x1 /* indicates vm_map_ram area*/
> > +#define VMAP_BLOCK 0x2 /* mark out the vmap_block sub-type*/
> > +#define VMAP_FLAGS_MASK 0x3
> > +
> > struct vmap_block_queue {
> > spinlock_t lock;
> > struct list_head free;
> > @@ -1988,7 +1994,8 @@ static void *new_vmap_block(unsigned int order, gfp_t gfp_mask)
> >
> > va = alloc_vmap_area(VMAP_BLOCK_SIZE, VMAP_BLOCK_SIZE,
> > VMALLOC_START, VMALLOC_END,
> > - node, gfp_mask);
> > + node, gfp_mask,
> > + VMAP_RAM|VMAP_BLOCK);
> > if (IS_ERR(va)) {
> > kfree(vb);
> > return ERR_CAST(va);
> > @@ -2297,7 +2304,8 @@ void *vm_map_ram(struct page **pages, unsigned int count, int node)
> > } else {
> > struct vmap_area *va;
> > va = alloc_vmap_area(size, PAGE_SIZE,
> > - VMALLOC_START, VMALLOC_END, node, GFP_KERNEL);
> > + VMALLOC_START, VMALLOC_END,
> > + node, GFP_KERNEL, VMAP_RAM);
> > if (IS_ERR(va))
> > return NULL;
> >
> > @@ -2537,7 +2545,7 @@ static struct vm_struct *__get_vm_area_node(unsigned long size,
> > if (!(flags & VM_NO_GUARD))
> > size += PAGE_SIZE;
> >
> > - va = alloc_vmap_area(size, align, start, end, node, gfp_mask);
> > + va = alloc_vmap_area(size, align, start, end, node, gfp_mask, 0);
> > if (IS_ERR(va)) {
> > kfree(area);
> > return NULL;
> > --
> > 2.34.1
> >
>
> Other than that typo,
>
> Reviewed-by: Lorenzo Stoakes <[email protected]>
>