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@xxxxxxxxxxxxx> 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.
