Repository navigation
feat: Logging license type usage - #268
dominicprice-lowrisc wants to merge 2 commits into
Conversation
1f66ad8 to
b6114ba
Compare
dominicprice-lowrisc
changed the title
There was a problem hiding this comment.
Thanks for the work on this @dominicprice-lowrisc, the overall approach LGTM.
I've left a lot of comments but most are very nitpicky, and hopefully quite easy to address or defer. The important comments are those on OpenTitan's commit guidelines and about handling resources vs. tools in the ResourceManager.
Signed-off-by: Dominic Price <dominic.price@lowrisc.org> feat: Improved formatting of log messages Signed-off-by: Dominic Price <dominic.price@lowrisc.org> test: intercept logs and check they are present Signed-off-by: Dominic Price <dominic.price@lowrisc.org>
… runner Signed-off-by: Dominic Price <dominic.price@lowrisc.org>
7df688f to
b72ac5f
Compare
There was a problem hiding this comment.
Thanks @dominicprice-lowrisc, I think this is looking good! I much prefer it hooking onto the scheduler externally now that you've implemented it that way.
I only really have two high-level comments to address before this is merged, which are with the commits themselves:
- There are several changes that are introduced in the first commit and then dropped in the second. The history would be clearer if this was not the case. You could go and edit each commit, but the easier option is probably just to squash the two together into one.
- Can you edit the commit message(s) so they don't contain the full squashed history?
@machshev It would be nice if you could confirm that this works for your desired use case. To extend support to lint & formal tools etc. I think there may(?) be some very minor extension needed in the FlowCfgs / Deploys to declare the tool as a resource, but that can be left for a separate PR as is needed.
This branch has not been deployed
2 participants
Description
This PR addresses #267 by adding additional logging at the verbose level. There is already logging for when a job has changed status (e.g. from scheduled to queued), and now a table is also logged showing per resource, the number of jobs with each status. I have attached an image and text copy of how these logs look when run in the OpenTitan repository. I have also added a test case to intercept the logs and check that each of the simulation tools is present in the logs at the debug level.
The way I implemented this was to create an index in the
ResourceManagerto track job statuses per resource, and register callbacks in theSchedulerto log this information whenever a job changes status. I also modifiedtool_meta_factoryin the tests file to randomly select real simulation tools names, rather than just using"test_tool"by default.I identified a minor bug in
logging.pywhere logs at the verbose level are always seen to be coming a line inlogging.py, rather than from the file and line number where that log function was being called. I fixed this by passingstacklevel=2.(The exact command I used to generate the attached screenshot was
uv run --no-sync dvsim hw/top_earlgrey/dv/top_earlgrey_sim_cfgs.hjson -i smoke --scratch ~/scratch --fixed-seed 1 --verbose=debug --cov -R A=20, with my fork of DVSim installed).(The image and text are now slightly out of date, with
Resourceinstead ofTool, and improved formatting)