[v4,0/7] mm/vmalloc.c: allow vread() to read out vm_map_ram areas

  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/87ilk6gos2.fsf@oracle.com/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 commands to
access kcore file to do statistics:

	makedumpfile --mem-usage /proc/kcore

Changelog
***
v3->v4:
- Fix typo in pach 2 catched by Lorenzo.
- Add WARN_ON(flags == VMAP_BLOCK) in vread() to address Dan's concern
  that VMAP_BLOCK could be set alone in vmap->flags.
- Add checking on the returned value from xa_load() in vmap_ram_vread(),
  Uladzislau commented on the risk of this place.
- Fix a bug in s_show() which is changed in patch 5. The old change will
  cause 'va->vm is NULL but the VMAP_RAM flag is not set' case wrongly
  handled. Please see below link for details.
  - https://lore.kernel.org/all/Y8aAmuUY9OxrYlLm@kili/T/#u
- Add Uladzislau and Lorenzo's Reviewed-by.

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                 | 126 ++++++++++++++++++++++++++++++-----
 4 files changed, 111 insertions(+), 20 deletions(-)
  

Baoquan He <bhe@redhat.com> writes:
> 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/87ilk6gos2.fsf@oracle.com/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 commands to
> access kcore file to do statistics:
>
> 	makedumpfile --mem-usage /proc/kcore

Hi Baoquan,

Sorry I haven't commented with testing info or review on each revision:
I'm not really familiar with the details necessary for review. However,
it looks like this is getting close to ready, so I did another test:

[opc@stepbren-ol8-1 drgn_vmalloc_test]$ sudo insmod drgn_vmalloc_test.ko
[opc@stepbren-ol8-1 drgn_vmalloc_test]$ sudo dmesg | tail -n 5
[   20.763310] missing module BTF, cannot register kfuncs
[   20.840200] missing module BTF, cannot register kfuncs
[   91.475814] drgn_vmalloc_test: loading out-of-tree module taints kernel.
[   91.479913] drgn_vmalloc_test: module verification failed: signature and/or required key missing - tainting kernel
[   91.484926] drgn_vmalloc_test: 0xffffa51ac2d00000
[opc@stepbren-ol8-1 drgn_vmalloc_test]$ sudo drgn
drgn 0.0.22 (using Python 3.6.8, elfutils 0.186, with libkdumpfile)
For help, type help(drgn).
>>> import drgn
>>> from drgn import NULL, Object, cast, container_of, execscript, offsetof, reinterpret, sizeof
>>> from drgn.helpers.common import *
>>> from drgn.helpers.linux import *
warning: could not get debugging information for:
drgn_vmalloc_test (could not find module in depmod)
>>> prog.read(0xffffa51ac2d00000, 64)
b'\x00\x00\x00\x00\x01\x00\x00\x00\x02\x00\x00\x00\x03\x00\x00\x00\x04\x00\x00\x00\x05\x00\x00\x00\x06\x00\x00\x00\x07\x00\x00\x00\x08\x00\x00\x00\t\x00\x00\x00\n\x00\x00\x00\x0b\x00\x00\x00\x0c\x00\x00\x00\r\x00\x00\x00\x0e\x00\x00\x00\x0f\x00\x00\x00'
>>>

So this definitely still resolves the originally reported issue. Feel
free to add, if you want:

Tested-by: Stephen Brennan <stephen.s.brennan@oracle.com>

Thanks for all the work here,
Stephen

>
> Changelog
> ***
> v3->v4:
> - Fix typo in pach 2 catched by Lorenzo.
> - Add WARN_ON(flags == VMAP_BLOCK) in vread() to address Dan's concern
>   that VMAP_BLOCK could be set alone in vmap->flags.
> - Add checking on the returned value from xa_load() in vmap_ram_vread(),
>   Uladzislau commented on the risk of this place.
> - Fix a bug in s_show() which is changed in patch 5. The old change will
>   cause 'va->vm is NULL but the VMAP_RAM flag is not set' case wrongly
>   handled. Please see below link for details.
>   - https://lore.kernel.org/all/Y8aAmuUY9OxrYlLm@kili/T/#u
> - Add Uladzislau and Lorenzo's Reviewed-by.
>
> 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                 | 126 ++++++++++++++++++++++++++++++-----
>  4 files changed, 111 insertions(+), 20 deletions(-)
>
> -- 
> 2.34.1
  

