From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v2 1/7] tst_atomic: Add load, store and use __atomic builtins
Date: Wed, 30 Aug 2017 15:39:28 +0200 [thread overview]
Message-ID: <20170830133928.GA22607@rei> (raw)
In-Reply-To: <87y3q1b5qx.fsf@our.domain.is.not.set>
Hi!
> > I gave this patch a go on number of old/new distros, x86_64, ppc64le
> > and s390 and I haven't ran into issues. My review is mostly only
> > comparison against kernel sources as much of inline assembly is beyond me.
> >
> > I think we could use a comment about what sort of ordering do we expect
> > from tst_atomic.h. I always assumed we care only for compiler barriers,
> > and when we use __sync_* functions, it's more for convenience (not
> > because we wanted full memory barrier).
>
> I think that is a good idea, I will write some justification and
> explanation in the header.
>
> In my particular use case (tst_fuzzy_sync), I need atleast an
> "acquire-release" memory barrier to ensure that some non-atomic updates
> to shared data between threads are globally visible after a given
> point. I could probably remove them and it would work on most machines
> most of the time. However if a bug does occure due to stale data, I
> think it would be extremely difficult for someone to diagnose.
When I added the atomic add functions I only cared about incrementing
the number of PASS/FAIL/ETC counters in the piece of shared memory used
for test results. So all I cared about was that the value is fetched,
incremented and written atomically.
I guess that we need more than compiler barriers then, since competing
threads may race for the increment we need the value to be coherent
across different CPU caches as well.
> > The rest of fallback functions looks good to me (at least for architectures
> > I checked). I expect fallback is used only by very small fringe of users
> > and their numbers will only go down in future.
> >
> > Regards,
> > Jan
>
> Yes I think so, GCC has had the sync intrinsics for a long time and
> Clang. If anyone is using a compiler without these intrinsics it would
> be interesting to know about it.
Just be vary that some less common architectures got the support much
later than x86.
--
Cyril Hrubis
chrubis@suse.cz
prev parent reply other threads:[~2017-08-30 13:39 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-28 11:02 [LTP] [PATCH v2 1/7] tst_atomic: Add load, store and use __atomic builtins Richard Palethorpe
2017-08-28 11:02 ` [LTP] [PATCH v2 2/7] tst_atomic: Add atomic store and load tests Richard Palethorpe
2017-08-28 11:02 ` [LTP] [PATCH v2 3/7] fzsync: Add long running thread support and deviation stats Richard Palethorpe
2017-08-28 11:02 ` [LTP] [PATCH v2 4/7] fzsync: Add functionality test for library Richard Palethorpe
2017-08-28 11:02 ` [LTP] [PATCH v2 5/7] Convert cve-2016-7117 test to use long running threads Richard Palethorpe
2017-08-28 11:02 ` [LTP] [PATCH v2 6/7] Convert cve-2014-0196 " Richard Palethorpe
2017-08-28 11:02 ` [LTP] [PATCH v2 7/7] Convert cve-2017-2671 " Richard Palethorpe
2017-08-29 14:58 ` [LTP] [PATCH v2 1/7] tst_atomic: Add load, store and use __atomic builtins Jan Stancek
2017-08-30 12:27 ` Richard Palethorpe
2017-08-30 13:39 ` Cyril Hrubis [this message]
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=20170830133928.GA22607@rei \
--to=chrubis@suse.cz \
--cc=ltp@lists.linux.it \
/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