diff mbox series

[1/9] arm64: assembler: add cond_yield macro

Message ID 20210128130625.54076-2-ardb@kernel.org (mailing list archive)
State New, archived
Headers show
Series arm64: rework NEON yielding to avoid scheduling from asm code | expand

Commit Message

Ard Biesheuvel Jan. 28, 2021, 1:06 p.m. UTC
Add a macro cond_yield that branches to a specified label when called if
the TIF_NEED_RESCHED flag is set and decreasing the preempt count would
make the task preemptible again, resulting in a schedule to occur. This
can be used by kernel mode SIMD code that keeps a lot of state in SIMD
registers, which would make chunking the input in order to perform the
cond_resched() check from C code disproportionately costly.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/include/asm/assembler.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Will Deacon Jan. 28, 2021, 8:24 p.m. UTC | #1
On Thu, Jan 28, 2021 at 02:06:17PM +0100, Ard Biesheuvel wrote:
> Add a macro cond_yield that branches to a specified label when called if
> the TIF_NEED_RESCHED flag is set and decreasing the preempt count would
> make the task preemptible again, resulting in a schedule to occur. This
> can be used by kernel mode SIMD code that keeps a lot of state in SIMD
> registers, which would make chunking the input in order to perform the
> cond_resched() check from C code disproportionately costly.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/arm64/include/asm/assembler.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index bf125c591116..5f977a7c6b43 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -745,6 +745,22 @@ USER(\label, ic	ivau, \tmp2)			// invalidate I line PoU
>  .Lyield_out_\@ :
>  	.endm
>  
> +	/*
> +	 * Check whether preempt-disabled code should yield as soon as it
> +	 * is able. This is the case if re-enabling preemption a single
> +	 * time results in a preempt count of zero, and the TIF_NEED_RESCHED
> +	 * flag is set. (Note that the latter is stored negated in the
> +	 * top word of the thread_info::preempt_count field)
> +	 */
> +	.macro		cond_yield, lbl:req, tmp:req
> +#ifdef CONFIG_PREEMPTION
> +	get_current_task \tmp
> +	ldr		\tmp, [\tmp, #TSK_TI_PREEMPT]
> +	cmp		\tmp, #PREEMPT_DISABLE_OFFSET
> +	beq		\lbl

Fancy that, I didn't know the '.' was optional in "b.eq"!

Anyway, a very similar code sequence exists inside if_will_cond_yield_neon,
only it doesn't touch the flags. Can we use that sequence instead, and then
use the new macro from there?

Will
Will Deacon Jan. 28, 2021, 8:25 p.m. UTC | #2
On Thu, Jan 28, 2021 at 08:24:01PM +0000, Will Deacon wrote:
> On Thu, Jan 28, 2021 at 02:06:17PM +0100, Ard Biesheuvel wrote:
> > Add a macro cond_yield that branches to a specified label when called if
> > the TIF_NEED_RESCHED flag is set and decreasing the preempt count would
> > make the task preemptible again, resulting in a schedule to occur. This
> > can be used by kernel mode SIMD code that keeps a lot of state in SIMD
> > registers, which would make chunking the input in order to perform the
> > cond_resched() check from C code disproportionately costly.
> > 
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  arch/arm64/include/asm/assembler.h | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> > index bf125c591116..5f977a7c6b43 100644
> > --- a/arch/arm64/include/asm/assembler.h
> > +++ b/arch/arm64/include/asm/assembler.h
> > @@ -745,6 +745,22 @@ USER(\label, ic	ivau, \tmp2)			// invalidate I line PoU
> >  .Lyield_out_\@ :
> >  	.endm
> >  
> > +	/*
> > +	 * Check whether preempt-disabled code should yield as soon as it
> > +	 * is able. This is the case if re-enabling preemption a single
> > +	 * time results in a preempt count of zero, and the TIF_NEED_RESCHED
> > +	 * flag is set. (Note that the latter is stored negated in the
> > +	 * top word of the thread_info::preempt_count field)
> > +	 */
> > +	.macro		cond_yield, lbl:req, tmp:req
> > +#ifdef CONFIG_PREEMPTION
> > +	get_current_task \tmp
> > +	ldr		\tmp, [\tmp, #TSK_TI_PREEMPT]
> > +	cmp		\tmp, #PREEMPT_DISABLE_OFFSET
> > +	beq		\lbl
> 
> Fancy that, I didn't know the '.' was optional in "b.eq"!
> 
> Anyway, a very similar code sequence exists inside if_will_cond_yield_neon,
> only it doesn't touch the flags. Can we use that sequence instead, and then
> use the new macro from there?

... and now I noticed the last patch :)

But it would still be nice not to clobber the flags inside the macro.

Will
Ard Biesheuvel Jan. 28, 2021, 8:26 p.m. UTC | #3
On Thu, 28 Jan 2021 at 21:25, Will Deacon <will@kernel.org> wrote:
>
> On Thu, Jan 28, 2021 at 08:24:01PM +0000, Will Deacon wrote:
> > On Thu, Jan 28, 2021 at 02:06:17PM +0100, Ard Biesheuvel wrote:
> > > Add a macro cond_yield that branches to a specified label when called if
> > > the TIF_NEED_RESCHED flag is set and decreasing the preempt count would
> > > make the task preemptible again, resulting in a schedule to occur. This
> > > can be used by kernel mode SIMD code that keeps a lot of state in SIMD
> > > registers, which would make chunking the input in order to perform the
> > > cond_resched() check from C code disproportionately costly.
> > >
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> > >  arch/arm64/include/asm/assembler.h | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > >
> > > diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> > > index bf125c591116..5f977a7c6b43 100644
> > > --- a/arch/arm64/include/asm/assembler.h
> > > +++ b/arch/arm64/include/asm/assembler.h
> > > @@ -745,6 +745,22 @@ USER(\label, ic        ivau, \tmp2)                    // invalidate I line PoU
> > >  .Lyield_out_\@ :
> > >     .endm
> > >
> > > +   /*
> > > +    * Check whether preempt-disabled code should yield as soon as it
> > > +    * is able. This is the case if re-enabling preemption a single
> > > +    * time results in a preempt count of zero, and the TIF_NEED_RESCHED
> > > +    * flag is set. (Note that the latter is stored negated in the
> > > +    * top word of the thread_info::preempt_count field)
> > > +    */
> > > +   .macro          cond_yield, lbl:req, tmp:req
> > > +#ifdef CONFIG_PREEMPTION
> > > +   get_current_task \tmp
> > > +   ldr             \tmp, [\tmp, #TSK_TI_PREEMPT]
> > > +   cmp             \tmp, #PREEMPT_DISABLE_OFFSET
> > > +   beq             \lbl
> >
> > Fancy that, I didn't know the '.' was optional in "b.eq"!
> >
> > Anyway, a very similar code sequence exists inside if_will_cond_yield_neon,
> > only it doesn't touch the flags. Can we use that sequence instead, and then
> > use the new macro from there?
>
> ... and now I noticed the last patch :)
>
> But it would still be nice not to clobber the flags inside the macro.
>

Yeah, that's a good point - I did not consider that. I'll fix that for v2.
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index bf125c591116..5f977a7c6b43 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -745,6 +745,22 @@  USER(\label, ic	ivau, \tmp2)			// invalidate I line PoU
 .Lyield_out_\@ :
 	.endm
 
+	/*
+	 * Check whether preempt-disabled code should yield as soon as it
+	 * is able. This is the case if re-enabling preemption a single
+	 * time results in a preempt count of zero, and the TIF_NEED_RESCHED
+	 * flag is set. (Note that the latter is stored negated in the
+	 * top word of the thread_info::preempt_count field)
+	 */
+	.macro		cond_yield, lbl:req, tmp:req
+#ifdef CONFIG_PREEMPTION
+	get_current_task \tmp
+	ldr		\tmp, [\tmp, #TSK_TI_PREEMPT]
+	cmp		\tmp, #PREEMPT_DISABLE_OFFSET
+	beq		\lbl
+#endif
+	.endm
+
 /*
  * This macro emits a program property note section identifying
  * architecture features which require special handling, mainly for