On 02/02/23 at 09:47am, Stephen Brennan wrote:
......snip...
> > Testing
> > ***
> > Only did the basic testing on kvm guest, and run below commands to
> > access kcore file to do statistics:
> >
> > 	makedumpfile --mem-usage /proc/kcore
> 
> Hi Baoquan,
> 
> Sorry I haven't commented with testing info or review on each revision:
> I'm not really familiar with the details necessary for review. However,
> it looks like this is getting close to ready, so I did another test:

That's OK, and your testing is very helpful because I don't know how to
create vm_map_ram() area to test the patches, just did basic testing.

> 
> [opc@stepbren-ol8-1 drgn_vmalloc_test]$ sudo insmod drgn_vmalloc_test.ko
> [opc@stepbren-ol8-1 drgn_vmalloc_test]$ sudo dmesg | tail -n 5
> [   20.763310] missing module BTF, cannot register kfuncs
> [   20.840200] missing module BTF, cannot register kfuncs
> [   91.475814] drgn_vmalloc_test: loading out-of-tree module taints kernel.
> [   91.479913] drgn_vmalloc_test: module verification failed: signature and/or required key missing - tainting kernel
> [   91.484926] drgn_vmalloc_test: 0xffffa51ac2d00000
> [opc@stepbren-ol8-1 drgn_vmalloc_test]$ sudo drgn
> drgn 0.0.22 (using Python 3.6.8, elfutils 0.186, with libkdumpfile)
> For help, type help(drgn).
> >>> import drgn
> >>> from drgn import NULL, Object, cast, container_of, execscript, offsetof, reinterpret, sizeof
> >>> from drgn.helpers.common import *
> >>> from drgn.helpers.linux import *
> warning: could not get debugging information for:
> drgn_vmalloc_test (could not find module in depmod)
> >>> prog.read(0xffffa51ac2d00000, 64)
> b'\x00\x00\x00\x00\x01\x00\x00\x00\x02\x00\x00\x00\x03\x00\x00\x00\x04\x00\x00\x00\x05\x00\x00\x00\x06\x00\x00\x00\x07\x00\x00\x00\x08\x00\x00\x00\t\x00\x00\x00\n\x00\x00\x00\x0b\x00\x00\x00\x0c\x00\x00\x00\r\x00\x00\x00\x0e\x00\x00\x00\x0f\x00\x00\x00'
> >>>
> 
> So this definitely still resolves the originally reported issue. Feel
> free to add, if you want:
> 
> Tested-by: Stephen Brennan <stephen.s.brennan@oracle.com>

I noticed Andrew had picked this v4 into his mm tree, maybe Andrew can
help add this Tested-by tag.
  

On Sat, 4 Feb 2023 12:12:08 +0800 Baoquan He <bhe@redhat.com> wrote:

> > So this definitely still resolves the originally reported issue. Feel
> > free to add, if you want:
> > 
> > Tested-by: Stephen Brennan <stephen.s.brennan@oracle.com>
> 
> I noticed Andrew had picked this v4 into his mm tree, maybe Andrew can
> help add this Tested-by tag.

I dropped this series and am now unable to locate a fix which addressed
the issue which Stephen hit.

Please send a v5?
  

On 02/04/23 at 03:13pm, Andrew Morton wrote:
> On Sat, 4 Feb 2023 12:12:08 +0800 Baoquan He <bhe@redhat.com> wrote:
> 
> > > So this definitely still resolves the originally reported issue. Feel
> > > free to add, if you want:
> > > 
> > > Tested-by: Stephen Brennan <stephen.s.brennan@oracle.com>
> > 
> > I noticed Andrew had picked this v4 into his mm tree, maybe Andrew can
> > help add this Tested-by tag.
> 
> I dropped this series and am now unable to locate a fix which addressed
> the issue which Stephen hit.

Stephen wanted to read out vm_map_ram areas, that can't be done w/o this
patchset. The patch 3 solves his problem.

> 
> Please send a v5?

I add Stephen's Reported-by and Tested-by in patch 3 and send v5.