Skip to content

Commit 4064b98

Browse files
xzpetertorvalds
authored andcommitted
mm: allow VM_FAULT_RETRY for multiple times
The idea comes from a discussion between Linus and Andrea [1]. Before this patch we only allow a page fault to retry once. We achieved this by clearing the FAULT_FLAG_ALLOW_RETRY flag when doing handle_mm_fault() the second time. This was majorly used to avoid unexpected starvation of the system by looping over forever to handle the page fault on a single page. However that should hardly happen, and after all for each code path to return a VM_FAULT_RETRY we'll first wait for a condition (during which time we should possibly yield the cpu) to happen before VM_FAULT_RETRY is really returned. This patch removes the restriction by keeping the FAULT_FLAG_ALLOW_RETRY flag when we receive VM_FAULT_RETRY. It means that the page fault handler now can retry the page fault for multiple times if necessary without the need to generate another page fault event. Meanwhile we still keep the FAULT_FLAG_TRIED flag so page fault handler can still identify whether a page fault is the first attempt or not. Then we'll have these combinations of fault flags (only considering ALLOW_RETRY flag and TRIED flag): - ALLOW_RETRY and !TRIED: this means the page fault allows to retry, and this is the first try - ALLOW_RETRY and TRIED: this means the page fault allows to retry, and this is not the first try - !ALLOW_RETRY and !TRIED: this means the page fault does not allow to retry at all - !ALLOW_RETRY and TRIED: this is forbidden and should never be used In existing code we have multiple places that has taken special care of the first condition above by checking against (fault_flags & FAULT_FLAG_ALLOW_RETRY). This patch introduces a simple helper to detect the first retry of a page fault by checking against both (fault_flags & FAULT_FLAG_ALLOW_RETRY) and !(fault_flag & FAULT_FLAG_TRIED) because now even the 2nd try will have the ALLOW_RETRY set, then use that helper in all existing special paths. One example is in __lock_page_or_retry(), now we'll drop the mmap_sem only in the first attempt of page fault and we'll keep it in follow up retries, so old locking behavior will be retained. This will be a nice enhancement for current code [2] at the same time a supporting material for the future userfaultfd-writeprotect work, since in that work there will always be an explicit userfault writeprotect retry for protected pages, and if that cannot resolve the page fault (e.g., when userfaultfd-writeprotect is used in conjunction with swapped pages) then we'll possibly need a 3rd retry of the page fault. It might also benefit other potential users who will have similar requirement like userfault write-protection. GUP code is not touched yet and will be covered in follow up patch. Please read the thread below for more information. [1] https://lore.kernel.org/lkml/[email protected]/ [2] https://lore.kernel.org/lkml/[email protected]/ Suggested-by: Linus Torvalds <[email protected]> Suggested-by: Andrea Arcangeli <[email protected]> Signed-off-by: Peter Xu <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Tested-by: Brian Geffon <[email protected]> Cc: Bobby Powers <[email protected]> Cc: David Hildenbrand <[email protected]> Cc: Denis Plotnikov <[email protected]> Cc: "Dr . David Alan Gilbert" <[email protected]> Cc: Hugh Dickins <[email protected]> Cc: Jerome Glisse <[email protected]> Cc: Johannes Weiner <[email protected]> Cc: "Kirill A . Shutemov" <[email protected]> Cc: Martin Cracauer <[email protected]> Cc: Marty McFadden <[email protected]> Cc: Matthew Wilcox <[email protected]> Cc: Maya Gokhale <[email protected]> Cc: Mel Gorman <[email protected]> Cc: Mike Kravetz <[email protected]> Cc: Mike Rapoport <[email protected]> Cc: Pavel Emelyanov <[email protected]> Link: http://lkml.kernel.org/r/[email protected] Signed-off-by: Linus Torvalds <[email protected]>
1 parent c270a7e commit 4064b98

File tree

27 files changed

+54
-57
lines changed

27 files changed

+54
-57
lines changed

