Re: prospective 13/12 s390 pgtable_list patch

Hugh Dickins
2023-06-27T03:48:03Z

[-- Attachment #1: Type: text/plain, Size: 5317 bytes --]

On Mon, 26 Jun 2023, Alexander Gordeev wrote:
> On Fri, Jun 23, 2023 at 01:53:43PM -0700, Hugh Dickins wrote:
> > > Anyway, that is just another option and I will try your patch.
> >
> > Thank you, please do, if you have the time: my series does need some
> > kind of s390 solution, depending on whatever s390 has in its tree at
> > the time: for now I should at least be showing the 13/12 (preferably
> > known to build and appearing to work!), even if all it does is help
> > people to say "no, that's too much".
>
> Hi Hugh,
>
> I took a bit different approach to run it: with and withough HH bits
> in play.
>
> IOW (if I am not mistaken) without the three patch serieses these two
> patches on top of v6.4-rc5 alone still should manage page allocation,
> as if pte_free_defer() does not exist/never get called:
>
> s390: add pte_free_defer() for pgtables sharing page
> prospective 13/12 s390 pgtable_list patch

That was a good idea, to try the "bare bones" first. Yes, I think that
should work okay: with no actual dependence on the preceding patches.

>
> These patches appear to work (aka survive LTP MM tests).

And that is very good news, thank you (I was expecting I'd at least
have done some silly typo like last time).

>
> The next step would be to try the three serieses on top of v6.4-rc5 plus
> patch 13/12, as you intended:
>
> https://lore.kernel.org/linux-mm/77a5d8c-406b-7068-4f17-23b7ac53bc83@google.com/

There's maybe no difference that matters, but that was the v1 series: v2 was
https://lore.kernel.org/linux-mm/a4963be9-7aa6-350-66d0-2ba843e1af44@google.com/
series of 23 posted on 2023-06-08.

> https://lore.kernel.org/linux-mm/68a97fbe-5c1e-7ac6-72c-7b9c6290b370@google.com/

There's maybe no difference that matters, but that was the v1 series: v2 was
https://lore.kernel.org/linux-mm/c1c9a74a-bc5b-15ea-e5d2-8ec34bc921d@google.com/
series of 32 posted on 2023-06-08.

> https://lore.kernel.org/linux-mm/54cb04f-3762-987f-8294-91dafd8ebfb0@google.com

Right.

> https://lore.kernel.org/r/a69a26c0-ec93-3ad-a443-6655b5e49df2@google.com

Right.

>
> I had to manually tweak this one with a fuzz in order to circumvent 'git am' error:
> [PATCH v2 28/32] mm/memory: allow pte_offset_map[_lock]() to fail: fix
>
> But in the end it still does not compile for me:

I think perhaps you missed out the actual
[PATCH v2 28/32] mm/memory: allow pte_offset_map[_lock]() to fail
(oh, not exactly, you were working with v1 there) and tried to apply
[PATCH v2 28/32 fix] mm/memory: allow pte_offset_map[_lock]() to fail: fix
instead of it; whereas the "fix" patch is a cumulative patch on top of the
missed one (and the "fix" patch is not at all important for your testing,
since update_mmu_tlb() is a no-op on s390).

(There was other pfaffing around on PATCH v2 28/32, but that was
about applying it to akpm's tree at the time: v6.4-rc5 no problem.)

>
> mm/memory.c: In function ‘zap_pmd_range’:
> mm/memory.c:1561:21: error: implicit declaration of function ‘pmd_none_or_trans_huge_or_clear_bad’ [-Werror=implicit-function-declaration]
> 1561 | if (pmd_none_or_trans_huge_or_clear_bad(pmd))
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> CC fs/btrfs/dev-replace.o
> In file included from ./include/linux/build_bug.h:5,
> from ./include/linux/container_of.h:5,
> from ./include/linux/list.h:5,
> from ./include/linux/smp.h:12,
> from ./include/linux/kernel_stat.h:5,
> from mm/memory.c:42:
> mm/memory.c: In function ‘do_anonymous_page’:
> mm/memory.c:4058:22: error: implicit declaration of function ‘pmd_trans_unstable’; did you mean ‘pud_trans_unstable’? [-Werror=implicit-function-declaration]
> 4058 | if (unlikely(pmd_trans_unstable(vmf->pmd)))
> | ^~~~~~~~~~~~~~~~~~
> ./include/linux/compiler.h:77:45: note: in definition of macro ‘unlikely’
> 77 | # define unlikely(x) __builtin_expect(!!(x), 0)
> | ^
> CC fs/xfs/xfs_iops.o
> mm/memory.c: In function ‘finish_fault’:
> mm/memory.c:4388:13: error: implicit declaration of function ‘pmd_devmap_trans_unstable’; did you mean ‘pud_trans_unstable’? [-Werror=implicit-function-declaration]
> 4388 | if (pmd_devmap_trans_unstable(vmf->pmd))
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> | pud_trans_unstable
>
>
> Could you please let me know if I am missing something?

I think, start again with the v2 of each of the three series on v6.4-rc5,
including the v2 28/32, and I expect that to work out fine.

What you already tested above has achieved a lot: since my changes left
pte_free_defer() and page_table_free_rcu() sharing __tlb_remove_table(),
so page_table_free_rcu() would already be exercising that list_add().

But you may have difficulty actually exercising the pte_free_defer().
Maybe hack in a counter (I tend to hack on BALLOON events in vmstat,
since normally those would be 0 for me) to check whether running LTP
is actually exercising those paths - if not, we'll devise something more.

Thanks,
Hugh