Re: prospective 13/12 s390 pgtable_list patch

Gerald Schaefer
2023-06-28T19:16:03Z

On Thu, 22 Jun 2023 22:49:43 -0700 (PDT)
Hugh Dickins <hughd@google.com> wrote:

> Hi Gerald,
>
> It's that moment you've been dreading: I'm hoping that you can, please,
> take a look at the patch below, and try building and running with it,
> on top of the v2 series of 12 I sent out on Tuesday.

Wow, not sure if this is now dreadful or awesome, it could be the latter
since this might actually work. But as Alexander already said, we would
rather not add more complexity, for an already questionable benefit with
regard to saving some page table memory.

I have revisited my last approach with not adding back fragments in the
pte_free_defer() path, and I am rather confident now that there is no
"list_del() without list_add()" flaw any more. I had to use the
page->pt_frag_refcount for this, to track list status. So even though we
do not need page->pt_mm any more, that union is still not free for other
things. But I guess that is acceptable, and I especially like that the
patch will almost not change existing code at all, apart from the list
status tracking.

See my reply to your patch 07/12 in the other thread, for the patch.
It is tested with LTP and patches from all three series applied, and
it would allow going further with your work by having some s390 solution,
that is neither obviously flawed, nor too complex to handle or understand
also in the future.

I also left a TODO comment in the patch, with regard to the still open
question if we need the gmap_unlink(mm, pgtable, addr) also in
pte_free_defer(), similar to page_table_free_rcu(). If yes, we would need
some way to pass along the addr, which should be possible. However, IIUC,
retract_page_tables() is currently ending up in a normal page_table_free(),
w/o your work. And there we also have no addr available, and do no
gmap_unlink(), so there might be a good chance that this is not needed here.