On 4/7/23 3:04 PM, Michał Mirosław wrote:
On Fri, 7 Apr 2023 at 11:35, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 12:23 PM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 23:12, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 1:12 AM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 09:40, Muhammad Usama Anjum usama.anjum@collabora.com wrote: [...]
--- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c
[...]
+static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start,
unsigned long end, struct mm_walk *walk)+{
[...]
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
ptl = pmd_trans_huge_lock(pmd, vma);if (ptl) {[...]
return ret;}+process_smaller_pages:
if (pmd_trans_unstable(pmd))return 0;Why pmd_trans_unstable() is needed here and not only after split_huge_pmd()?
I'm not entirely sure. But the idea is if THP is unstable, we should return. As it doesn't seem like after splitting THP can be unstable, we should not check it. Do you agree with the following?
The description of pmd_trans_unstable() [1] seems to indicate that it is needed only after split_huge_pmd().
[1] https://elixir.bootlin.com/linux/v6.3-rc5/source/include/linux/pgtable.h#L13...
Sorry, yeah pmd_trans_unstable() is need after split. But it is also needed in normal case when ptl is NULL to rule out the case if pmd is unstable before performing operation on normal pages:
ptl = pmd_trans_huge_lock(pmd, vma); if (ptl) { ... } if (pmd_trans_unstable(pmd)) return 0;
This file has usage examples of pmd_trans_unstable():
https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L634 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1195 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1543 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1887
So we are good with what we have in this patch.
Shouldn't we signal ACTION_AGAIN then in order to call .pte_hole?
I'm not sure. I've not done research on it if we need to signal ACTION_AGAIN as this function pagemap_scan_pmd_entry() mimics how pagemap_pmd_range() handles reads to the pagemap file. pagemap_pmd_range() isn't doing anything if pmd is unstable. Hence we also not doing anything.
Best Regards Michał Mirosław