Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Liam Howlett <liam.howlett@oracle.com>
To:     Daniel Latypov <dlatypov@google.com>
CC:     SeongJae Park <sj@kernel.org>,
        "brendanhiggins@google.com" <brendanhiggins@google.com>,
        "kunit-dev@googlegroups.com" <kunit-dev@googlegroups.com>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        "maple-tree@lists.infradead.org" <maple-tree@lists.infradead.org>,
        "damon@lists.linux.dev" <damon@lists.linux.dev>,
        kernel test robot <lkp@intel.com>
Subject: Re: [PATCH v3 30/48] mm/damon: Stop using vma_mas_store() for maple
 tree store
Thread-Topic: [PATCH v3 30/48] mm/damon: Stop using vma_mas_store() for maple
 tree store
Thread-Index: AQHZKhwz3Un6d8d2IEit8wQQlG94y66i+rGAgAA05wCAAAeQAA==
Date:   Tue, 17 Jan 2023 22:47:36 +0000
Message-ID: <20230117224734.c4lo4spezufwsims@revolver>
References: <20230117023335.1690727-31-Liam.Howlett@oracle.com>
 <20230117191109.116438-1-sj@kernel.org>
 <CAGS_qxru93E+=6nD31=8UGS=7ADnb3VG3hv=hfh-R1zVcqTJ3w@mail.gmail.com>
