lkml.org 
[lkml]   [2017]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/3] asm-generic, x86: wrap atomic operations
    On Tue, Mar 21, 2017 at 12:25:06PM +0300, Andrey Ryabinin wrote:
    > On 03/20/2017 08:17 PM, Mark Rutland wrote:
    > > Hi,
    > >
    > > On Tue, Mar 14, 2017 at 08:24:13PM +0100, Dmitry Vyukov wrote:
    > >> /**
    > >> - * atomic_read - read atomic variable
    > >> + * arch_atomic_read - read atomic variable
    > >> * @v: pointer of type atomic_t
    > >> *
    > >> * Atomically reads the value of @v.
    > >> */
    > >> -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);
    > >> + /*
    > >> + * We use READ_ONCE_NOCHECK() because atomic_read() contains KASAN
    > >> + * instrumentation. Double instrumentation is unnecessary.
    > >> + */
    > >> + return READ_ONCE_NOCHECK((v)->counter);
    > >> }
    > >
    > > Just to check, we do this to avoid duplicate reports, right?
    > >
    > > If so, double instrumentation isn't solely "unnecessary"; it has a
    > > functional difference, and we should explicitly describe that in the
    > > comment.
    > >
    > > ... or are duplicate reports supressed somehow?
    >
    > They are not suppressed yet. But I think we should just switch kasan
    > to single shot mode, i.e. report only the first error. Single bug
    > quite often has multiple invalid memory accesses causing storm in
    > dmesg. Also write OOB might corrupt metadata so the next report will
    > print bogus alloc/free stacktraces.
    > In most cases we need to look only at the first report, so reporting
    > anything after the first is just counterproductive.

    FWIW, that sounds sane to me.

    Given that, I agree with your comment regarding READ_ONCE{,_NOCHECK}().

    If anyone really wants all the reports, we could have a boot-time option
    to do that.

    Thanks,
    Mark.

    \
     
     \ /
      Last update: 2017-03-21 11:42    [W:3.177 / U:0.160 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site