Re: [PATCH 2/4] utrace_set_events: consolidate setting _UTRACE_DEATH_EVENTS checks
On 08/16, Roland McGrath wrote: In short: this double check allows to avoid tasklist when utrace_flags already has these bits while engine-flags doesn't. But multitracing is unlikely case, in the likely case if we add _UTRACE_DEATH_EVENTS to engine-flags we set these bits in utrace_flags too. I think it makes sense to consolidate these checks to make the code a bit more understandable. I don't really agree about unlikely case. In many uses, the systemtap task-finder will have a utrace engine on every task in the system, for example. Moreover, this is an importantly distinct particular kind of micro-optimization to be doing here: avoiding a system-wide lock. Any place that we take tasklist_lock at all, we are introducing a system-wide slowdown or limitation on scaling, just because of our tracing of one task. So optimizing the minimize that is qualitatively different and really much more important than avoiding taking the utrace lock, for example. But with that note in your mind, I am happy to take this patch now as a simplification and let reoptimization come back later. OK, agreed. Oleg.
Re: [PATCH 2/4] utrace_set_events: consolidate setting _UTRACE_DEATH_EVENTS checks
Hmm. For unknown reason I do not see this 2/4 patch on utrace-devel, strange. So I am attaching it in case my email was really lost and you didn't get it. Indeed, it did not come through to me or the list. Thanks, Roland
Re: [PATCH 2/4] utrace_set_events: consolidate setting _UTRACE_DEATH_EVENTS checks
utrace_set_events() checks the setting _UTRACE_DEATH_EVENTS case twice, and it is not immediately obvious why the first check is needed, and why it is not racy (we are checking exit_state without tasklist). The code is correct, but looks confusing. More comments are always good. In short: this double check allows to avoid tasklist when utrace_flags already has these bits while engine-flags doesn't. But multitracing is unlikely case, in the likely case if we add _UTRACE_DEATH_EVENTS to engine-flags we set these bits in utrace_flags too. I think it makes sense to consolidate these checks to make the code a bit more understandable. I don't really agree about unlikely case. In many uses, the systemtap task-finder will have a utrace engine on every task in the system, for example. Moreover, this is an importantly distinct particular kind of micro-optimization to be doing here: avoiding a system-wide lock. Any place that we take tasklist_lock at all, we are introducing a system-wide slowdown or limitation on scaling, just because of our tracing of one task. So optimizing the minimize that is qualitatively different and really much more important than avoiding taking the utrace lock, for example. But with that note in your mind, I am happy to take this patch now as a simplification and let reoptimization come back later. Thanks, Roland
