Re: [PATCH V3 0/4] mm/debug_vm_pgtable: Add some more tests
On Wed, Jun 24, 2020 at 08:43:10AM +0530, Anshuman Khandual wrote: [...] > Hello Gerald/Christophe/Vineet, > > It would be really great if you could give this series a quick test > on s390/ppc/arc platforms respectively. Thank you. That worked for me with the default and debug s390 configurations. Would you like to try with some particular options or combinations of the options? > - Anshuman ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH V3 0/4] mm/debug_vm_pgtable: Add some more tests
On Wed, Jun 24, 2020 at 01:48:08PM +0200, Gerald Schaefer wrote: > On Wed, 24 Jun 2020 13:05:39 +0200 > Alexander Gordeev wrote: > > > On Wed, Jun 24, 2020 at 08:43:10AM +0530, Anshuman Khandual wrote: > > > > [...] > > > > > Hello Gerald/Christophe/Vineet, > > > > > > It would be really great if you could give this series a quick test > > > on s390/ppc/arc platforms respectively. Thank you. > > > > That worked for me with the default and debug s390 configurations. > > Would you like to try with some particular options or combinations > > of the options? > > It will be enabled automatically on all archs that set > ARCH_HAS_DEBUG_VM_PGTABLE, which we do for s390 unconditionally. > Also, DEBUG_VM has to be set, which we have only in the debug config. > So only the s390 debug config will have it enabled, you can check > dmesg for "debug_vm_pgtable" to see when / where it was run, and if it > triggered any warnings. Yes, that is what I did ;) I should have been more clear. I wonder whether Anshuman has in mind other options which possibly makes sense to set or unset and check how it goes with non-standard configurations. > I also checked with the v3 series, and it works fine for s390. ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH V4 0/4] mm/debug_vm_pgtable: Add some more tests
On Mon, Jul 06, 2020 at 06:18:32AM +0530, Anshuman Khandual wrote: [...] > Tested on arm64, x86 platforms but only build tested on all other enabled > platforms through ARCH_HAS_DEBUG_VM_PGTABLE i.e powerpc, arc, s390. The Sorry for missing to test earlier. Works for me on s390. Also, tried with few relevant config options to set/unset. Thanks! ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v4 0/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO API
On Mon, Feb 03, 2025 at 08:58:49AM +0200, Dmitry V. Levin wrote: Hi Dmitry, > PTRACE_SET_SYSCALL_INFO is a generic ptrace API that complements > PTRACE_GET_SYSCALL_INFO by letting the ptracer modify details of > system calls the tracee is blocked in. ... FWIW, I am getting these on s390: # ./tools/testing/selftests/ptrace/set_syscall_info TAP version 13 1..1 # Starting 1 tests from 1 test cases. # RUN global.set_syscall_info ... # set_syscall_info.c:87:set_syscall_info:Expected exp_entry->nr (-1) == info->entry.nr (65535) # set_syscall_info.c:88:set_syscall_info:wait #3: PTRACE_GET_SYSCALL_INFO #2: syscall nr mismatch # set_syscall_info: Test terminated by assertion # FAIL global.set_syscall_info not ok 1 global.set_syscall_info # FAILED: 0 / 1 tests passed. # Totals: pass:0 fail:1 xfail:0 xpass:0 skip:0 error:0 I remember one of the earlier versions (v1 or v2) was working for me. Thanks! ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 6/6] mm: Introduce ctor/dtor at PGD level
On Wed, Jan 22, 2025 at 08:49:54AM +0100, Heiko Carstens wrote:
> > > static inline pgd_t *pgd_alloc(struct mm_struct *mm)
> > > {
> > > - return (pgd_t *) crst_table_alloc(mm);
> > > + unsigned long *table = crst_table_alloc(mm);
> > > +
> > > + if (!table)
> > > + return NULL;
> >
> > I do not know status of this series, but FWIW, this call is missed:
> >
> > crst_table_init(table, _REGION1_ENTRY_EMPTY);
>
> Why is that missing?
Because the follow-up pagetable_pgd_ctor() is called against uninitialized
page table, while other pagetable_pXd_ctor() variants are called against
initialized one. I could imagine complications as result of that.
Whether Region1 table is the right choice is a big question though, as you
noticed below.
> A pgd table can be a Region1, Region2, or Region3 table. The only caller of
> this function is mm_init() via mm_alloc_pgd(); and right after mm_alloc_pgd()
> there is a call to init_new_context() which will initialize the pgd correctly.
init_new_context() is in a way a constructor as well, so whole thing looks odd
to me. But I do not immedeately see a better way :(
> I guess what really gets odd, and might be broken (haven't checked yet) is
> what happens on dynamic upgrade of page table levels (->crst_table_upgrade()).
Hmm, that is a good point.
> With that a pgd may become a pud, and with that we get an imbalance with
> the ctor/dtor calls for the various page table levels when they get freed
The ctor/dtor mismatch should not be a problem, as pagetable_pgd|p4d|pud_ctor()
are the same and there is one pagetable_dtor() for all top levels as of now.
But if it ever comes to separate implementations, then we are in the world
of pain.
> again. Plus, at first glance, it looks also broken that we have open-coded
> crst_alloc() calls instead of using the "proper" page table allocation API
> within crst_table_upgrade(), which again would cause an imbalance.
This is a good point too.
Many thanks!
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 6/6] mm: Introduce ctor/dtor at PGD level
On Fri, Jan 03, 2025 at 06:44:15PM +, Kevin Brodsky wrote:
Hi Kevin,
...
> diff --git a/arch/s390/include/asm/pgalloc.h b/arch/s390/include/asm/pgalloc.h
> index 5fced6d3c36b..b19b6ed2ab53 100644
> --- a/arch/s390/include/asm/pgalloc.h
> +++ b/arch/s390/include/asm/pgalloc.h
> @@ -130,11 +130,18 @@ static inline void pud_populate(struct mm_struct *mm,
> pud_t *pud, pmd_t *pmd)
>
> static inline pgd_t *pgd_alloc(struct mm_struct *mm)
> {
> - return (pgd_t *) crst_table_alloc(mm);
> + unsigned long *table = crst_table_alloc(mm);
> +
> + if (!table)
> + return NULL;
I do not know status of this series, but FWIW, this call is missed:
crst_table_init(table, _REGION1_ENTRY_EMPTY);
> + pagetable_pgd_ctor(virt_to_ptdesc(table));
> +
> + return (pgd_t *) table;
> }
>
> static inline void pgd_free(struct mm_struct *mm, pgd_t *pgd)
> {
> + pagetable_dtor(virt_to_ptdesc(pgd));
> crst_table_free(mm, (unsigned long *) pgd);
> }
...
Thanks!
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 6/6] mm: Introduce ctor/dtor at PGD level
On Wed, Jan 22, 2025 at 03:06:05PM +0100, Alexander Gordeev wrote:
Hi Kevin,
> On Wed, Jan 22, 2025 at 08:49:54AM +0100, Heiko Carstens wrote:
> > > > static inline pgd_t *pgd_alloc(struct mm_struct *mm)
> > > > {
> > > > - return (pgd_t *) crst_table_alloc(mm);
> > > > + unsigned long *table = crst_table_alloc(mm);
> > > > +
> > > > + if (!table)
> > > > + return NULL;
> > >
> > > I do not know status of this series, but FWIW, this call is missed:
> > >
> > > crst_table_init(table, _REGION1_ENTRY_EMPTY);
> >
> > Why is that missing?
>
> Because the follow-up pagetable_pgd_ctor() is called against uninitialized
> page table, while other pagetable_pXd_ctor() variants are called against
> initialized one. I could imagine complications as result of that.
>
> Whether Region1 table is the right choice is a big question though, as you
> noticed below.
As discussed with Heiko, we do not want to add the extra crst_table_init() call
at least due to performance impact. So please ignore my comment.
> > A pgd table can be a Region1, Region2, or Region3 table. The only caller of
> > this function is mm_init() via mm_alloc_pgd(); and right after
> > mm_alloc_pgd()
> > there is a call to init_new_context() which will initialize the pgd
> > correctly.
>
> init_new_context() is in a way a constructor as well, so whole thing looks odd
> to me. But I do not immedeately see a better way :(
>
> > I guess what really gets odd, and might be broken (haven't checked yet) is
> > what happens on dynamic upgrade of page table levels
> > (->crst_table_upgrade()).
>
> Hmm, that is a good point.
>
> > With that a pgd may become a pud, and with that we get an imbalance with
> > the ctor/dtor calls for the various page table levels when they get freed
>
> The ctor/dtor mismatch should not be a problem, as
> pagetable_pgd|p4d|pud_ctor()
> are the same and there is one pagetable_dtor() for all top levels as of now.
> But if it ever comes to separate implementations, then we are in the world
> of pain.
>
> > again. Plus, at first glance, it looks also broken that we have open-coded
> > crst_alloc() calls instead of using the "proper" page table allocation API
> > within crst_table_upgrade(), which again would cause an imbalance.
>
> This is a good point too.
The below bits are seems to be missed. We will test it and send a patch.
diff --git a/arch/s390/mm/pgalloc.c b/arch/s390/mm/pgalloc.c
index a4e761902093..d33f55b7ee98 100644
--- a/arch/s390/mm/pgalloc.c
+++ b/arch/s390/mm/pgalloc.c
@@ -88,12 +88,14 @@ int crst_table_upgrade(struct mm_struct *mm, unsigned long
end)
if (unlikely(!p4d))
goto err_p4d;
crst_table_init(p4d, _REGION2_ENTRY_EMPTY);
+ pagetable_p4d_ctor(virt_to_ptdesc(p4d));
}
if (end > _REGION1_SIZE) {
pgd = crst_table_alloc(mm);
if (unlikely(!pgd))
goto err_pgd;
crst_table_init(pgd, _REGION1_ENTRY_EMPTY);
+ pagetable_pgd_ctor(virt_to_ptdesc(pgd));
}
spin_lock_bh(&mm->page_table_lock);
@@ -130,6 +132,7 @@ int crst_table_upgrade(struct mm_struct *mm, unsigned long
end)
return 0;
err_pgd:
+ pagetable_dtor(virt_to_ptdesc(p4d));
crst_table_free(mm, p4d);
err_p4d:
return -ENOMEM;
Thanks!
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
