Conversation
|
Thanks for this Luc, this is very useful! Just one question. Should we move the tests to the catalogue? I see two arguments for doing so:
If you want to maintain the expected files, see here for what I did with a similar requirement https://github.com/herd/herdtools7/pull/1864/changes#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52 |
|
Exposing some LKMM tests in the web interface looks like a great idea. Nevertheless, I'd realy like to also have some LKMM testing to be part of systematic testing, given the often unforseen impact of some changes on LKMM we experienced in the past. Your solution from PR #1864 can apparently give us both. Should we wait for PR #1864 to be merged? |
633a4d5 to
0efa116
Compare
|
Hi @relokin. I have moved all tests to the now serious "linux" book of the web interface and used your Makefile entry "test.herd.cata-extended.%" to include the tests of the book into the standard test suite ( Web interface: https://diy.inria.fr/TST/www/index.html?record=linux&bell=linux-kernel&cat=linux-kernel&litmus=MP%2Brelacq&cfg=linux-kernel |
|
Could you help me understand the motivation for the new |
2ff3c33 to
cad0a90
Compare
For the linux arch, herd7 loads three files in addition to the configuration file (.cfg): model (.cat), bell file (.bell) and macros (.def). For running herd7 as Hence the new feature has more to do with herd7 than with the web interface. However it may be necessary to load the `macro file. I am not so sure about the bell file... Another option would have been to add the |
relokin
left a comment
There was a problem hiding this comment.
Thanks for this Luc, I've got two comments.
| @@ -0,0 +1,40 @@ | |||
| C Z6.0+pooncelock+pooncelock+pombonce | |||
There was a problem hiding this comment.
This catalogue has two tests with almost identical names:
- Z6.0+pooncelock+pooncelock+pombonce
- Z6.0+pooncelock+poonceLock+pombonce
In case-insensitive filesystems (for example, by default APFS on MacOS), this creates problems. Can we change the name of one of the tests?
There was a problem hiding this comment.
Nice catch, I am fixing this as you suggest.
| @@ -0,0 +1,3 @@ | |||
| include $dirname/../cats | |||
There was a problem hiding this comment.
isn't this very fragile? Is it hard to create a full libdir inside this catalogue?
There was a problem hiding this comment.
I do not see what you mean by "full libdir inside this catalogue".
There was a problem hiding this comment.
AFAICT, the issue is that the Makefile sets libdir to herd/libdir, while you want herd7 to be able to find linux-kernel.bell, linux-kernel.cat, linux-kernel.def, and lock.cat, which currently live in catalogue/linux/cats.
I can see three possible ways to fix this:
- Your approach: extend the list of paths in which
herd7looks for files. - Copy these files into
herd/libdir. - Create a complete
libdirundercatalogue/linux/libdircontaining all the.cat,.bell, and.deffiles needed to run this catalogue.
I think the second and third options are less fragile. More generally, independently of this particular catalogue, I would expect us to want all the files needed to run herd7 with the LKMM model to be available in one place, as that would also make things easier for users.
There was a problem hiding this comment.
I would also be in favour of solving this problem with (2) or (3), or other "internal" solution, before considering working around it by changing the public configuration language of herd. I do see the utility in what $dirname proposes, however, user-facing changes come with a maintenance commitment to keep that interface stable, and I think that warrants its own PR and discussion.
There was a problem hiding this comment.
The four files linux-kernel.bell, linux-kernel.cat, linux-kernel.def, and lock.cat are from the linux sources and I'd rather have them in the catalogue (with README) than in herd7 standard libdir. Hence I'd rather have solution (3). However, lock.cat includes the files cross.cat and cos-opt.cat. Hence those files would have to be copied into catalogue/linux/libdir. Additionally a stdlib.cat would also have to be copied there. I do not find this that elegant.
Thanks for the additional context. As a data point, for the mere purpose of I realise this only works if |
I follow your suggestion, which provides a simple and workable solution. |
The regression consists in not marking some effects as being a data port, resulting in less dependencies. However, by contrast with C11 models, the LKMM sometimes relies on dependencies.
The new book includes:
- Linux Kernel Memory Model at date, copied from linux sources
(https://github.com/torvalds/linux/tree/master/tools/memory-model)
- Some litmus tests taken from linux sources (see above) and
from the "perfbook" (https://kernel.googlesource.com/pub/scm/linux/kernel/git/paulmck/perfbook/+/refs/heads/master/CodeSamples/formal/herd/) by
"Paul E. McKenney" <paulmck@kernel.org>.
Those tests were suggested to us by "Akira Yokosawa" <akiyks@gmail.com>.
To that aim we use the extension introduced by PR #1864.
This PR add a few linux kernel memory model tests to our regression suite. The model, configuration files and most tests are taken from Linux sources. A few tests are authored by @akiyks.