| Message ID | 20240610120209.66311-3-ioworker0@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Reclaim lazyfree THP without splitting | expand |
On 10.06.24 14:02, Lance Yang wrote: > Introduce the page_vma_mapped_walk_restart() helper to handle scenarios > where the page table walk needs to be restarted due to changes in the page > table, such as when a PMD is split. It releases the PTL held during the > previous walk and resets the state, allowing a new walk to start at the > current address stored in pvmw->address. > > Suggested-by: David Hildenbrand <david@redhat.com> > Signed-off-by: Lance Yang <ioworker0@gmail.com> > --- > include/linux/rmap.h | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/include/linux/rmap.h b/include/linux/rmap.h > index 7229b9baf20d..5f18509610cc 100644 > --- a/include/linux/rmap.h > +++ b/include/linux/rmap.h > @@ -710,6 +710,28 @@ static inline void page_vma_mapped_walk_done(struct page_vma_mapped_walk *pvmw) > spin_unlock(pvmw->ptl); > } > > +/** > + * page_vma_mapped_walk_restart - Restart the page table walk. > + * @pvmw: Pointer to struct page_vma_mapped_walk. > + * > + * It restarts the page table walk when changes occur in the page > + * table, such as splitting a PMD. Ensures that the PTL held during > + * the previous walk is released and resets the state to allow for > + * a new walk starting at the current address stored in pvmw->address. > + */ > +static inline void > +page_vma_mapped_walk_restart(struct page_vma_mapped_walk *pvmw) > +{ > + WARN_ON_ONCE(!pvmw->pmd); Can we have this more general, like WARN_ON_ONCE(!pvmw->pmd && !pvmw->pte); And then setting both to NULL below? > + WARN_ON_ONCE(!pvmw->ptl); This is confusing: you check for ptl below. What would be clearer is if (likely(pvmw->ptl)) spin_unlock(pvmw->ptl); else WARN_ON_ONCE(1); > + > + if (pvmw->ptl) > + spin_unlock(pvmw->ptl); > + > + pvmw->ptl = NULL; > + pvmw->pmd = NULL; > +} > + > bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw); > > /* I'd suggest squashing that into the next patch.
On Thu, Jun 13, 2024 at 4:30 PM David Hildenbrand <david@redhat.com> wrote: > > On 10.06.24 14:02, Lance Yang wrote: > > Introduce the page_vma_mapped_walk_restart() helper to handle scenarios > > where the page table walk needs to be restarted due to changes in the page > > table, such as when a PMD is split. It releases the PTL held during the > > previous walk and resets the state, allowing a new walk to start at the > > current address stored in pvmw->address. > > > > Suggested-by: David Hildenbrand <david@redhat.com> > > Signed-off-by: Lance Yang <ioworker0@gmail.com> > > --- > > include/linux/rmap.h | 22 ++++++++++++++++++++++ > > 1 file changed, 22 insertions(+) > > > > diff --git a/include/linux/rmap.h b/include/linux/rmap.h > > index 7229b9baf20d..5f18509610cc 100644 > > --- a/include/linux/rmap.h > > +++ b/include/linux/rmap.h > > @@ -710,6 +710,28 @@ static inline void page_vma_mapped_walk_done(struct page_vma_mapped_walk *pvmw) > > spin_unlock(pvmw->ptl); > > } > > > > +/** > > + * page_vma_mapped_walk_restart - Restart the page table walk. > > + * @pvmw: Pointer to struct page_vma_mapped_walk. > > + * > > + * It restarts the page table walk when changes occur in the page > > + * table, such as splitting a PMD. Ensures that the PTL held during > > + * the previous walk is released and resets the state to allow for > > + * a new walk starting at the current address stored in pvmw->address. > > + */ > > +static inline void > > +page_vma_mapped_walk_restart(struct page_vma_mapped_walk *pvmw) > > +{ > > + WARN_ON_ONCE(!pvmw->pmd); > > Can we have this more general, like > > WARN_ON_ONCE(!pvmw->pmd && !pvmw->pte); Sure, let’s make it more general. > > And then setting both to NULL below? > > > > + WARN_ON_ONCE(!pvmw->ptl); > > This is confusing: you check for ptl below. What would be clearer is > > if (likely(pvmw->ptl)) > spin_unlock(pvmw->ptl); > else > WARN_ON_ONCE(1); Will adjust as you suggested, thanks! > > > > + > > + if (pvmw->ptl) > > + spin_unlock(pvmw->ptl); > > + > > + pvmw->ptl = NULL; > > + pvmw->pmd = NULL; > > +} > > + > > bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw); > > > > /* > > I'd suggest squashing that into the next patch. Got it. Thanks, Lance > > -- > Cheers, > > David / dhildenb >
diff --git a/include/linux/rmap.h b/include/linux/rmap.h index 7229b9baf20d..5f18509610cc 100644 --- a/include/linux/rmap.h +++ b/include/linux/rmap.h @@ -710,6 +710,28 @@ static inline void page_vma_mapped_walk_done(struct page_vma_mapped_walk *pvmw) spin_unlock(pvmw->ptl); } +/** + * page_vma_mapped_walk_restart - Restart the page table walk. + * @pvmw: Pointer to struct page_vma_mapped_walk. + * + * It restarts the page table walk when changes occur in the page + * table, such as splitting a PMD. Ensures that the PTL held during + * the previous walk is released and resets the state to allow for + * a new walk starting at the current address stored in pvmw->address. + */ +static inline void +page_vma_mapped_walk_restart(struct page_vma_mapped_walk *pvmw) +{ + WARN_ON_ONCE(!pvmw->pmd); + WARN_ON_ONCE(!pvmw->ptl); + + if (pvmw->ptl) + spin_unlock(pvmw->ptl); + + pvmw->ptl = NULL; + pvmw->pmd = NULL; +} + bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw); /*
Introduce the page_vma_mapped_walk_restart() helper to handle scenarios where the page table walk needs to be restarted due to changes in the page table, such as when a PMD is split. It releases the PTL held during the previous walk and resets the state, allowing a new walk to start at the current address stored in pvmw->address. Suggested-by: David Hildenbrand <david@redhat.com> Signed-off-by: Lance Yang <ioworker0@gmail.com> --- include/linux/rmap.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)