On Fri, Sep 13, 2024 at 03:50:00PM -0700, Jeff Xu wrote: > Even though the number of lines is large in these patches, its main > intention is to test Pedro's in-place change (from can_modify_mm to > can_modify_vma). Before this patch, the test had a common pattern: > setup memory layout, seal the memory, perform a few mm-api steps, verify > return code (not zero). Because of the nature of out-of-loop, it is > sufficient to just verify the error code in a few cases. > With Pedro's in-loop change, the sealing check happens later in the > stack, thus there are more things and scenarios to verify. And there were > feedback to me during in-loop change that selftest should be extensive > enough to discover all regressions. Even though this viewpoint is subject > to debate. Since none would want to do it, I thought I would just do it. > So the Patch V3 1/5 is dedicated entirely to increasing the verification > for existing scenarios, this including checking return code code, vma-size, > etc after mm api return. > Patch V3 3/5 are for unmap(), during review of V2 of Pedro's in-loop > change, we discovered a bug in unmap(), and unmap() is not atomic. > This leads to 4/5(mmap), 5/5(mremap), which calls munmap(). > In addition, I add scenarios to cover cross-multiple-vma cases. > The high-level goal of mseal test are two folds: > 1> make sure sealing is working correctly under different scenarios, > i.e. sealed mapping are not modified. > 2> For unsealed memory, added mseal code doesn't regress on regular mm API. > The goal 2 is as important as 1, that is why tests usually are done in > two phases, one with sealing, the other without. That's vastly more detail than is in the changelogs for the actual patches (which are just a few lines each) or the cover letter of the series. I don't have the MM knowledge to assess the detail of what you're saying but I can't help but think that it'd help a lot with review if all this detail were part of the actual submission.
Attachment:
signature.asc
Description: PGP signature
