On Wed, Dec 06, 2017 at 12:25:44PM +0000, Ard Biesheuvel wrote: > On 6 December 2017 at 12:12, Dave P Martin <Dave.Martin@xxxxxxx> wrote: > > On Wed, Dec 06, 2017 at 11:57:12AM +0000, Ard Biesheuvel wrote: > >> On 6 December 2017 at 11:51, Dave Martin <Dave.Martin@xxxxxxx> wrote: > >> > On Tue, Dec 05, 2017 at 06:04:34PM +0000, Ard Biesheuvel wrote: > >> >> On 5 December 2017 at 12:45, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote: > >> >> > > >> >> > > >> >> >> On 5 Dec 2017, at 12:28, Dave Martin <Dave.Martin@xxxxxxx> wrote: [...] > >> >> >> If we had > >> >> >> > >> >> >> if_cond_yield_neon > >> >> >> // patchup code > >> >> >> endif_yield_neon > >> >> >> > >> >> > >> >> I like this, but ... > >> >> > >> >> >> then the caller is free to conditionally branch over that as appropriate > >> >> >> like > >> >> >> > >> >> >> loop: > >> >> >> // crypto stuff > >> >> >> tst x0, #0xf > >> >> >> b.ne loop > >> >> >> > >> >> >> if_cond_yield_neon > >> >> >> // patchup code > >> >> >> endif_cond_yield_neon > >> >> >> > >> >> > >> >> I need to put the post patchup code somewhere too. Please refer to the > >> >> CRC patches for the best examples of this. > >> > > >> > I'm not sure I follow. If you need to do something different after > >> > patchup, can't you either pull that code into the if...endif too, or > >> > otherwise put a branch before the endif? > >> > > >> > >> No, not really. > >> > >> What I currently have is > >> > >> > if_cond_yield_neon > >> > // patchup code > >> > endif_cond_yield_neon > >> > >> which is being turned into > >> > >> get_thread_info x0 > >> ldr w1, [x0, #TSK_TI_PREEMPT] > >> ldr x0, [x0, #TSK_TI_FLAGS] > >> cmp w1, #1 // == PREEMPT_OFFSET > >> csel x0, x0, xzr, eq > >> tbnz x0, #TIF_NEED_RESCHED, 5555f // needs rescheduling? > >> 4444: > >> > >> .subsection 1 > >> 5555: > >> // patchup code > >> bl kernel_neon_end > >> bl kernel_neon_begin > >> // post patchup code goes here > >> b 4444b > >> .subsection > >> > >> so what I basically need is a place to put the post patchup code, > >> unless I open code it and branch to a different label right after > >> kernel_neon_begin > > > > Ah, I get you. I had convinced myself that there was only post- > > patchup code. > > > > So you conceptually want something like > > > > if_will_cond_yield_neon > > // pre-yield patchup code > > do_cond_yield_neon > > // post-yield patchup code > > endif_yield_neon > > > > .macro if_will_cond_yield_neon > > // do preempt_count == 1 && TIF_NEED_RESCHED check > > b.wont_yield 3432f > > .endm > > > > .macro do_cond_yield_neon > > bl kernel_neon_end > > bl kernel_neon_begin > > .endm > > > > .macro endif_yield_neon > > 3432: > > .endm > > > > The main difference I see is that this doesn't get the slow-case > > code out of line. > > > > Or have I still missed something? > > > > No, that seems accurate. I think the idiom looks much better, but I'd > still like to get the code out of line, so that everything disappears > completely for !CONFIG_PREEMPT. Sure, out-of-lining should still be possible by spilling into another section or subsection similarly to what you were already doing -- and it may still make sense to have (optional?) label arguments to allow some branch elision. I didn't want to complicate things initially. > I'll try to rework my stuff using this as a starting point. Cool, let me know. Cheers ---Dave
