On Mon, Dec 07, 2020 at 04:08:46PM -0300, Martin Pieuchot wrote:
> Diff below rewrites uvm_fault() using a loop.
>
> I added a KERNEL_LOCK/UNLOCK() dance around the part that won't be
> unlocked soon to illustrate where this is going.
>
> ok?
yes, ok jmatthew@
>
> Index: uvm/uvm_fault.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
> retrieving revision 1.108
> diff -u -p -r1.108 uvm_fault.c
> --- uvm/uvm_fault.c 19 Nov 2020 17:06:40 -0000 1.108
> +++ uvm/uvm_fault.c 7 Dec 2020 18:20:16 -0000
> @@ -907,7 +907,7 @@ uvm_fault(vm_map_t orig_map, vaddr_t vad
> boolean_t shadowed;
> struct vm_anon *anons_store[UVM_MAXRANGE], **anons;
> struct vm_page *pages[UVM_MAXRANGE];
> - int error;
> + int error = ERESTART;
>
> uvmexp.faults++; /* XXX: locking? */
> TRACEPOINT(uvm, fault, vaddr, fault_type, access_type, NULL);
> @@ -923,43 +923,32 @@ uvm_fault(vm_map_t orig_map, vaddr_t vad
> flt.narrow = FALSE; /* normal fault */
>
>
> - /* "goto ReFault" means restart the page fault from ground zero. */
> -ReFault:
> - anons = anons_store;
> -
> - error = uvm_fault_check(&ufi, &flt, &anons, access_type);
> - switch (error) {
> - case 0:
> - break;
> - case ERESTART:
> - goto ReFault;
> - default:
> - return error;
> - }
> -
> - /* (shadowed == TRUE) if there is an anon at the faulting address */
> - shadowed = uvm_fault_upper_lookup(&ufi, &flt, anons, pages);
> -
> - /* handle case 1: fault on an anon in our amap */
> - if (shadowed == TRUE) {
> - error = uvm_fault_upper(&ufi, &flt, anons, fault_type,
> - access_type);
> - switch (error) {
> - case ERESTART:
> - goto ReFault;
> - default:
> - return error;
> + /*
> + * ReFault
> + */
> + while (error == ERESTART) {
> + anons = anons_store;
> +
> + error = uvm_fault_check(&ufi, &flt, &anons, access_type);
> + if (error != 0)
> + continue;
> +
> + /* True if there is an anon at the faulting address */
> + shadowed = uvm_fault_upper_lookup(&ufi, &flt, anons, pages);
> + if (shadowed == TRUE) {
> + /* case 1: fault on an anon in our amap */
> + error = uvm_fault_upper(&ufi, &flt, anons, fault_type,
> + access_type);
> + } else {
> + /* case 2: fault on backing object or zero fill */
> + KERNEL_LOCK();
> + error = uvm_fault_lower(&ufi, &flt, pages, fault_type,
> + access_type);
> + KERNEL_UNLOCK();
> }
> }
>
> - /* handle case 2: faulting on backing object or zero fill */
> - error = uvm_fault_lower(&ufi, &flt, pages, fault_type, access_type);
> - switch (error) {
> - case ERESTART:
> - goto ReFault;
> - default:
> - return error;
> - }
> + return error;
> }
>
> int
>
