From: Jiri Olsa <olsajiri@gmail.com> To: David Hildenbrand <david@redhat.com> Cc: Jiri Olsa <olsajiri@gmail.com>, Ryan Roberts <ryan.roberts@arm.com>, Andrew Morton <akpm@linux-foundation.org>, Matthew Wilcox <willy@infradead.org>, Yin Fengwei <fengwei.yin@intel.com>, Yu Zhao <yuzhao@google.com>, Catalin Marinas <catalin.marinas@arm.com>, Anshuman Khandual <anshuman.khandual@arm.com>, Yang Shi <shy828301@gmail.com>, "Huang, Ying" <ying.huang@intel.com>, Zi Yan <ziy@nvidia.com>, Luis Chamberlain <mcgrof@kernel.org>, Itaru Kitayama <itaru.kitayama@gmail.com>, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, John Hubbard <jhubbard@nvidia.com>, David Rientjes <rientjes@google.com>, Vlastimil Babka <vbabka@suse.cz>, Hugh Dickins <hughd@google.com>, Kefeng Wang <wangkefeng.wang@huawei.com>, Barry Song <21cnbao@gmail.com>, Alistair Popple <apopple@nvidia.com>, linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Barry Song <v-songbaohua@oppo.com> Subject: Re: [PATCH v9 02/10] mm: Non-pmd-mappable, large folios for folio_add_new_anon_rmap() Date: Sun, 14 Jan 2024 21:55:15 +0100 [thread overview] Message-ID: <ZaRKMwKJIBmh8-lD@krava> (raw) In-Reply-To: <41dc7dff-1ea8-4894-a487-88d46ec2b2d8@redhat.com> On Sun, Jan 14, 2024 at 06:33:56PM +0100, David Hildenbrand wrote: > On 13.01.24 23:42, Jiri Olsa wrote: > > On Thu, Dec 07, 2023 at 04:12:03PM +0000, Ryan Roberts wrote: > > > In preparation for supporting anonymous multi-size THP, improve > > > folio_add_new_anon_rmap() to allow a non-pmd-mappable, large folio to be > > > passed to it. In this case, all contained pages are accounted using the > > > order-0 folio (or base page) scheme. > > > > > > Reviewed-by: Yu Zhao <yuzhao@google.com> > > > Reviewed-by: Yin Fengwei <fengwei.yin@intel.com> > > > Reviewed-by: David Hildenbrand <david@redhat.com> > > > Reviewed-by: Barry Song <v-songbaohua@oppo.com> > > > Tested-by: Kefeng Wang <wangkefeng.wang@huawei.com> > > > Tested-by: John Hubbard <jhubbard@nvidia.com> > > > Signed-off-by: Ryan Roberts <ryan.roberts@arm.com> > > > --- > > > mm/rmap.c | 28 ++++++++++++++++++++-------- > > > 1 file changed, 20 insertions(+), 8 deletions(-) > > > > > > diff --git a/mm/rmap.c b/mm/rmap.c > > > index 2a1e45e6419f..846fc79f3ca9 100644 > > > --- a/mm/rmap.c > > > +++ b/mm/rmap.c > > > @@ -1335,32 +1335,44 @@ void page_add_anon_rmap(struct page *page, struct vm_area_struct *vma, > > > * This means the inc-and-test can be bypassed. > > > * The folio does not have to be locked. > > > * > > > - * If the folio is large, it is accounted as a THP. As the folio > > > + * If the folio is pmd-mappable, it is accounted as a THP. As the folio > > > * is new, it's assumed to be mapped exclusively by a single process. > > > */ > > > void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma, > > > unsigned long address) > > > { > > > - int nr; > > > + int nr = folio_nr_pages(folio); > > > - VM_BUG_ON_VMA(address < vma->vm_start || address >= vma->vm_end, vma); > > > + VM_BUG_ON_VMA(address < vma->vm_start || > > > + address + (nr << PAGE_SHIFT) > vma->vm_end, vma); > > > > hi, > > I'm hitting this bug (console output below) with adding uprobe > > on simple program like: > > > > $ cat up.c > > int main(void) > > { > > return 0; > > } > > > > # bpftrace -e 'uprobe:/home/jolsa/up:_start {}' > > > > $ ./up > > > > it's on top of current linus tree master: > > 052d534373b7 Merge tag 'exfat-for-6.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/linkinjeon/exfat > > > > before this patch it seems to work, I can send my .config if needed > > bpf only inserts a small folio, so no magic there. > > It was: > VM_BUG_ON_VMA(address < vma->vm_start || address >= vma->vm_end, vma); > And now it is > VM_BUG_ON_VMA(address < vma->vm_start || address + (nr << PAGE_SHIFT) > vma->vm_end, vma); > > I think this change is sane. As long as the address is aligned to full pages > (which it better should be) > > Staring at uprobe_write_opcode, likely vaddr isn't aligned ... > > Likely (hopefully) that is not an issue for __folio_set_anon(), because linear_page_index() > will mask these bits off. > > > Would the following change fix it for you? great, that fixes it for me, you can add my Tested-by: Jiri Olsa <jolsa@kernel.org> thanks, jirka > > From c640a8363e47bc96965a35115a040b5f876c4320 Mon Sep 17 00:00:00 2001 > From: David Hildenbrand <david@redhat.com> > Date: Sun, 14 Jan 2024 18:32:57 +0100 > Subject: [PATCH] tmp > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > kernel/events/uprobes.c | 2 +- > mm/rmap.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > index 485bb0389b488..929e98c629652 100644 > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -537,7 +537,7 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm, > } > } > - ret = __replace_page(vma, vaddr, old_page, new_page); > + ret = __replace_page(vma, vaddr & PAGE_MASK, old_page, new_page); > if (new_page) > put_page(new_page); > put_old: > diff --git a/mm/rmap.c b/mm/rmap.c > index f5d43edad529a..a903db4df6b97 100644 > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -1408,6 +1408,7 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma, > { > int nr = folio_nr_pages(folio); > + VM_WARN_ON_FOLIO(!IS_ALIGNED(address, PAGE_SIZE), folio); > VM_WARN_ON_FOLIO(folio_test_hugetlb(folio), folio); > VM_BUG_ON_VMA(address < vma->vm_start || > address + (nr << PAGE_SHIFT) > vma->vm_end, vma); > -- > 2.43.0 > > > > -- > Cheers, > > David / dhildenb >
WARNING: multiple messages have this Message-ID (diff)
From: Jiri Olsa <olsajiri@gmail.com> To: David Hildenbrand <david@redhat.com> Cc: Jiri Olsa <olsajiri@gmail.com>, Ryan Roberts <ryan.roberts@arm.com>, Andrew Morton <akpm@linux-foundation.org>, Matthew Wilcox <willy@infradead.org>, Yin Fengwei <fengwei.yin@intel.com>, Yu Zhao <yuzhao@google.com>, Catalin Marinas <catalin.marinas@arm.com>, Anshuman Khandual <anshuman.khandual@arm.com>, Yang Shi <shy828301@gmail.com>, "Huang, Ying" <ying.huang@intel.com>, Zi Yan <ziy@nvidia.com>, Luis Chamberlain <mcgrof@kernel.org>, Itaru Kitayama <itaru.kitayama@gmail.com>, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, John Hubbard <jhubbard@nvidia.com>, David Rientjes <rientjes@google.com>, Vlastimil Babka <vbabka@suse.cz>, Hugh Dickins <hughd@google.com>, Kefeng Wang <wangkefeng.wang@huawei.com>, Barry Song <21cnbao@gmail.com>, Alistair Popple <apopple@nvidia.com>, linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Barry Song <v-songbaohua@oppo.com> Subject: Re: [PATCH v9 02/10] mm: Non-pmd-mappable, large folios for folio_add_new_anon_rmap() Date: Sun, 14 Jan 2024 21:55:15 +0100 [thread overview] Message-ID: <ZaRKMwKJIBmh8-lD@krava> (raw) In-Reply-To: <41dc7dff-1ea8-4894-a487-88d46ec2b2d8@redhat.com> On Sun, Jan 14, 2024 at 06:33:56PM +0100, David Hildenbrand wrote: > On 13.01.24 23:42, Jiri Olsa wrote: > > On Thu, Dec 07, 2023 at 04:12:03PM +0000, Ryan Roberts wrote: > > > In preparation for supporting anonymous multi-size THP, improve > > > folio_add_new_anon_rmap() to allow a non-pmd-mappable, large folio to be > > > passed to it. In this case, all contained pages are accounted using the > > > order-0 folio (or base page) scheme. > > > > > > Reviewed-by: Yu Zhao <yuzhao@google.com> > > > Reviewed-by: Yin Fengwei <fengwei.yin@intel.com> > > > Reviewed-by: David Hildenbrand <david@redhat.com> > > > Reviewed-by: Barry Song <v-songbaohua@oppo.com> > > > Tested-by: Kefeng Wang <wangkefeng.wang@huawei.com> > > > Tested-by: John Hubbard <jhubbard@nvidia.com> > > > Signed-off-by: Ryan Roberts <ryan.roberts@arm.com> > > > --- > > > mm/rmap.c | 28 ++++++++++++++++++++-------- > > > 1 file changed, 20 insertions(+), 8 deletions(-) > > > > > > diff --git a/mm/rmap.c b/mm/rmap.c > > > index 2a1e45e6419f..846fc79f3ca9 100644 > > > --- a/mm/rmap.c > > > +++ b/mm/rmap.c > > > @@ -1335,32 +1335,44 @@ void page_add_anon_rmap(struct page *page, struct vm_area_struct *vma, > > > * This means the inc-and-test can be bypassed. > > > * The folio does not have to be locked. > > > * > > > - * If the folio is large, it is accounted as a THP. As the folio > > > + * If the folio is pmd-mappable, it is accounted as a THP. As the folio > > > * is new, it's assumed to be mapped exclusively by a single process. > > > */ > > > void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma, > > > unsigned long address) > > > { > > > - int nr; > > > + int nr = folio_nr_pages(folio); > > > - VM_BUG_ON_VMA(address < vma->vm_start || address >= vma->vm_end, vma); > > > + VM_BUG_ON_VMA(address < vma->vm_start || > > > + address + (nr << PAGE_SHIFT) > vma->vm_end, vma); > > > > hi, > > I'm hitting this bug (console output below) with adding uprobe > > on simple program like: > > > > $ cat up.c > > int main(void) > > { > > return 0; > > } > > > > # bpftrace -e 'uprobe:/home/jolsa/up:_start {}' > > > > $ ./up > > > > it's on top of current linus tree master: > > 052d534373b7 Merge tag 'exfat-for-6.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/linkinjeon/exfat > > > > before this patch it seems to work, I can send my .config if needed > > bpf only inserts a small folio, so no magic there. > > It was: > VM_BUG_ON_VMA(address < vma->vm_start || address >= vma->vm_end, vma); > And now it is > VM_BUG_ON_VMA(address < vma->vm_start || address + (nr << PAGE_SHIFT) > vma->vm_end, vma); > > I think this change is sane. As long as the address is aligned to full pages > (which it better should be) > > Staring at uprobe_write_opcode, likely vaddr isn't aligned ... > > Likely (hopefully) that is not an issue for __folio_set_anon(), because linear_page_index() > will mask these bits off. > > > Would the following change fix it for you? great, that fixes it for me, you can add my Tested-by: Jiri Olsa <jolsa@kernel.org> thanks, jirka > > From c640a8363e47bc96965a35115a040b5f876c4320 Mon Sep 17 00:00:00 2001 > From: David Hildenbrand <david@redhat.com> > Date: Sun, 14 Jan 2024 18:32:57 +0100 > Subject: [PATCH] tmp > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > kernel/events/uprobes.c | 2 +- > mm/rmap.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > index 485bb0389b488..929e98c629652 100644 > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -537,7 +537,7 @@ int uprobe_write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm, > } > } > - ret = __replace_page(vma, vaddr, old_page, new_page); > + ret = __replace_page(vma, vaddr & PAGE_MASK, old_page, new_page); > if (new_page) > put_page(new_page); > put_old: > diff --git a/mm/rmap.c b/mm/rmap.c > index f5d43edad529a..a903db4df6b97 100644 > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -1408,6 +1408,7 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma, > { > int nr = folio_nr_pages(folio); > + VM_WARN_ON_FOLIO(!IS_ALIGNED(address, PAGE_SIZE), folio); > VM_WARN_ON_FOLIO(folio_test_hugetlb(folio), folio); > VM_BUG_ON_VMA(address < vma->vm_start || > address + (nr << PAGE_SHIFT) > vma->vm_end, vma); > -- > 2.43.0 > > > > -- > Cheers, > > David / dhildenb > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2024-01-14 20:55 UTC|newest] Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-12-07 16:12 [PATCH v9 00/10] Multi-size THP for anonymous memory Ryan Roberts 2023-12-07 16:12 ` Ryan Roberts 2023-12-07 16:12 ` [PATCH v9 01/10] mm: Allow deferred splitting of arbitrary anon large folios Ryan Roberts 2023-12-07 16:12 ` Ryan Roberts 2023-12-07 16:12 ` [PATCH v9 02/10] mm: Non-pmd-mappable, large folios for folio_add_new_anon_rmap() Ryan Roberts 2023-12-07 16:12 ` Ryan Roberts 2024-01-13 22:42 ` Jiri Olsa 2024-01-13 22:42 ` Jiri Olsa 2024-01-14 17:33 ` David Hildenbrand 2024-01-14 17:33 ` David Hildenbrand 2024-01-14 20:55 ` Jiri Olsa [this message] 2024-01-14 20:55 ` Jiri Olsa 2024-01-15 8:50 ` Ryan Roberts 2024-01-15 8:50 ` Ryan Roberts 2024-01-15 9:38 ` David Hildenbrand 2024-01-15 9:38 ` David Hildenbrand 2024-01-24 11:15 ` Sven Schnelle 2024-01-24 11:15 ` Sven Schnelle 2024-01-24 11:19 ` Jiri Olsa 2024-01-24 11:19 ` Jiri Olsa 2024-01-24 12:02 ` Ryan Roberts 2024-01-24 12:02 ` Ryan Roberts 2024-01-24 12:06 ` Jiri Olsa 2024-01-24 12:06 ` Jiri Olsa 2024-01-24 12:17 ` Ryan Roberts 2024-01-24 12:17 ` Ryan Roberts 2024-01-24 12:28 ` Sven Schnelle 2024-01-24 12:28 ` Sven Schnelle 2024-01-24 12:42 ` Ryan Roberts 2024-01-24 12:42 ` Ryan Roberts 2023-12-07 16:12 ` [PATCH v9 03/10] mm: thp: Introduce multi-size THP sysfs interface Ryan Roberts 2023-12-07 16:12 ` Ryan Roberts 2023-12-12 14:54 ` David Hildenbrand 2023-12-12 14:54 ` David Hildenbrand 2023-12-12 15:32 ` Ryan Roberts 2023-12-12 15:32 ` Ryan Roberts 2023-12-12 16:27 ` Andrew Morton 2023-12-12 16:27 ` Andrew Morton 2023-12-07 16:12 ` [PATCH v9 04/10] mm: thp: Support allocation of anonymous multi-size THP Ryan Roberts 2023-12-07 16:12 ` Ryan Roberts 2023-12-12 15:02 ` David Hildenbrand 2023-12-12 15:02 ` David Hildenbrand 2023-12-12 15:38 ` Ryan Roberts 2023-12-12 15:38 ` Ryan Roberts 2023-12-12 16:35 ` David Hildenbrand 2023-12-12 16:35 ` David Hildenbrand 2023-12-13 7:21 ` Dan Carpenter 2023-12-13 7:21 ` Dan Carpenter 2023-12-14 10:54 ` Ryan Roberts 2023-12-14 10:54 ` Ryan Roberts 2023-12-14 11:30 ` Dan Carpenter 2023-12-14 11:30 ` Dan Carpenter 2023-12-14 12:12 ` Ryan Roberts 2023-12-14 12:12 ` Ryan Roberts 2023-12-14 16:02 ` [PATCH] mm: Resolve some multi-size THP review nits Ryan Roberts 2023-12-14 16:02 ` Ryan Roberts 2023-12-07 16:12 ` [PATCH v9 05/10] selftests/mm/kugepaged: Restore thp settings at exit Ryan Roberts 2023-12-07 16:12 ` Ryan Roberts 2023-12-07 16:12 ` [PATCH v9 06/10] selftests/mm: Factor out thp settings management Ryan Roberts 2023-12-07 16:12 ` Ryan Roberts 2023-12-07 16:12 ` [PATCH v9 07/10] selftests/mm: Support multi-size THP interface in thp_settings Ryan Roberts 2023-12-07 16:12 ` Ryan Roberts 2023-12-07 16:12 ` [PATCH v9 08/10] selftests/mm/khugepaged: Enlighten for multi-size THP Ryan Roberts 2023-12-07 16:12 ` Ryan Roberts 2023-12-07 16:12 ` [PATCH v9 09/10] selftests/mm/cow: Generalize do_run_with_thp() helper Ryan Roberts 2023-12-07 16:12 ` Ryan Roberts 2024-01-03 6:21 ` Itaru Kitayama 2024-01-03 6:21 ` Itaru Kitayama 2024-01-03 8:33 ` Ryan Roberts 2024-01-03 8:33 ` Ryan Roberts 2024-01-04 0:09 ` Itaru Kitayama 2024-01-04 0:09 ` Itaru Kitayama 2023-12-07 16:12 ` [PATCH v9 10/10] selftests/mm/cow: Add tests for anonymous multi-size THP Ryan Roberts 2023-12-07 16:12 ` Ryan Roberts 2023-12-07 22:05 ` [PATCH v9 00/10] Multi-size THP for anonymous memory Andrew Morton 2023-12-07 22:05 ` Andrew Morton 2023-12-11 11:51 ` Ryan Roberts 2023-12-11 11:51 ` Ryan Roberts
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=ZaRKMwKJIBmh8-lD@krava \ --to=olsajiri@gmail.com \ --cc=21cnbao@gmail.com \ --cc=akpm@linux-foundation.org \ --cc=anshuman.khandual@arm.com \ --cc=apopple@nvidia.com \ --cc=catalin.marinas@arm.com \ --cc=david@redhat.com \ --cc=fengwei.yin@intel.com \ --cc=hughd@google.com \ --cc=itaru.kitayama@gmail.com \ --cc=jhubbard@nvidia.com \ --cc=kirill.shutemov@linux.intel.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mcgrof@kernel.org \ --cc=rientjes@google.com \ --cc=ryan.roberts@arm.com \ --cc=shy828301@gmail.com \ --cc=v-songbaohua@oppo.com \ --cc=vbabka@suse.cz \ --cc=wangkefeng.wang@huawei.com \ --cc=willy@infradead.org \ --cc=ying.huang@intel.com \ --cc=yuzhao@google.com \ --cc=ziy@nvidia.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.