Hi Mark On Wed, Sep 18, 2024 at 6:18 AM Mark Brown <broonie@xxxxxxxxxx> wrote: > > 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. Agreed, will update and give more detail in the next version of the patch. Thanks -Jeff
