* SeongJae Park <sj@xxxxxxxxxx> [230118 21:00]: > Hello Daniel and Liam, > > > Sorry for late reply. > > On Tue, 17 Jan 2023 22:47:36 +0000 Liam Howlett <liam.howlett@xxxxxxxxxx> wrote: > > > * Daniel Latypov <dlatypov@xxxxxxxxxx> [230117 17:20]: > > > On Tue, Jan 17, 2023 at 11:11 AM SeongJae Park <sj@xxxxxxxxxx> wrote: > > > > > > > > Cc-ing kunit people. > > > > > > > > Hi Liam, > > > > > > > > > > > > Could we put touching file name on the summary? > > > > E.g., mm/damon/vaddr-test: Stop using ... Sure thing. ... > > > > > > > > In case of the __link_vmas() failure, I think we should skip this test using > > > > 'kunit_skip()', rather marking this test failed. > > > > > > As noted above, I'd suggest we also pass in the `test` object to > > > __link_vmas() and call kunit_skip() from there. > > > > My thoughts were if we are testing adding nothing to the list, then > > there is probably a problem with the test and so that should be > > highlighted with a failure. > > > > I really don't mind either way. > > I didn't wrote '__link_vmas()' to test vma manipulation functions it internally > uses, but just to offload test setup for 'damon_test_three_regions_in_vmas()'. > I agree that the detailed failure reason could be helpful for better > understanding as the function can now fail from 'mas_store_gfp()'s memory > allocation failure. > > That said, I think we can get the detail from the return value of > '__link_vmas()'. I'm further worrying if passing 'test' object to the function > makes people think the function itself is for testing something inside it. > > Also, I don't think the function returning non-error for zero value 'nr_vmas' > as a problem but just expected behavior, as previously commented[1]. > > So I'd prefer doing kunit_skip() here. Sounds good to me. Thanks for clarifying. I'll make the necessary changes for v4 of the patches. > > If I'm missing something or wrong, please let me know. > > [1] https://lore.kernel.org/damon/20230117191614.116521-1-sj@xxxxxxxxxx/ > >
