Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1600612 > unrolled thread

Re: [PATCH] x86, kasan: add KASAN checks to atomic operations

Started byPeter Zijlstra <peterz@infradead.org>
First post2017-03-14 16:40 +0100
Last post2017-03-14 20:30 +0100
Articles 3 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH] x86, kasan: add KASAN checks to atomic operations Peter Zijlstra <peterz@infradead.org> - 2017-03-14 16:40 +0100
    Re: [PATCH] x86, kasan: add KASAN checks to atomic operations Mark Rutland <mark.rutland@arm.com> - 2017-03-14 16:50 +0100
      Re: [PATCH] x86, kasan: add KASAN checks to atomic operations Dmitry Vyukov <dvyukov@google.com> - 2017-03-14 20:30 +0100

#1600612 — Re: [PATCH] x86, kasan: add KASAN checks to atomic operations

FromPeter Zijlstra <peterz@infradead.org>
Date2017-03-14 16:40 +0100
SubjectRe: [PATCH] x86, kasan: add KASAN checks to atomic operations
Message-ID<tkWIi-25L-7@gated-at.bofh.it>
On Tue, Mar 14, 2017 at 04:22:52PM +0100, Dmitry Vyukov wrote:
> -static __always_inline int atomic_read(const atomic_t *v)
> +static __always_inline int arch_atomic_read(const atomic_t *v)
>  {
> -	return READ_ONCE((v)->counter);
> +	return READ_ONCE_NOCHECK((v)->counter);

Should NOCHEKC come with a comment, because i've no idea why this is so.

>  }

[toc] | [next] | [standalone]


#1600627

FromMark Rutland <mark.rutland@arm.com>
Date2017-03-14 16:50 +0100
Message-ID<tkWRX-29k-3@gated-at.bofh.it>
In reply to#1600612
On Tue, Mar 14, 2017 at 04:32:30PM +0100, Peter Zijlstra wrote:
> On Tue, Mar 14, 2017 at 04:22:52PM +0100, Dmitry Vyukov wrote:
> > -static __always_inline int atomic_read(const atomic_t *v)
> > +static __always_inline int arch_atomic_read(const atomic_t *v)
> >  {
> > -	return READ_ONCE((v)->counter);
> > +	return READ_ONCE_NOCHECK((v)->counter);
> 
> Should NOCHEKC come with a comment, because i've no idea why this is so.

I suspect the idea is that given the wrapper will have done the KASAN
check, duplicating it here is either sub-optimal, or results in
duplicate splats. READ_ONCE() has an implicit KASAN check,
READ_ONCE_NOCHECK() does not.

If this is to solve duplicate splats, it'd be worth having a
WRITE_ONCE_NOCHECK() for arch_atomic_set().

Agreed on the comment, regardless.

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]


#1600841

FromDmitry Vyukov <dvyukov@google.com>
Date2017-03-14 20:30 +0100
Message-ID<tl0iS-4M7-13@gated-at.bofh.it>
In reply to#1600627
On Tue, Mar 14, 2017 at 4:44 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>> > -static __always_inline int atomic_read(const atomic_t *v)
>> > +static __always_inline int arch_atomic_read(const atomic_t *v)
>> >  {
>> > -   return READ_ONCE((v)->counter);
>> > +   return READ_ONCE_NOCHECK((v)->counter);
>>
>> Should NOCHEKC come with a comment, because i've no idea why this is so.
>
> I suspect the idea is that given the wrapper will have done the KASAN
> check, duplicating it here is either sub-optimal, or results in
> duplicate splats. READ_ONCE() has an implicit KASAN check,
> READ_ONCE_NOCHECK() does not.
>
> If this is to solve duplicate splats, it'd be worth having a
> WRITE_ONCE_NOCHECK() for arch_atomic_set().
>
> Agreed on the comment, regardless.


Reverted xchg changes.
Added comments re READ_ONCE_NOCHECK() and WRITE_ONCE().
Added file comment.
Split into 3 patches and mailed.

Thanks!

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web