Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disabling diag_lx_debug in the periodic timer functions #47

Open
nsajko opened this issue Dec 30, 2017 · 2 comments
Open

disabling diag_lx_debug in the periodic timer functions #47

nsajko opened this issue Dec 30, 2017 · 2 comments

Comments

@nsajko
Copy link
Contributor

nsajko commented Dec 30, 2017

Why do saej1979 and iso14230 timer functions disable debugging for callee functions? (Note: added in d6cecf3 .)

It is a non-essential feature as far as I understand?

The issue is that if you are going to override user settings locally, suddenly the diag_x_debug variables being global instead of parameter based becomes much uglier/hairier to reason about, at least it seems like that to me.

On the other hand, right now the accesses to the diag_x_debug variables are racy, and if there were not for those local overrides in the timer functions, the concurrency issues could be fixed by hiding the global variables behind atomic set/store and get/load functions (see the atomics stuff in proposed commit b9ccc01 "Add atomic types, funcs; fix with periodic timer deletion data race"), which would be nicer than using mutexes directly.

@fenugrec
Copy link
Owner

fenugrec commented Dec 30, 2017

Hi,
yes - the debug level flags are naively implemented, and I disabled logging in the async timer functions partly due to this. Also because having the periodic timer functions spew messages interspersed with what the user is currently doing was incredibly messy !

I recommend you just ignore this for a while longer, I'm already working on integrating zflog to improve this:

  • By default he logging output will piped (thread-safe) to a file instead of stderr
  • still not sure if I want to keep the L0/L1/L2 debug flags : if the messages are activated by "severity" instead (warn/info/debug), everything will end up in the log file anyway instead of cluttering the CLI
  • Instead of having stacks of diag_iseterr() messages in the CLI (usually the same error piled up by every level), maybe restrict stderr usage to the error source, and the top level "sink"

@nsajko
Copy link
Contributor Author

nsajko commented Jan 1, 2018

Judging by your last comment it wont be merged, but this is what I meant with "the concurrency issues could be fixed by hiding the global variables behind atomic set/store and get/load functions": #48

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants