See testsuite/buildok/pmap_foreach.stp To iterate a pmap, the generated code correctly does l->__tmp10 = _stp_map_start (_stp_pmap_get_agg(global_foo)); then gets confused and does not use the agg map to iterate: l->__tmp10 = _stp_map_iter (global_foo, l->__tmp10);
Created attachment 809 [details] enable pmap iteration This patch seems trivial enough that I wonder why it hasn't already been fixed. Am I missing something?
(In reply to comment #1) > Created an attachment (id=809) > enable pmap iteration The patch allows the testcase to build, but it deadlocks when you run it. The deadlock is here: foreach (i in foo) printf("count of foo[%d] = %d\n", i, @count(foo[i])) This happens because the 'foreach' opens a readlock on foo, and then '@count' tries to open a writelock. While this is a valid problem, it may be out of the scope of this bug.
Thanks for the patch, please commit at will. Too bad this didn't show up with a buildok test. There are a number of trivial little bugs in the system like this one, left there partly to encourage systemtap newbies to get involved, and partly because I just never got to them. :-) Regarding the pmap deadlock, one possible fix is to have foreach() iterating over a pmap hold an exclusive lock around the whole loop, and the @extraction operators to hold none, when they're nested within foreach(). In fact, ordinary array reads enclosed within foreach() don't need to be locked either. Let's transmute this bugzilla entry to track this bug.
(In reply to comment #3) > Regarding the pmap deadlock, one possible fix is to have foreach() iterating > over a pmap hold an exclusive lock around the whole loop, and the @extraction > operators to hold none, when they're nested within foreach(). In fact, ordinary > array reads enclosed within foreach() don't need to be locked either. Let's > transmute this bugzilla entry to track this bug. Maybe I'm missing something, but the problem is the writelock. Why does the generated code take a writelock on the pmap when it is reading stats? I suspect the reason is confusion over reading from a map and a pmap. You need a writelock when doing _stp_pmap_agg() or _stp_pmap_get(). However, _stp_pmap_agg() creates a normal map and you only want to readlock it when reading. Until this gets fixed, pmaps of stats are broken because we cannot print without using foreach.
(In reply to comment #4) > Maybe I'm missing something, but the problem is the writelock. Why does the > generated code take a writelock on the pmap when it is reading stats? [...] We would hold an exclusive ("write") lock around a pmap iteration in order to prevent concurrent updates to it. This is exactly analogous to taking a shared ("read") lock around a scalar foreach. With the changes of bug #2057, there is now no translator-emitted locking around accumulation, which may now expose us to this problem. The translator should probably do what I originally intended: emit a shared ("read") lock around accumulation, and rely in no way on spinlocks in the runtime.
OK, now I see the source of the misunderstanding. foreach (i in foo) printf("count of foo[%d] = %d\n", i, @count(foo[i])) generates code like this: write_lock() _stp_pmap_agg() write_unlock() read_lock() foreach { write_lock() _stp_pmap_get_ix() write_unlock() } read_unlock() ------------- The problem is the write)lock() needs to go away and instead use _stp_map_get_ix(_stp_pmap_get_agg(global_foo), i) _stp_pmap_agg() is much more effcient that calling _stp_pmap_get_xx() for each index. In fact I consided not even including the _stp_pmap_get functions in the runtime.
This should be fixed now. (translate.cxx rev 1.91) testsuite/buildok/pmap_foreach.stp now compiles and runs successfully.