On 2024/8/21 17:41, David Hildenbrand wrote:
On 21.08.24 11:24, Qi Zheng wrote:
On 2024/8/21 17:17, LEROY Christophe wrote:
Le 21/08/2024 à 10:18, Qi Zheng a écrit :
In handle_pte_fault(), we may modify the vmf->pte after acquiring the
vmf->ptl, so convert it to using pte_offset_map_maywrite_nolock(). But
since we already do the pte_same() check, so there is no need to get
pmdval to do pmd_same() check, just pass NULL to pmdvalp parameter.
Signed-off-by: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
---
mm/memory.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/mm/memory.c b/mm/memory.c
index 93c0c25433d02..d3378e98faf13 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5504,9 +5504,14 @@ static vm_fault_t handle_pte_fault(struct
vm_fault *vmf)
* pmd by anon khugepaged, since that takes mmap_lock in
write
* mode; but shmem or file collapse to THP could still
morph
* it into a huge pmd: just retry later if so.
+ *
+ * Use the maywrite version to indicate that vmf->pte will be
+ * modified, but since we will use pte_same() to detect the
+ * change of the pte entry, there is no need to get pmdval.
*/
- vmf->pte = pte_offset_map_nolock(vmf->vma->vm_mm, vmf->pmd,
- vmf->address, &vmf->ptl);
+ vmf->pte = pte_offset_map_maywrite_nolock(vmf->vma->vm_mm,
+ vmf->pmd, vmf->address,
+ NULL, &vmf->ptl);
I think we discussed that passing NULL should be forbidden for that
function.
Yes, but for some maywrite case, there is no need to get pmdval to
do pmd_same() check. So I passed NULL and added a comment to
explain this.
This might be the demonstration that the function name is becoming
too long.
Can you find shorter names ?
Maybe use abbreviations?
pte_offset_map_ro_nolock()
pte_offset_map_rw_nolock()
At least the "ro" is better, but "rw" does not express the "maywrite" --
because without taking the lock we are not allowed to write. But maybe
"rw" is good enough for that if we document it properly.
OK, will change to it in the next version.
And you can use up to 100 characters, if it helps readability
Got it.
