Re: [PATCH v3 01/12] mm/debug_vm_pgtable: Introduce struct pgtable_debug_args

From: Gavin Shan
Date: Wed Jul 21 2021 - 08:04:03 EST


On 7/21/21 8:59 PM, Anshuman Khandual wrote:
On 7/21/21 4:09 PM, Anshuman Khandual wrote:
On 7/21/21 3:50 PM, Gavin Shan wrote:
On 7/21/21 3:44 PM, Anshuman Khandual wrote:
On 7/19/21 6:36 PM, Gavin Shan wrote:
In debug_vm_pgtable(), there are many local variables introduced to
track the needed information and they are passed to the functions for
various test cases. It'd better to introduce a struct as place holder
for these information. With it, what the functions for various test
cases need is the struct, to simplify the code. It also makes code
easier to be maintained.

Besides, set_xxx_at() could access the data on the corresponding pages
in the page table modifying tests. So the accessed pages in the tests
should have been allocated from buddy. Otherwise, we're accessing pages
that aren't owned by us. This causes issues like page flag corruption.

This introduces "struct pgtable_debug_args". The struct is initialized
and destroyed, but the information in the struct isn't used yet. They
will be used in subsequent patches.

Signed-off-by: Gavin Shan <gshan@xxxxxxxxxx>
---
  mm/debug_vm_pgtable.c | 197 +++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 196 insertions(+), 1 deletion(-)

I saw you've finished the review on PATCH[v3 01/12] and PATCH[v3 02/12].
I will wait to integrate your comments to v4 until you finish the review
on all patches in v3 series
Yes, please do wait for the complete review and test before going for V4.
Also please add the following emails on copy next time, so that we might
have some more reviews here. Thank you.

+ Christophe Leroy <christophe.leroy@xxxxxxxxxx>
+ Gerald Schaefer <gerald.schaefer@xxxxxxxxxx>

This one instead.

Gerald Schaefer <gerald.schaefer@xxxxxxxxxxxxx>


Sure, It's always nice to have more reviews from the experts. I will
include them when I post v4. Thanks again for your time to review :)

Thanks,
Gavin