arch/alpha/mm/fault.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ do_page_fault(unsigned long address, unsigned long mmcsr,
169169
else
170170
current->min_flt++;
171171
if (fault & VM_FAULT_RETRY) {
172-
flags &= ~FAULT_FLAG_ALLOW_RETRY;
172+
flags |= FAULT_FLAG_TRIED;
173173

174174
/* No need to up_read(&mm->mmap_sem) as we would
175175
* have already released it in __lock_page_or_retry

arch/arc/mm/fault.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
145145
*/
146146
if (unlikely((fault & VM_FAULT_RETRY) &&
147147
(flags & FAULT_FLAG_ALLOW_RETRY))) {
148-
flags &= ~FAULT_FLAG_ALLOW_RETRY;
149148
flags |= FAULT_FLAG_TRIED;
150149
goto retry;
151150
}

arch/arm/mm/fault.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -319,9 +319,6 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
319319
regs, addr);
320320
}
321321
if (fault & VM_FAULT_RETRY) {
322-
/* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk
323-
* of starvation. */
324-
flags &= ~FAULT_FLAG_ALLOW_RETRY;
325322
flags |= FAULT_FLAG_TRIED;
326323
goto retry;
327324
}

arch/arm64/mm/fault.c

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -521,12 +521,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
521521
}
522522

523523
if (fault & VM_FAULT_RETRY) {
524-
/*
525-
* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk of
526-
* starvation.
527-
*/
528524
if (mm_flags & FAULT_FLAG_ALLOW_RETRY) {
529-
mm_flags &= ~FAULT_FLAG_ALLOW_RETRY;
530525
mm_flags |= FAULT_FLAG_TRIED;
531526
goto retry;
532527
}

arch/hexagon/mm/vm_fault.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ void do_page_fault(unsigned long address, long cause, struct pt_regs *regs)
102102
else
103103
current->min_flt++;
104104
if (fault & VM_FAULT_RETRY) {
105-
flags &= ~FAULT_FLAG_ALLOW_RETRY;
106105
flags |= FAULT_FLAG_TRIED;
107106
goto retry;
108107
}

arch/ia64/mm/fault.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,6 @@ ia64_do_page_fault (unsigned long address, unsigned long isr, struct pt_regs *re
167167
else
168168
current->min_flt++;
169169
if (fault & VM_FAULT_RETRY) {
170-
flags &= ~FAULT_FLAG_ALLOW_RETRY;
171170
flags |= FAULT_FLAG_TRIED;
172171

173172
/* No need to up_read(&mm->mmap_sem) as we would

arch/m68k/mm/fault.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,6 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
162162
else
163163
current->min_flt++;
164164
if (fault & VM_FAULT_RETRY) {
165-
/* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk
166-
* of starvation. */
167-
flags &= ~FAULT_FLAG_ALLOW_RETRY;
168165
flags |= FAULT_FLAG_TRIED;
169166

170167
/*

arch/microblaze/mm/fault.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,6 @@ void do_page_fault(struct pt_regs *regs, unsigned long address,
236236
else
237237
current->min_flt++;
238238
if (fault & VM_FAULT_RETRY) {
239-
flags &= ~FAULT_FLAG_ALLOW_RETRY;
240239
flags |= FAULT_FLAG_TRIED;
241240

242241
/*

arch/mips/mm/fault.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,6 @@ static void __kprobes __do_page_fault(struct pt_regs *regs, unsigned long write,
178178
tsk->min_flt++;
179179
}
180180
if (fault & VM_FAULT_RETRY) {
181-
flags &= ~FAULT_FLAG_ALLOW_RETRY;
182181
flags |= FAULT_FLAG_TRIED;
183182

184183
/*

arch/nds32/mm/fault.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,6 @@ void do_page_fault(unsigned long entry, unsigned long addr,
246246
1, regs, addr);
247247
}
248248
if (fault & VM_FAULT_RETRY) {
249-
flags &= ~FAULT_FLAG_ALLOW_RETRY;
250249
flags |= FAULT_FLAG_TRIED;
251250

252251
/* No need to up_read(&mm->mmap_sem) as we would

0 commit comments

Comments
 (0)