| Message ID | 20230201091339.61761-1-bhe@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mm/vmalloc.c: allow vread() to read out vm_map_ram areas | expand |
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.
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(-)