[Bug translator/2056] New: Cannot iterate pmaps

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug translator/2056] New: Cannot iterate pmaps

glaubitz at physik dot fu-berlin.de
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);

--
           Summary: Cannot iterate pmaps
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: critical
          Priority: P2
         Component: translator
        AssignedTo: systemtap at sources dot redhat dot com
        ReportedBy: hunt at redhat dot com


http://sourceware.org/bugzilla/show_bug.cgi?id=2056

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
Reply | Threaded
Open this post in threaded view
|

[Bug translator/2056] Cannot iterate pmaps

glaubitz at physik dot fu-berlin.de

------- Additional Comments From joshua dot i dot stone at intel dot com  2005-12-21 20:30 -------
Created an attachment (id=809)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=809&action=view)
enable pmap iteration

This patch seems trivial enough that I wonder why it hasn't already been fixed.
 Am I missing something?

--


http://sourceware.org/bugzilla/show_bug.cgi?id=2056

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
Reply | Threaded
Open this post in threaded view
|

[Bug translator/2056] Cannot iterate pmaps

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de

------- Additional Comments From joshua dot i dot stone at intel dot com  2005-12-21 20:37 -------
(In reply to comment #1)
> Created an attachment (id=809)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=809&action=view)
> 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.

--
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |joshua dot i dot stone at
                   |                            |intel dot com


http://sourceware.org/bugzilla/show_bug.cgi?id=2056

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
Reply | Threaded
Open this post in threaded view
|

[Bug translator/2056] Cannot iterate pmaps

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de

------- Additional Comments From fche at redhat dot com  2005-12-21 20:45 -------
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.

--


http://sourceware.org/bugzilla/show_bug.cgi?id=2056

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
Reply | Threaded
Open this post in threaded view
|

[Bug translator/2056] avoid locking within foreach iteration for maps & pmaps

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de


--
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|critical                    |normal
             Status|NEW                         |ASSIGNED
           Priority|P2                          |P1
   Last reconfirmed|0000-00-00 00:00:00         |2006-01-04 21:00:04
               date|                            |
            Summary|Cannot iterate pmaps        |avoid locking within foreach
                   |                            |iteration for maps & pmaps


http://sourceware.org/bugzilla/show_bug.cgi?id=2056

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

Reply | Threaded
Open this post in threaded view
|

[Bug translator/2056] avoid locking within foreach iteration for maps & pmaps

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de

------- Additional Comments From hunt at redhat dot com  2006-01-05 06:08 -------
(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.


--


http://sourceware.org/bugzilla/show_bug.cgi?id=2056

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

Reply | Threaded
Open this post in threaded view
|

[Bug translator/2056] avoid locking within foreach iteration for maps & pmaps

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de

------- Additional Comments From fche at redhat dot com  2006-01-05 22:08 -------
(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.


--


http://sourceware.org/bugzilla/show_bug.cgi?id=2056

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

Reply | Threaded
Open this post in threaded view
|

[Bug translator/2056] avoid locking within foreach iteration for maps & pmaps

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de

------- Additional Comments From hunt at redhat dot com  2006-01-05 22:35 -------
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.



--


http://sourceware.org/bugzilla/show_bug.cgi?id=2056

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

Reply | Threaded
Open this post in threaded view
|

[Bug translator/2056] avoid locking within foreach iteration for maps & pmaps

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de

------- Additional Comments From joshua dot i dot stone at intel dot com  2006-01-13 03:58 -------
This should be fixed now.  (translate.cxx rev 1.91)

testsuite/buildok/pmap_foreach.stp now compiles and runs successfully.

--
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://sourceware.org/bugzilla/show_bug.cgi?id=2056

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.