Comment 11 for bug 2038249

Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote :

Hi Chengen,

Thanks for the great work for these SRUs.

I'd like to suggest an update/improvement to the Test Plan section.
Even though crash is so broken that it can't even open a file,
once it starts working at all, it would be important to check
that it is working _correctly_.

So, please, could you add verifications for basic correctness
of the commands being addressed per patch (for GA and HWE)?
e.g., kmem -s|-S, module memory layout, etc.

And I had questions on 2 patches:

Patch 3:
---

 31 + * commit e36ce448a08d removed kmem_cache.freelist_cache in 6.1,
 32 + * so use freelist_size instead.
 33 */
 34 - if (MEMBER_EXISTS("kmem_cache", "freelist_cache")) {
 35 + if (MEMBER_EXISTS("kmem_cache", "freelist_size")) {

This is an inconditional change before/after 6.1, which thus could impact Jammy, as it ships both 5.15 and 6.2 kernels.

However, it seems the (new value) attribute 'freelist_size' already exists, so it should be fine in Jammy _if_ 5.15 has it too.

Could you please confirm?

Patch 5:
---

Some of this backport's context update is because this patch is not included,
and it would also fit the 6.2 criteria (changes in 6.1). It seems only code
adds though, not sure how it changes any behavior (or more patches needed).

Can you clarify if it's not really needed for 6.5?
(I haven't followed the maple tree closely, but the commit message suggests it's important.)

If it's not needed _right_ now, i.e., if this SRU is priority, and crash
at least _works_ (which is a good improvement), I think it would be fine
to add it later.

 commit 872cad2d63b3a07f65323fe80a7abb29ea276b44
 Author: Tao Liu <email address hidden>
 Date: Tue Jan 10 14:56:27 2023 +0800

     Port the maple tree data structures and functions

     There have been two ways to iterate vm_area_struct until Linux 6.0:
      1) by rbtree, aka vma.vm_rb;
      2) by linked list, aka vma.vm_{next,prev}.
     However with the maple tree patches[1][2] in Linux 6.1, vm_rb and
     vm_{next,prev} are removed from vm_area_struct. The vm_area_dump()
     in crash mainly uses the linked list for vma iteration, which will
     not work for this case. So the maple tree iteration needs to be
     ported to crash.

This patch 5 is also big, adding a lot, anyway, but it goes to gdb-10.2.patch,
which only changed context lines for the backport, indeed.

A pure-code review against upstream seems a big effort (~2000 lines, I managed
up to ~1000, and it seems to match upstream).

I guess this patch will be reasonably verified if the crash commands to show
module memory layout on kernel 6.4+ (6.5 in our case) run correctly.