From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave.Martin@arm.com (Dave Martin) Date: Wed, 6 Dec 2017 14:37:57 +0000 Subject: [PATCH v2 11/19] arm64: assembler: add macro to conditionally yield the NEON under PREEMPT In-Reply-To: References: <20171204122645.31535-1-ard.biesheuvel@linaro.org> <20171204122645.31535-12-ard.biesheuvel@linaro.org> <20171205122757.GW22781@e103592.cambridge.arm.com> <20171206115134.GZ22781@e103592.cambridge.arm.com> <20171206121251.GM12608@e103592.cambridge.arm.com> Message-ID: <20171206143756.GC22781@e103592.cambridge.arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, Dec 06, 2017 at 12:25:44PM +0000, Ard Biesheuvel wrote: > On 6 December 2017 at 12:12, Dave P Martin wrote: > > On Wed, Dec 06, 2017 at 11:57:12AM +0000, Ard Biesheuvel wrote: > >> On 6 December 2017 at 11:51, Dave Martin wrote: > >> > On Tue, Dec 05, 2017 at 06:04:34PM +0000, Ard Biesheuvel wrote: > >> >> On 5 December 2017 at 12:45, Ard Biesheuvel wrote: > >> >> > > >> >> > > >> >> >> On 5 Dec 2017, at 12:28, Dave Martin 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