In-Reply-To: <CAGS_qxru93E+=6nD31=8UGS=7ADnb3VG3hv=hfh-R1zVcqTJ3w@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?ZzjlvUHTNeuI8GSzID5kaP6XDb/Ron6ejVaKZ7EhrWPDro/MGpyq0b+ch8ze?=
 =?us-ascii?Q?d42ldG3AsW3bk0fTYtJTnmtwOYvR/ZeVi3R4JubRhE2x6rM2eiGH3Y67mZmL?=
 =?us-ascii?Q?0TQoSahMcngkPbQGG0gdZT9obN/bMApqCC44IAaDvbL0mlZ+U5fjj+kMir4+?=
 =?us-ascii?Q?iQz/3YIgL/ElUUbZVVbFcwJ1DtxaWjFRdbWKsckHB1SalTchsvuopNaPb6wN?=
 =?us-ascii?Q?tJOf4yWESXexX75NGQkRrRbB6Y4c4nMLZjbIgcp7P4vWSHLWv8xvE/eZ0n3Q?=
 =?us-ascii?Q?GvjqTEipI05S3xkQlJ/GVsU/kLj1sPzLhAId6gftVtyQlKU9s2inRSgBmESU?=
 =?us-ascii?Q?iEkIY/duOwq9vl5oqpfyfJVxa244q+rrLfJtO+HGatqApOdImk/gxoZy3ZKk?=
 =?us-ascii?Q?kLEPT20bhwVXaPZsgzkn9vjqngNZ2iozHhkE1JUmE4EAcVtA9JBLwDJbPR+Z?=
 =?us-ascii?Q?Q3jJHTFZidZdfB4f2vjf/BAaFSUuYeZIcF3+EXdWxuHQNaGteZjcamTae/64?=
 =?us-ascii?Q?9wk4AL3pywHKbzLZO8lVSgdyr2RLJhgRmkF7x6mPqacLGyVXHEzsZg+Sd4Gv?=
 =?us-ascii?Q?nfiTtuIUkU/OG7LE7XaXGQANFYG9P+dYj1yBdz7lbrC+Mg08fU1dyfO1NM0R?=
 =?us-ascii?Q?xSCnSMzv5Jk9XEipbNgOfkdnJtf7wytHYQ5ag0II4EWzamN2JCG14n2eoJ3W?=
 =?us-ascii?Q?/VUCaz9No8PWiBak641nGHLdSFUW70jT44puRtz62uDFdjyLZoT/4AWAdoUs?=
 =?us-ascii?Q?GmRWvyIy0dv0B7UjmM9TUf64WSmsL63EHafbzLrn+RypwTwdpZqggyqS3XEY?=
 =?us-ascii?Q?AtPtAfM1DhninY2GGJArtMQ+0rHEfWVyiUo/NAXLFImyB8kBtWczx9x84Y9m?=
 =?us-ascii?Q?PvaFpbqkEBxRhLdxM/A0otXRJkTDtHEwXege40Uro9P44s0+RQBIRu6R9P5m?=
 =?us-ascii?Q?yWPb+nqWzau807g9Gy273UVBFIL8+/lBLgk7zXQb03T5kYE9XQMTC4NUWeiM?=
 =?us-ascii?Q?DwhoSoeckc2NTJskDUU+w0Q+D+1sR1SPO+w1ZRNTAkP/h5lXorv7S4b60z4N?=
 =?us-ascii?Q?mLZnTlibp0YEi4icoHZmQ5gBFeY9JJAgQ7dJ1LrNdu0uReWL9sGKKqO+eemP?=
 =?us-ascii?Q?T/H+Ug4CFMIeAR60WzlFeev5TXsf5GaP9E0p+gZMxTySYNnq3J23JdX+2JuS?=
 =?us-ascii?Q?0Iqocy2zATtMr0KEKNMjF/szHrqXVqblhjWm7APjw5nIvMA4TOkAB3ZXZRgc?=
 =?us-ascii?Q?WzPe8dj+XKTVbDveOiKpLhaYIqT4aqFLInIc4/PErTNAUNEIwA1+cR1e7ZEl?=
 =?us-ascii?Q?Fr2Ylgw6qNslQs3/PTxbl8U8Mu0UGD50ZbaDnhnJ11kZm7Lo4Oq00j/5TLdP?=
 =?us-ascii?Q?Prla2YkJ9Zip+TU7RhaOcrSWnOkFM36TONBzCSALWHyMviF1AfAkQPbrLx+H?=
 =?us-ascii?Q?+M2/sLI01dytbZ5JCytYhmvS19tn1o0AXfX0IxkJXCNHsbBR+HBxzxZ4OkTS?=
 =?us-ascii?Q?t2ENMXJP8VTW7CPHMavpBDflgVg3DZfvb4U10txmgRCRZXR7VgPMsJZL4Nid?=
 =?us-ascii?Q?LB+bp27znygYizIx8HtvNc9Wz+mRx/lDaaBXJD7fi4VWNih7DeDNZexSJ5HZ?=
 =?us-ascii?Q?Vg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <B54EEAB33643BF48A0607275433C3B82@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?EN9PNM0L2+mZd6AQAkrbVPuCSVa25PLgBSKcF0RCErYI92C0+KWslDnevryz?=
 =?us-ascii?Q?I9DJoOPaU5O34X0hcw8nJ44yoA5/xJXXQ6s1eBLi68eu0MrV6VzrFVquNAWv?=
 =?us-ascii?Q?t+O7BojhIDQV4vewK6jtQ9IK7vUjGbTALy9UhXwSL/H9XkyNZD5jX+r+xMpD?=
 =?us-ascii?Q?p7rFa4apz8YguBeXZFeiWYdCHUCSaHYdlPOjPKlopdldgUQkRvD7kSV/Pg8o?=
 =?us-ascii?Q?vGupPWLFke0DlYhxFHgwgTsjTPU/ALDyZG4JB7K8sZlUVAfwKXKnnGhJOJIa?=
 =?us-ascii?Q?vGFYM92SnqzDZIr627KawF/b7//Dp3RhdAAEIFjVm2k7qScybwUYmeOlt4II?=
 =?us-ascii?Q?u06MGtA66gHRtjVHpUGOn5iDyLEMW+S33tIMtDs4MlmU6dWR7pPXKAMI7eCx?=
 =?us-ascii?Q?aq5+TlC+nhc/6WLbFh8LeeMRCQ6mrosWpcoKRClj8aHGYC3F7tFM8u347dgx?=
 =?us-ascii?Q?gL93uBycPeYZuKZlXQJ17eN6wbRvwSaR4u2YNhB68fi31hZSr7r3WmFOeGzp?=
 =?us-ascii?Q?H2p3/oBUcY81JISyw6Yvftd1ovMitLiP7QJ3DG/ONF7imTMi+zOkYZQMWxSJ?=
 =?us-ascii?Q?Vw92bVwY/Oj2Tk2gCX5zA6EFYfMKjKgrN8ANVUcJMjt96Cob2E9/VrlKk5E8?=
 =?us-ascii?Q?V4VhiOfOzGkv1aKCxiDcVwi58wdLX2pB3sdXUSQTQsdJgdYNiqi/glso3GnY?=
 =?us-ascii?Q?oaWsopfo7ad+75M59wo5YCq8ZqZVLRf5Jpx3KAfiMZSlJIceUmUlDbceR6k5?=
 =?us-ascii?Q?dtuOhcoU70AO2hyc/syJdqxWwUMlV1tP4IZQSyIykicKEsaSpuVwes+whdrg?=
 =?us-ascii?Q?FM+HblScj6HHwvPdHrEIt6zp9uGJadSVTxM8ebYwH5rnqTIKLYgo7Qq+2+09?=
 =?us-ascii?Q?fUXUM94tidOP78YEam+y+U0iD9r5fqSI0BHA7pNnu0f+o+wFlHkh8P4mcoWm?=
 =?us-ascii?Q?iU5f9UVmF92271h5+6IJW9sTmh1nY/WiylIc8GcNOHgs+2xy92c9Qdn4kS6u?=
 =?us-ascii?Q?s6uGflNjocW5jZrA44eQs5DGkAcxATxHaHysLZ5Br2iTC+ZIhFvDyKKpR842?=
 =?us-ascii?Q?wyXpR8kN?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4b78d82d-0928-4062-bdbc-08daf8dcd493
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Jan 2023 22:47:36.8979
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ZmwzmVk84QtZ+BCv3ZRPJTLm/zDjV4hJ2yvizn2H4laVekW4/0ZMbaGd6B1ZkUFtA0JCXWIgeIkBQADylQ8iSw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR10MB7268
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.923,Hydra:6.0.562,FMLib:17.11.122.1
 definitions=2023-01-17_10,2023-01-17_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 mlxlogscore=999
 malwarescore=0 bulkscore=0 suspectscore=0 mlxscore=0 phishscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2212070000 definitions=main-2301170182
