* [PATCH v2 26/44] mm/damon: Stop using vma_mas_store() for maple tree store [not found] <20230105191517.3099082-1-Liam.Howlett@oracle.com> @ 2023-01-05 19:16 ` Liam Howlett 2023-01-05 19:32 ` SeongJae Park 0 siblings, 1 reply; 4+ messages in thread From: Liam Howlett @ 2023-01-05 19:16 UTC (permalink / raw) To: maple-tree@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton Cc: Liam Howlett, SeongJae Park, damon@lists.linux.dev, kernel test robot From: "Liam R. Howlett" <Liam.Howlett@oracle.com> Prepare for the removal of the vma_mas_store() function by open coding 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 | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mm/damon/vaddr-test.h b/mm/damon/vaddr-test.h index bce37c487540..41532f7355d0 100644 --- a/mm/damon/vaddr-test.h +++ b/mm/damon/vaddr-test.h @@ -24,8 +24,10 @@ static void __link_vmas(struct maple_tree *mt, struct vm_area_struct *vmas, return; mas_lock(&mas); - for (i = 0; i < nr_vmas; i++) - vma_mas_store(&vmas[i], &mas); + for (i = 0; i < nr_vmas; i++) { + mas_set_range(&mas, vmas[i].vm_start, vmas[i].vm_end - 1); + mas_store_gfp(&mas, &vmas[i], GFP_KERNEL); + } mas_unlock(&mas); } -- 2.35.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 26/44] mm/damon: Stop using vma_mas_store() for maple tree store 2023-01-05 19:16 ` [PATCH v2 26/44] mm/damon: Stop using vma_mas_store() for maple tree store Liam Howlett @ 2023-01-05 19:32 ` SeongJae Park 2023-01-05 19:52 ` Liam Howlett 0 siblings, 1 reply; 4+ messages in thread From: SeongJae Park @ 2023-01-05 19:32 UTC (permalink / raw) To: Liam Howlett Cc: maple-tree@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton, SeongJae Park, damon@lists.linux.dev, kernel test robot Hi Liam, On Thu, 5 Jan 2023 19:16:00 +0000 Liam Howlett <liam.howlett@oracle.com> wrote: > From: "Liam R. Howlett" <Liam.Howlett@oracle.com> > > Prepare for the removal of the vma_mas_store() function by open coding > the maple tree store in this test code. But seems this series is not really removing 'vma_mas_store()'. Wouldn't it better to do the preparation and removal together in a same patch series? > 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 | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/mm/damon/vaddr-test.h b/mm/damon/vaddr-test.h > index bce37c487540..41532f7355d0 100644 > --- a/mm/damon/vaddr-test.h > +++ b/mm/damon/vaddr-test.h > @@ -24,8 +24,10 @@ static void __link_vmas(struct maple_tree *mt, struct vm_area_struct *vmas, > return; > > mas_lock(&mas); > - for (i = 0; i < nr_vmas; i++) > - vma_mas_store(&vmas[i], &mas); > + for (i = 0; i < nr_vmas; i++) { > + mas_set_range(&mas, vmas[i].vm_start, vmas[i].vm_end - 1); > + mas_store_gfp(&mas, &vmas[i], GFP_KERNEL); > + } On the latest mm-unstable, vma_mas_store() uses mas_store_prealloc() instead of mas_store_gfp(). Seems the difference would make no problem to this test code in most cases, but could I ask the reason for this change? Also, should we check the return value of mas_store_gfp()? > mas_unlock(&mas); > } > > -- > 2.35.1 Thanks, SJ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 26/44] mm/damon: Stop using vma_mas_store() for maple tree store 2023-01-05 19:32 ` SeongJae Park @ 2023-01-05 19:52 ` Liam Howlett 2023-01-05 20:16 ` SeongJae Park 0 siblings, 1 reply; 4+ messages in thread From: Liam Howlett @ 2023-01-05 19:52 UTC (permalink / raw) To: SeongJae Park Cc: maple-tree@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton, damon@lists.linux.dev, kernel test robot * SeongJae Park <sj@kernel.org> [230105 14:33]: > Hi Liam, > > On Thu, 5 Jan 2023 19:16:00 +0000 Liam Howlett <liam.howlett@oracle.com> wrote: > > > From: "Liam R. Howlett" <Liam.Howlett@oracle.com> > > > > Prepare for the removal of the vma_mas_store() function by open coding > > the maple tree store in this test code. > > But seems this series is not really removing 'vma_mas_store()'. Wouldn't it > better to do the preparation and removal together in a same patch series? It does from the all code but the nommu side. The definition is dropped from the header and c file in "mmap: Convert __vma_adjust() to use vma iterator" [1]. > > > 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 | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/mm/damon/vaddr-test.h b/mm/damon/vaddr-test.h > > index bce37c487540..41532f7355d0 100644 > > --- a/mm/damon/vaddr-test.h > > +++ b/mm/damon/vaddr-test.h > > @@ -24,8 +24,10 @@ static void __link_vmas(struct maple_tree *mt, struct vm_area_struct *vmas, > > return; > > > > mas_lock(&mas); > > - for (i = 0; i < nr_vmas; i++) > > - vma_mas_store(&vmas[i], &mas); > > + for (i = 0; i < nr_vmas; i++) { > > + mas_set_range(&mas, vmas[i].vm_start, vmas[i].vm_end - 1); > > + mas_store_gfp(&mas, &vmas[i], GFP_KERNEL); > > + } > > On the latest mm-unstable, vma_mas_store() uses mas_store_prealloc() instead of > mas_store_gfp(). Seems the difference would make no problem to this test code > in most cases, but could I ask the reason for this change? mas_store_prealloc() expects the maple state to have the necessary memory to store the value. Using this function is the right way of storing the range. In fact, we would only need a single node since these values will be append operations anyways. > > Also, should we check the return value of mas_store_gfp()? I can add this. The only reason we would return an error is on ENOMEM which seems unlikely here. Again, it is a single node that will be used. The size is 256B, but it's safer to add the check. [1] https://lore.kernel.org/linux-mm/20230105191517.3099082-28-Liam.Howlett@oracle.com/ Thanks, Liam ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 26/44] mm/damon: Stop using vma_mas_store() for maple tree store 2023-01-05 19:52 ` Liam Howlett @ 2023-01-05 20:16 ` SeongJae Park 0 siblings, 0 replies; 4+ messages in thread From: SeongJae Park @ 2023-01-05 20:16 UTC (permalink / raw) To: Liam Howlett Cc: SeongJae Park, maple-tree@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton, damon@lists.linux.dev, kernel test robot Hi Liam, On Thu, 5 Jan 2023 19:52:21 +0000 Liam Howlett <liam.howlett@oracle.com> wrote: > * SeongJae Park <sj@kernel.org> [230105 14:33]: > > Hi Liam, > > > > On Thu, 5 Jan 2023 19:16:00 +0000 Liam Howlett <liam.howlett@oracle.com> wrote: > > > > > From: "Liam R. Howlett" <Liam.Howlett@oracle.com> > > > > > > Prepare for the removal of the vma_mas_store() function by open coding > > > the maple tree store in this test code. > > > > But seems this series is not really removing 'vma_mas_store()'. Wouldn't it > > better to do the preparation and removal together in a same patch series? > > It does from the all code but the nommu side. The definition is dropped > from the header and c file in "mmap: Convert __vma_adjust() to use vma > iterator" [1]. Thank you for nice explanation. > > > > > > 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 | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/mm/damon/vaddr-test.h b/mm/damon/vaddr-test.h > > > index bce37c487540..41532f7355d0 100644 > > > --- a/mm/damon/vaddr-test.h > > > +++ b/mm/damon/vaddr-test.h > > > @@ -24,8 +24,10 @@ static void __link_vmas(struct maple_tree *mt, struct vm_area_struct *vmas, > > > return; > > > > > > mas_lock(&mas); > > > - for (i = 0; i < nr_vmas; i++) > > > - vma_mas_store(&vmas[i], &mas); > > > + for (i = 0; i < nr_vmas; i++) { > > > + mas_set_range(&mas, vmas[i].vm_start, vmas[i].vm_end - 1); > > > + mas_store_gfp(&mas, &vmas[i], GFP_KERNEL); > > > + } > > > > On the latest mm-unstable, vma_mas_store() uses mas_store_prealloc() instead of > > mas_store_gfp(). Seems the difference would make no problem to this test code > > in most cases, but could I ask the reason for this change? > > mas_store_prealloc() expects the maple state to have the necessary > memory to store the value. Using this function is the right way of > storing the range. In fact, we would only need a single node since > these values will be append operations anyways. Again, thank you for nice explanation. > > > > > Also, should we check the return value of mas_store_gfp()? > > I can add this. The only reason we would return an error is on ENOMEM > which seems unlikely here. Again, it is a single node that will be > used. The size is 256B, but it's safer to add the check. You're right. I'd prefer having the check, but I'd not block this for the trivial nit. Reviewed-by: SeongJae Park <sj@kernel.org> Thanks, SJ > > [1] https://lore.kernel.org/linux-mm/20230105191517.3099082-28-Liam.Howlett@oracle.com/ > > > Thanks, > Liam > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-01-05 21:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20230105191517.3099082-1-Liam.Howlett@oracle.com>
2023-01-05 19:16 ` [PATCH v2 26/44] mm/damon: Stop using vma_mas_store() for maple tree store Liam Howlett
2023-01-05 19:32 ` SeongJae Park
2023-01-05 19:52 ` Liam Howlett
2023-01-05 20:16 ` SeongJae Park
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox