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

Groups > linux.kernel > #1672405 > unrolled thread

Re: [PATCH v4 5/7] kasan: allow kasan_check_read/write() to accept pointers to volatiles

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 v4 5/7] kasan: allow kasan_check_read/write() to accept  pointers to volatiles Ingo Molnar <mingo@kernel.org> - 2017-06-22 10:30 +0200
    Re: [PATCH v4 5/7] kasan: allow kasan_check_read/write() to accept  pointers to volatiles Dmitry Vyukov <dvyukov@google.com> - 2017-06-22 16:20 +0200

#1672405 — Re: [PATCH v4 5/7] kasan: allow kasan_check_read/write() to accept pointers to volatiles

* Dmitry Vyukov <dvyukov@google.com> wrote:

> On Mon, Jun 19, 2017 at 12:50 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Sat, Jun 17, 2017 at 11:15:31AM +0200, Dmitry Vyukov wrote:
> >> Currently kasan_check_read/write() accept 'const void*', make them
> >> accept 'const volatile void*'. This is required for instrumentation
> >> of atomic operations and there is just no reason to not allow that.
> >>
> >> Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
> >> Cc: Mark Rutland <mark.rutland@arm.com>
> >> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> >> Cc: Thomas Gleixner <tglx@linutronix.de>
> >> Cc: "H. Peter Anvin" <hpa@zytor.com>
> >> Cc: Peter Zijlstra <peterz@infradead.org>
> >> Cc: Andrew Morton <akpm@linux-foundation.org>
> >> Cc: linux-kernel@vger.kernel.org
> >> Cc: x86@kernel.org
> >> Cc: linux-mm@kvack.org
> >> Cc: kasan-dev@googlegroups.com
> >
> > Looks sane to me, and I can confirm this doesn't advervsely affect
> > arm64. FWIW:
> >
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
> >
> > Mark.
> 
> 
> Great! Thanks for testing.
> 
> Ingo, what are your thoughts? Are you taking this to locking tree? When?

Yeah, it all looks pretty clean to me too. I've applied the first three patches to 
the locking tree, but did some minor stylistic cleanups to the first patch to 
harmonize the style of the code - which made the later patches not apply cleanly.

Mind sending the remaining patches against the locking tree, tip:locking/core? 
(Please also add in all the acks you got.)

This should also give people (Peter, Linus?) a last minute chance to object to my 
suggestion of increasing the linecount in patch #1:

 0f2376eb0ff8: locking/atomic/x86: Un-macro-ify atomic ops implementation

 arch/x86/include/asm/atomic.h      | 69 ++++++++++++++++++++++++++++++++++++++++++++++-----------------------
 arch/x86/include/asm/atomic64_32.h | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------
 arch/x86/include/asm/atomic64_64.h | 67 ++++++++++++++++++++++++++++++++++++++++++++-----------------------
 3 files changed, 147 insertions(+), 70 deletions(-)

... to me the end result looks much more readable despite the +70 lines of code, 
but if anyone feels strongly about this please holler!

Thanks,

	Ingo

[toc] | [next] | [standalone]

#1672667

On Thu, Jun 22, 2017 at 10:25 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Dmitry Vyukov <dvyukov@google.com> wrote:
>
>> On Mon, Jun 19, 2017 at 12:50 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Sat, Jun 17, 2017 at 11:15:31AM +0200, Dmitry Vyukov wrote:
>> >> Currently kasan_check_read/write() accept 'const void*', make them
>> >> accept 'const volatile void*'. This is required for instrumentation
>> >> of atomic operations and there is just no reason to not allow that.
>> >>
>> >> Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
>> >> Cc: Mark Rutland <mark.rutland@arm.com>
>> >> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
>> >> Cc: Thomas Gleixner <tglx@linutronix.de>
>> >> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> >> Cc: Peter Zijlstra <peterz@infradead.org>
>> >> Cc: Andrew Morton <akpm@linux-foundation.org>
>> >> Cc: linux-kernel@vger.kernel.org
>> >> Cc: x86@kernel.org
>> >> Cc: linux-mm@kvack.org
>> >> Cc: kasan-dev@googlegroups.com
>> >
>> > Looks sane to me, and I can confirm this doesn't advervsely affect
>> > arm64. FWIW:
>> >
>> > Acked-by: Mark Rutland <mark.rutland@arm.com>
>> >
>> > Mark.
>>
>>
>> Great! Thanks for testing.
>>
>> Ingo, what are your thoughts? Are you taking this to locking tree? When?
>
> Yeah, it all looks pretty clean to me too. I've applied the first three patches to
> the locking tree, but did some minor stylistic cleanups to the first patch to
> harmonize the style of the code - which made the later patches not apply cleanly.
>
> Mind sending the remaining patches against the locking tree, tip:locking/core?
> (Please also add in all the acks you got.)

Mailed v5 rebased on tip:locking/core (now only 4 patches).
Added Acked/Reviewed-By that I got.

> This should also give people (Peter, Linus?) a last minute chance to object to my
> suggestion of increasing the linecount in patch #1:
>
>  0f2376eb0ff8: locking/atomic/x86: Un-macro-ify atomic ops implementation
>
>  arch/x86/include/asm/atomic.h      | 69 ++++++++++++++++++++++++++++++++++++++++++++++-----------------------
>  arch/x86/include/asm/atomic64_32.h | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------
>  arch/x86/include/asm/atomic64_64.h | 67 ++++++++++++++++++++++++++++++++++++++++++++-----------------------
>  3 files changed, 147 insertions(+), 70 deletions(-)
>
> ... to me the end result looks much more readable despite the +70 lines of code,
> but if anyone feels strongly about this please holler!
>
> Thanks,
>
>         Ingo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web