X-Proofpoint-GUID: CvTB7N1rgOanU0YILr9s9iTXxHb9jPo4
X-Proofpoint-ORIG-GUID: CvTB7N1rgOanU0YILr9s9iTXxHb9jPo4
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

* Daniel Latypov <dlatypov@google.com> [230117 17:20]:
> On Tue, Jan 17, 2023 at 11:11 AM SeongJae Park <sj@kernel.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 ...
> >
> > On Tue, 17 Jan 2023 02:34:19 +0000 Liam Howlett <liam.howlett@oracle.co=
m> wrote:
> >
> > > From: "Liam R. Howlett" <Liam.Howlett@oracle.com>
> > >
> > > Prepare for the removal of the vma_mas_store() function by open codin=
g
> > > the maple tree store in this test code.  Set the range of the maple
> > > state and call the store function directly.
> > >
> > > Cc: SeongJae Park <sj@kernel.org>
> > > Cc: damon@lists.linux.dev
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
> > > ---
> > >  mm/damon/vaddr-test.h | 19 +++++++++++++------
> > >  1 file changed, 13 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/mm/damon/vaddr-test.h b/mm/damon/vaddr-test.h
> > > index bce37c487540..6098933d3272 100644
> > > --- a/mm/damon/vaddr-test.h
> > > +++ b/mm/damon/vaddr-test.h
> > > @@ -14,19 +14,26 @@
> > >
> > >  #include <kunit/test.h>
> > >
> > > -static void __link_vmas(struct maple_tree *mt, struct vm_area_struct=
 *vmas,
> > > +static int __link_vmas(struct maple_tree *mt, struct vm_area_struct =
*vmas,
> > >                       ssize_t nr_vmas)
> > >  {
> > > -     int i;
> > > +     int i, ret =3D -ENOMEM;
> > >       MA_STATE(mas, mt, 0, 0);
> > >
> > >       if (!nr_vmas)
> > > -             return;
> > > +             return -ENOENT;
>=20
> We could pass in the `test` object here and give more detailed info, e.g.
>   (if !nr_vmas)
>      kunit_skip(test, "...");
>=20
> And below could be
>=20
> bool stored_all =3D false; // instead of ret
> ...
> for (...) {
>=20
> }
> stored_all =3D true;
>=20
> failed:
>   mas_unlock(&mas);
>   if (!stored_all) kunit_skip(test, "failed to...");
>=20
> > >
> > >       mas_lock(&mas);
> > > -     for (i =3D 0; i < nr_vmas; i++)
> > > -             vma_mas_store(&vmas[i], &mas);
> > > +     for (i =3D 0; i < nr_vmas; i++) {
> > > +             mas_set_range(&mas, vmas[i].vm_start, vmas[i].vm_end - =
1);
> > > +             if (mas_store_gfp(&mas, &vmas[i], GFP_KERNEL))
> > > +                     goto failed;
> > > +     }
> > > +     ret =3D 0;
> > > +
> > > +failed:
> > >       mas_unlock(&mas);
> > > +     return ret;
> > >  }
> > >
> > >  /*
> > > @@ -71,7 +78,7 @@ static void damon_test_three_regions_in_vmas(struct=
 kunit *test)
> > >       };
> > >
> > >       mt_init_flags(&mm.mm_mt, MM_MT_FLAGS);
> > > -     __link_vmas(&mm.mm_mt, vmas, ARRAY_SIZE(vmas));
> > > +     KUNIT_EXPECT_EQ(test, __link_vmas(&mm.mm_mt, vmas, ARRAY_SIZE(v=
mas)), 0);
> >
> > In case of the __link_vmas() failure, I think we should skip this test =
using
> > 'kunit_skip()', rather marking this test failed.
>=20
> 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.