On 2024/12/20 21:50, Kevin Brodsky wrote:
On 20/12/2024 12:46, Qi Zheng wrote:
Hi Kevin,
On 2024/12/20 18:49, Kevin Brodsky wrote:
[...]
Qi, shall we collaborate to make our series complementary? I believe my
series covers patch 2 and 4 of your series, but it goes further by
covering all levels and all architectures, and patches introducing
ctor/dtor are already split as Alexander suggested on your series. So my
suggestion would be:
* Remove patch 1 in my series - I'd just introduce
pagetable_{p4d,pgd}_[cd]tor with the same implementation as
pagetable_pud_[cd]tor.
* Remove patch 2 and 4 from your series and rebase it on mine.
I quickly went through your patch series. It looks like my patch 2 and
your patch 6 are duplicated, so you want me to remove my patch 2.
But I think you may not be able to simple let arm64, riscv and x86 to
use generic p4d_{alloc_one,free}(). Because even if
CONFIG_PGTABLE_LEVELS > 4, the pgtable_l5_enabled() may not be true.
For example, in arm64:
#if CONFIG_PGTABLE_LEVELS > 4
static __always_inline bool pgtable_l5_enabled(void)
{
if (!alternative_has_cap_likely(ARM64_ALWAYS_BOOT))
return vabits_actual == VA_BITS;
return alternative_has_cap_unlikely(ARM64_HAS_VA52);
}
Correct. That's why the implementation of p4d_free() I introduce in
patch 6 checks mm_p4d_folded(), which is implemented as
!pgtable_l5_enabled() on those architectures (see last paragraph in
commit message). In fact it turns out Alexander suggested exactly this
approach [2].
OK, I see.
Did I miss something?
My patch series is not only for cleanup, but also for fixes of
UAF issue [1], so is it possible to rebase your patch series onto
mine? I can post v3 ASAP.
I see, yours should be merged first then. The issue is that yours would
depend on some of the patches in mine, not the other way round.
My suggestion would then be for you to take patch 5, 6 and 7 from my
series, as they match Alexander's suggestions (and patch 5 is I think a
useful simplification), and replace patch 2 in your series with those. I
would then rebase my series on top and adapt it accordingly. Does that
sound reasonable?
Sounds good. But maybe just patch 5 and 6. Because I actually did
the work of your patch 7 in my patch 2 and 4.
So, is it okay to do something like the following?
1. I separate the ctor()/dtor() part from my patch 2, and then replace
the rest with your patch 6.
2. take your patch 5 form your series
If it's ok, I will post the v3 next Monday. ;)
Thanks!
- Kevin
[2]
https://lore.kernel.org/all/Z2RKpdv7pL34MIEt@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
