On 31 July 2015 at 05:42, Pi-Cheng Chen pi-cheng.chen@linaro.org wrote:
Hi Juri,
On Tue, Jul 28, 2015 at 04:05:54PM +0100, Juri Lelli wrote:
Hi Pi-Cheng,
On 15/07/15 04:39, Pi-Cheng Chen wrote:
This series adds several new features to rt-app, e.g. signal, wait, broadcast, run, sleep ...etc. and updates the example taskset.json file to reflect those new features.
First of all, thanks a lot again for the hard work of sending these series out. Really appreciated :).
Regarding this series. I started reviewing it, but I noticed that it seems composed of "intermediate" changes that ended up to simpler/ cleaner features in the end (on Linaro's rt-app repo). I understand this to be a foundational series, but, since we have the opportunity to write a clean git history here, don't you think it would be better to squash some patches from the full history as to get something like:
- json grammar clean-up (e.g., remove lock order and resource defs)
- introduce self contained features in their final shape (e.g., run, sleep, timer, sync, etc.)
- introduce examples alongside features
- introduce phases
- other things/fixes
Yes. I agree with you. This will make things cleaner. But for the 2nd point: "introduce self contained features in their final shape", I am not sure. Would it lead to the loss of some development history?
@Vincent: How do you think about it?
I have no objection to reorder patches in order to have patches linked to same features together like the example of the loop and the calibration. We can also probably squash some patches that fixes bugs into the original patch that creates the feature. Nevertheless, the order of the patches have tried to keep rt-app usable along the tree and IMHO it's important to maintain that
Regards, Vincent
I understand it is some work to do, but what are your feelings about this? I really think it is important to come up with a clean set of changes.
Anyway, I already merged patch 04/11 (loop feature). Also, I think patch 05/11 can go in as is, but it would be nice to have "calibration: parse for CPU or forces ns per loop value" together with it (and any other calibration related patches as well).
I will do it.
Thanks for reviewing.
Best Regards, Pi-Cheng
Best,
- Juri
These patches are also found: ssh://git@git.linaro.org/people/picheng.chen/rt-app.git features_series1
Sanjay Singh Rawat (1): add delay param in thread parameters
Vincent Guittot (10): Add new wait, signal, and broadcast resources update the .json test file to reflect new capabilities add loop feature rt-app: make load frequency independent. update example file with new loop capabilities add sleep and run type of resources update taskset.json example with run and sleep capabilities make possible to remove the sleep step modify stat to reflect new behavior update example file with more capabilities
doc/taskset.json | 104 ++++++++++++---------- src/rt-app.c | 155 +++++++++++++++++++++++++++------ src/rt-app_parse_config.c | 214 +++++++++++++++++++++++++++++++++++++--------- src/rt-app_types.h | 35 +++++++- src/rt-app_utils.c | 64 ++++++++++++++ src/rt-app_utils.h | 9 ++ 6 files changed, 467 insertions(+), 114 deletions(-)