| Message ID | 20240802155524.517137-10-david@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: replace follow_page() by folio_walk | expand |
On Fri, 2 Aug 2024 17:55:22 +0200 David Hildenbrand <david@redhat.com> wrote: > Let's get rid of another follow_page() user and perform the conversion > under PTL: Note that this is also what follow_page_pte() ends up doing. > > Unfortunately we cannot currently optimize out the additional reference, > because arch_make_folio_accessible() must be called with a raised > refcount to protect against concurrent conversion to secure. We can just > move the arch_make_folio_accessible() under the PTL, like > follow_page_pte() would. > > We'll effectively drop the "writable" check implied by FOLL_WRITE: > follow_page_pte() would also not check that when calling > arch_make_folio_accessible(), so there is no good reason for doing that > here. > > We'll lose the secretmem check from follow_page() as well, about which > we shouldn't really care about. > > Signed-off-by: David Hildenbrand <david@redhat.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > arch/s390/mm/fault.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c > index 8e149ef5e89b..ad8b0d6b77ea 100644 > --- a/arch/s390/mm/fault.c > +++ b/arch/s390/mm/fault.c > @@ -34,6 +34,7 @@ > #include <linux/uaccess.h> > #include <linux/hugetlb.h> > #include <linux/kfence.h> > +#include <linux/pagewalk.h> > #include <asm/asm-extable.h> > #include <asm/asm-offsets.h> > #include <asm/ptrace.h> > @@ -492,9 +493,9 @@ void do_secure_storage_access(struct pt_regs *regs) > union teid teid = { .val = regs->int_parm_long }; > unsigned long addr = get_fault_address(regs); > struct vm_area_struct *vma; > + struct folio_walk fw; > struct mm_struct *mm; > struct folio *folio; > - struct page *page; > struct gmap *gmap; > int rc; > > @@ -536,15 +537,18 @@ void do_secure_storage_access(struct pt_regs *regs) > vma = find_vma(mm, addr); > if (!vma) > return handle_fault_error(regs, SEGV_MAPERR); > - page = follow_page(vma, addr, FOLL_WRITE | FOLL_GET); > - if (IS_ERR_OR_NULL(page)) { > + folio = folio_walk_start(&fw, vma, addr, 0); > + if (!folio) { > mmap_read_unlock(mm); > break; > } > - folio = page_folio(page); > - if (arch_make_folio_accessible(folio)) > - send_sig(SIGSEGV, current, 0); > + /* arch_make_folio_accessible() needs a raised refcount. */ > + folio_get(folio); > + rc = arch_make_folio_accessible(folio); > folio_put(folio); > + folio_walk_end(&fw, vma); > + if (rc) > + send_sig(SIGSEGV, current, 0); > mmap_read_unlock(mm); > break; > case KERNEL_FAULT:
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c index 8e149ef5e89b..ad8b0d6b77ea 100644 --- a/arch/s390/mm/fault.c +++ b/arch/s390/mm/fault.c @@ -34,6 +34,7 @@ #include <linux/uaccess.h> #include <linux/hugetlb.h> #include <linux/kfence.h> +#include <linux/pagewalk.h> #include <asm/asm-extable.h> #include <asm/asm-offsets.h> #include <asm/ptrace.h> @@ -492,9 +493,9 @@ void do_secure_storage_access(struct pt_regs *regs) union teid teid = { .val = regs->int_parm_long }; unsigned long addr = get_fault_address(regs); struct vm_area_struct *vma; + struct folio_walk fw; struct mm_struct *mm; struct folio *folio; - struct page *page; struct gmap *gmap; int rc; @@ -536,15 +537,18 @@ void do_secure_storage_access(struct pt_regs *regs) vma = find_vma(mm, addr); if (!vma) return handle_fault_error(regs, SEGV_MAPERR); - page = follow_page(vma, addr, FOLL_WRITE | FOLL_GET); - if (IS_ERR_OR_NULL(page)) { + folio = folio_walk_start(&fw, vma, addr, 0); + if (!folio) { mmap_read_unlock(mm); break; } - folio = page_folio(page); - if (arch_make_folio_accessible(folio)) - send_sig(SIGSEGV, current, 0); + /* arch_make_folio_accessible() needs a raised refcount. */ + folio_get(folio); + rc = arch_make_folio_accessible(folio); folio_put(folio); + folio_walk_end(&fw, vma); + if (rc) + send_sig(SIGSEGV, current, 0); mmap_read_unlock(mm); break; case KERNEL_FAULT:
Let's get rid of another follow_page() user and perform the conversion under PTL: Note that this is also what follow_page_pte() ends up doing. Unfortunately we cannot currently optimize out the additional reference, because arch_make_folio_accessible() must be called with a raised refcount to protect against concurrent conversion to secure. We can just move the arch_make_folio_accessible() under the PTL, like follow_page_pte() would. We'll effectively drop the "writable" check implied by FOLL_WRITE: follow_page_pte() would also not check that when calling arch_make_folio_accessible(), so there is no good reason for doing that here. We'll lose the secretmem check from follow_page() as well, about which we shouldn't really care about. Signed-off-by: David Hildenbrand <david@redhat.com> --- arch/s390/mm/fault.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)