Re: Call uvm_grow() on armv7
On 2020 Sep 25 (Fri) at 14:51:01 +0200 (+0200), Mark Kettenis wrote: :> Date: Fri, 25 Sep 2020 14:38:23 +0200 :> From: Peter Hessler :> :> After Mark noticed that arm64 didn't have it, I checked armv7 and it :> also doesn't have it. :> :> Successfully tested on a Tinker-RK3288 :> :> OK? :> :> :> Index: sys/arch/arm/arm/fault.c :> === :> RCS file: /home/cvs/openbsd/src/sys/arch/arm/arm/fault.c,v :> retrieving revision 1.41 :> diff -u -p -u -p -r1.41 fault.c :> --- sys/arch/arm/arm/fault.c 14 Sep 2020 18:23:32 - 1.41 :> +++ sys/arch/arm/arm/fault.c 25 Sep 2020 06:21:48 - :> @@ -331,6 +331,8 @@ data_abort_handler(trapframe_t *tf) :> pcb->pcb_onfault = NULL; :> KERNEL_LOCK(); :> error = uvm_fault(map, va, 0, ftype); :> +if (error == 0) : :You need a map != kernel_map check here as this trap can be called for :both kernel faults and userland faults. : :> +uvm_grow(p, va); :> KERNEL_UNLOCK(); :> pcb->pcb_onfault = onfault; :> :> @@ -588,6 +590,8 @@ prefetch_abort_handler(trapframe_t *tf) :> :> KERNEL_LOCK(); :> error = uvm_fault(map, va, 0, PROT_READ | PROT_EXEC); :> +if (error == 0) : :But not here since this one always has map != kernel_map. : :> +uvm_grow(p, va); :> KERNEL_UNLOCK(); :> if (__predict_true(error == 0)) :> goto out; :> Thanks, fixed. Index: sys/arch/arm/arm/fault.c === RCS file: /home/cvs/openbsd/src/sys/arch/arm/arm/fault.c,v retrieving revision 1.41 diff -u -p -u -p -r1.41 fault.c --- sys/arch/arm/arm/fault.c14 Sep 2020 18:23:32 - 1.41 +++ sys/arch/arm/arm/fault.c25 Sep 2020 13:16:55 - @@ -331,6 +331,8 @@ data_abort_handler(trapframe_t *tf) pcb->pcb_onfault = NULL; KERNEL_LOCK(); error = uvm_fault(map, va, 0, ftype); + if (error == 0 && map != kernel_map) + uvm_grow(p, va); KERNEL_UNLOCK(); pcb->pcb_onfault = onfault; @@ -588,6 +590,8 @@ prefetch_abort_handler(trapframe_t *tf) KERNEL_LOCK(); error = uvm_fault(map, va, 0, PROT_READ | PROT_EXEC); + if (error == 0) + uvm_grow(p, va); KERNEL_UNLOCK(); if (__predict_true(error == 0)) goto out; :> :> :> On 2020 Sep 24 (Thu) at 23:16:08 +0200 (+0200), Mark Kettenis wrote: :> :The call is missing from the trap handler, probably because I was :> :looking at arm64 where it is missing as well. The result is that the :> :stack size accounting will be wrong. :> : :> :In the diff below I only added the call to the "data" trap. That :> :means that an "instruction" trap will not run the accounting code. Is :> :that correct? The uvm_fault() call should never return success in :> :that case unless the stack has been mapped executable... :> : -- Millihelen, adj: The amount of beauty required to launch one ship.
Re: Call uvm_grow() on armv7
> Date: Fri, 25 Sep 2020 14:38:23 +0200 > From: Peter Hessler > > After Mark noticed that arm64 didn't have it, I checked armv7 and it > also doesn't have it. > > Successfully tested on a Tinker-RK3288 > > OK? > > > Index: sys/arch/arm/arm/fault.c > === > RCS file: /home/cvs/openbsd/src/sys/arch/arm/arm/fault.c,v > retrieving revision 1.41 > diff -u -p -u -p -r1.41 fault.c > --- sys/arch/arm/arm/fault.c 14 Sep 2020 18:23:32 - 1.41 > +++ sys/arch/arm/arm/fault.c 25 Sep 2020 06:21:48 - > @@ -331,6 +331,8 @@ data_abort_handler(trapframe_t *tf) > pcb->pcb_onfault = NULL; > KERNEL_LOCK(); > error = uvm_fault(map, va, 0, ftype); > + if (error == 0) You need a map != kernel_map check here as this trap can be called for both kernel faults and userland faults. > + uvm_grow(p, va); > KERNEL_UNLOCK(); > pcb->pcb_onfault = onfault; > > @@ -588,6 +590,8 @@ prefetch_abort_handler(trapframe_t *tf) > > KERNEL_LOCK(); > error = uvm_fault(map, va, 0, PROT_READ | PROT_EXEC); > + if (error == 0) But not here since this one always has map != kernel_map. > + uvm_grow(p, va); > KERNEL_UNLOCK(); > if (__predict_true(error == 0)) > goto out; > > > > On 2020 Sep 24 (Thu) at 23:16:08 +0200 (+0200), Mark Kettenis wrote: > :The call is missing from the trap handler, probably because I was > :looking at arm64 where it is missing as well. The result is that the > :stack size accounting will be wrong. > : > :In the diff below I only added the call to the "data" trap. That > :means that an "instruction" trap will not run the accounting code. Is > :that correct? The uvm_fault() call should never return success in > :that case unless the stack has been mapped executable... > : > > > -- > Nature is by and large to be found out of doors, a location where, it > cannot be argued, there are never enough comfortable chairs. > -- Fran Leibowitz >
Call uvm_grow() on armv7
After Mark noticed that arm64 didn't have it, I checked armv7 and it also doesn't have it. Successfully tested on a Tinker-RK3288 OK? Index: sys/arch/arm/arm/fault.c === RCS file: /home/cvs/openbsd/src/sys/arch/arm/arm/fault.c,v retrieving revision 1.41 diff -u -p -u -p -r1.41 fault.c --- sys/arch/arm/arm/fault.c14 Sep 2020 18:23:32 - 1.41 +++ sys/arch/arm/arm/fault.c25 Sep 2020 06:21:48 - @@ -331,6 +331,8 @@ data_abort_handler(trapframe_t *tf) pcb->pcb_onfault = NULL; KERNEL_LOCK(); error = uvm_fault(map, va, 0, ftype); + if (error == 0) + uvm_grow(p, va); KERNEL_UNLOCK(); pcb->pcb_onfault = onfault; @@ -588,6 +590,8 @@ prefetch_abort_handler(trapframe_t *tf) KERNEL_LOCK(); error = uvm_fault(map, va, 0, PROT_READ | PROT_EXEC); + if (error == 0) + uvm_grow(p, va); KERNEL_UNLOCK(); if (__predict_true(error == 0)) goto out; On 2020 Sep 24 (Thu) at 23:16:08 +0200 (+0200), Mark Kettenis wrote: :The call is missing from the trap handler, probably because I was :looking at arm64 where it is missing as well. The result is that the :stack size accounting will be wrong. : :In the diff below I only added the call to the "data" trap. That :means that an "instruction" trap will not run the accounting code. Is :that correct? The uvm_fault() call should never return success in :that case unless the stack has been mapped executable... : -- Nature is by and large to be found out of doors, a location where, it cannot be argued, there are never enough comfortable chairs. -- Fran Leibowitz
