From: Mark Rutland <mark.rutland@arm.com>
To: Marco Elver <elver@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
"Paul E. McKenney" <paulmck@kernel.org>,
Will Deacon <will@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
Dmitry Vyukov <dvyukov@google.com>,
Alexander Potapenko <glider@google.com>,
kasan-dev <kasan-dev@googlegroups.com>,
LKML <linux-kernel@vger.kernel.org>,
linux-arch <linux-arch@vger.kernel.org>
Subject: Re: [PATCH 8/8] locking/atomics: Use read-write instrumentation for atomic RMWs
Date: Fri, 14 Aug 2020 12:28:26 +0100 [thread overview]
Message-ID: <20200814112826.GB68877@C02TD0UTHF1T.local> (raw)
In-Reply-To: <CANpmjNM6C6QtrtLhRkbmfc3jLqYaQOvvM_vKA6UyrkWadkdzNQ@mail.gmail.com>
Hi,
Sorry to come to this rather late -- this comment equally applies to v2
so I'm replying here to have context.
On Wed, Jul 22, 2020 at 12:11:18PM +0200, Marco Elver wrote:
> On Tue, 21 Jul 2020 at 16:19, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Tue, Jul 21, 2020 at 12:30:16PM +0200, Marco Elver wrote:
> >
> > > diff --git a/scripts/atomic/gen-atomic-instrumented.sh b/scripts/atomic/gen-atomic-instrumented.sh
> > > index 6afadf73da17..5cdcce703660 100755
> > > --- a/scripts/atomic/gen-atomic-instrumented.sh
> > > +++ b/scripts/atomic/gen-atomic-instrumented.sh
> > > @@ -5,9 +5,10 @@ ATOMICDIR=$(dirname $0)
> > >
> > > . ${ATOMICDIR}/atomic-tbl.sh
> > >
> > > -#gen_param_check(arg)
> > > +#gen_param_check(meta, arg)
> > > gen_param_check()
> > > {
> > > + local meta="$1"; shift
> > > local arg="$1"; shift
> > > local type="${arg%%:*}"
> > > local name="$(gen_param_name "${arg}")"
> > > @@ -17,17 +18,24 @@ gen_param_check()
> > > i) return;;
> > > esac
> > >
> > > - # We don't write to constant parameters
> > > - [ ${type#c} != ${type} ] && rw="read"
> > > + if [ ${type#c} != ${type} ]; then
> > > + # We don't write to constant parameters
> > > + rw="read"
> > > + elif [ "${meta}" != "s" ]; then
> > > + # Atomic RMW
> > > + rw="read_write"
> > > + fi
> >
> > If we have meta, should we then not be consistent and use it for read
> > too? Mark?
>
> gen_param_check seems to want to generate an 'instrument_' check per
> pointer argument. So if we have 1 argument that is a constant pointer,
> and one that isn't, it should generate different instrumentation for
> each. By checking the argument type, we get that behaviour. Although
> we are making the assumption that if meta indicates it's not a 's'tore
> (with void return), it's always a read-write access on all non-const
> pointers.
>
> Switching over to checking only meta would always generate the same
> 'instrument_' call for each argument. Although right now that would
> seem to work because we don't yet have an atomic that accepts a
> constant pointer and a non-const one.
>
> Preferences?
Given the only non-rmw cases use the 'l' and 's' meta values, and those
only have a single argument, I reckon it's preferable to special-case
those specifically, e.g.
case "{meta}" in
l) rw="read";;
s) rw="write";;
*) rw="read_write";;
esac
... then we can rework that in future if we ever need to handle multiple
atomic variables that have distinct r/w/rw access types.
Thanks,
Mark.
next prev parent reply other threads:[~2020-08-14 11:28 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-21 10:30 [PATCH 0/8] kcsan: Compound read-write instrumentation Marco Elver
2020-07-21 10:30 ` [PATCH 1/8] kcsan: Support compounded " Marco Elver
2020-07-21 10:30 ` Marco Elver
2020-07-21 10:30 ` [PATCH 2/8] objtool, kcsan: Add __tsan_read_write to uaccess whitelist Marco Elver
2020-07-21 14:04 ` Peter Zijlstra
2020-07-21 10:30 ` [PATCH 3/8] kcsan: Skew delay to be longer for certain access types Marco Elver
2020-07-21 10:30 ` Marco Elver
2020-07-21 14:05 ` Peter Zijlstra
2020-07-21 14:26 ` Marco Elver
2020-07-21 14:34 ` peterz
2020-07-21 10:30 ` [PATCH 4/8] kcsan: Add missing CONFIG_KCSAN_IGNORE_ATOMICS checks Marco Elver
2020-07-21 14:09 ` Peter Zijlstra
2020-07-21 14:21 ` Marco Elver
2020-07-21 10:30 ` [PATCH 5/8] kcsan: Test support for compound instrumentation Marco Elver
2020-07-21 10:30 ` Marco Elver
2020-07-21 11:06 ` Marco Elver
2020-07-21 10:30 ` [PATCH 6/8] instrumented.h: Introduce read-write instrumentation hooks Marco Elver
2020-07-21 10:30 ` [PATCH 7/8] asm-generic/bitops: Use instrument_read_write() where appropriate Marco Elver
2020-07-21 10:30 ` Marco Elver
2020-07-21 10:30 ` [PATCH 8/8] locking/atomics: Use read-write instrumentation for atomic RMWs Marco Elver
2020-07-21 14:18 ` Peter Zijlstra
2020-07-21 14:18 ` Peter Zijlstra
2020-07-22 10:11 ` Marco Elver
2020-08-14 11:28 ` Mark Rutland [this message]
2020-08-14 11:31 ` Mark Rutland
2020-08-14 11:59 ` Marco Elver
2020-08-14 12:34 ` Mark Rutland
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=20200814112826.GB68877@C02TD0UTHF1T.local \
--to=mark.rutland@arm.com \
--cc=arnd@arndb.de \
--cc=dvyukov@google.com \
--cc=elver@google.com \
--cc=glider@google.com \
--cc=kasan-dev@googlegroups.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=paulmck@kernel.org \
--cc=peterz@infradead.org \
--cc=will@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).