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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.