linux-kernel - Re: [PATCH v3 30/48] mm/damon: Stop using vma_mas

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230119185503.wapuhphnjw2sn7wt@revolver>
Date:   Thu, 19 Jan 2023 13:55:03 -0500
From:   "Liam R. Howlett" <Liam.Howlett@...cle.com>
To:     SeongJae Park <sj@...nel.org>
Cc:     Daniel Latypov <dlatypov@...gle.com>,
        "brendanhiggins@...gle.com" <brendanhiggins@...gle.com>,
        "kunit-dev@...glegroups.com" <kunit-dev@...glegroups.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "maple-tree@...ts.infradead.org" <maple-tree@...ts.infradead.org>,
        "damon@...ts.linux.dev" <damon@...ts.linux.dev>,
        kernel test robot <lkp@...el.com>
Subject: Re: [PATCH v3 30/48] mm/damon: Stop using vma_mas_store() for maple
 tree store

* SeongJae Park <sj@...nel.org> [230118 21:00]:
> Hello Daniel and Liam,
> 
> 
> Sorry for late reply.
> 
> On Tue, 17 Jan 2023 22:47:36 +0000 Liam Howlett <liam.howlett@...cle.com> wrote:
> 
> > * Daniel Latypov <dlatypov@...gle.com> [230117 17:20]:
> > > On Tue, Jan 17, 2023 at 11:11 AM SeongJae Park <sj@...nel.org> 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@kernel.org/
> 
>