Re: [PATCH v3 30/48] mm/damon: Stop using vma_mas_store() for maple tree store

[Liam R. Howlett]

* 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/
> 
>