All of lore.kernel.org
 help / color / mirror / Atom feed
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm64: neon: Fix function may_use_simd() return error status
Date: Wed, 11 Jul 2018 17:03:15 +0100	[thread overview]
Message-ID: <20180711160315.GD18477@arm.com> (raw)
In-Reply-To: <20180711154758.4k2phfrzl2r5m34o@lakrids.cambridge.arm.com>

On Wed, Jul 11, 2018 at 04:47:58PM +0100, Mark Rutland wrote:
> On Wed, Jul 11, 2018 at 09:20:03AM +0200, Ard Biesheuvel wrote:
> > On 11 July 2018 at 03:09, Yandong.Zhao <yandong77520@gmail.com> wrote:
> > > diff --git a/arch/arm64/include/asm/simd.h b/arch/arm64/include/asm/simd.h
> > > index fa8b3fe..784a8c2 100644
> > > --- a/arch/arm64/include/asm/simd.h
> > > +++ b/arch/arm64/include/asm/simd.h
> > > @@ -29,7 +29,8 @@
> > >  static __must_check inline bool may_use_simd(void)
> > >  {
> > >         /*
> > > -        * The raw_cpu_read() is racy if called with preemption enabled.
> > > +        * The this_cpu_read() is racy if called with preemption enabled,
> > > +        * since the task may subsequently migrate to another CPU.
> > >          * This is not a bug: kernel_neon_busy is only set when
> > >          * preemption is disabled, so we cannot migrate to another CPU
> > >          * while it is set, nor can we migrate to a CPU where it is set.
> 
> It would be nice if we could clarify the "is racy" part here.
> 
> How about:
> 
> 	/*
> 	 * kernel_neon_busy is only set while preemption is disabled,
> 	 * and is clear whenever preemption is enabled. Since
> 	 * this_cpu_read() is atomic w.r.t. preemption, kernel_neon_busy
> 	 * cannot change under our feet -- if it's set we cannot be
> 	 * migrated, and if it's clear we cannot be migrated to a CPU
> 	 * where it is set.
> 	 */
> 
> With that:
> 
> Reviewed-by: Mark Rutland <mark.rutland@arm..com>

Thanks. Applied with the updated comment and your tag..

Will

WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Dave Martin <Dave.Martin@arm.com>,
	"Yandong.Zhao" <yandong77520@gmail.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	zhaoyd@thundersoft.com, zhaoxb@thundersoft.com,
	fanlc0801@thundersoft.com
Subject: Re: [PATCH] arm64: neon: Fix function may_use_simd() return error status
Date: Wed, 11 Jul 2018 17:03:15 +0100	[thread overview]
Message-ID: <20180711160315.GD18477@arm.com> (raw)
In-Reply-To: <20180711154758.4k2phfrzl2r5m34o@lakrids.cambridge.arm.com>

On Wed, Jul 11, 2018 at 04:47:58PM +0100, Mark Rutland wrote:
> On Wed, Jul 11, 2018 at 09:20:03AM +0200, Ard Biesheuvel wrote:
> > On 11 July 2018 at 03:09, Yandong.Zhao <yandong77520@gmail.com> wrote:
> > > diff --git a/arch/arm64/include/asm/simd.h b/arch/arm64/include/asm/simd.h
> > > index fa8b3fe..784a8c2 100644
> > > --- a/arch/arm64/include/asm/simd.h
> > > +++ b/arch/arm64/include/asm/simd.h
> > > @@ -29,7 +29,8 @@
> > >  static __must_check inline bool may_use_simd(void)
> > >  {
> > >         /*
> > > -        * The raw_cpu_read() is racy if called with preemption enabled.
> > > +        * The this_cpu_read() is racy if called with preemption enabled,
> > > +        * since the task may subsequently migrate to another CPU.
> > >          * This is not a bug: kernel_neon_busy is only set when
> > >          * preemption is disabled, so we cannot migrate to another CPU
> > >          * while it is set, nor can we migrate to a CPU where it is set.
> 
> It would be nice if we could clarify the "is racy" part here.
> 
> How about:
> 
> 	/*
> 	 * kernel_neon_busy is only set while preemption is disabled,
> 	 * and is clear whenever preemption is enabled. Since
> 	 * this_cpu_read() is atomic w.r.t. preemption, kernel_neon_busy
> 	 * cannot change under our feet -- if it's set we cannot be
> 	 * migrated, and if it's clear we cannot be migrated to a CPU
> 	 * where it is set.
> 	 */
> 
> With that:
> 
> Reviewed-by: Mark Rutland <mark.rutland@arm..com>

Thanks. Applied with the updated comment and your tag..

Will

  reply	other threads:[~2018-07-11 16:03 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-11  1:09 [PATCH] arm64: neon: Fix function may_use_simd() return error status Yandong.Zhao
2018-07-11  1:09 ` Yandong.Zhao
2018-07-11  7:20 ` Ard Biesheuvel
2018-07-11  7:20   ` Ard Biesheuvel
2018-07-11 15:47   ` Mark Rutland
2018-07-11 15:47     ` Mark Rutland
2018-07-11 16:03     ` Will Deacon [this message]
2018-07-11 16:03       ` Will Deacon
2018-07-11 16:07       ` Mark Rutland
2018-07-11 16:07         ` Mark Rutland
2018-07-11 10:55 ` Dave Martin
2018-07-11 10:55   ` Dave Martin
  -- strict thread matches above, loose matches on Subject: below --
2018-07-12  3:29 Yandong.Zhao
2018-07-12  3:29 ` Yandong.Zhao
2018-07-12  9:56 ` Dave Martin
2018-07-12  9:56   ` Dave Martin
2018-07-11 11:06 Yandong.Zhao
2018-07-11 11:06 ` Yandong.Zhao
2018-07-11 12:05 ` Will Deacon
2018-07-11 12:05   ` Will Deacon
2018-07-11 12:58   ` Dave Martin
2018-07-11 12:58     ` Dave Martin
2018-07-10  2:21 Yandong.Zhao
2018-07-10  2:21 ` Yandong.Zhao
2018-07-10 13:11 ` Dave Martin
2018-07-10 13:11   ` Dave Martin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180711160315.GD18477@arm.com \
    --to=will.deacon@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.