Andreas Hansson got review request #1771!

Information
Submitter:	Andreas Hansson
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 9612:7c0917e48c0f
---------------------------
mem: Separate waiting for the bus and waiting for a peer

This patch splits the retryList into a list of ports that are waiting
for the bus itself to become available, and a map that tracks the
ports where forwarding failed due to a peer not accepting the
packet. Thus, when a retry reaches the bus, it can be sent to the
appropriate port that initiated that transaction.

As a consequence of this patch, only ports that are really ready to go
will get a retry, thus reducing the amount of redundant failed
attempts. This patch also makes it easier to reason about the order of
servicing requests as the ports waiting for the bus are now clearly
FIFO and much easier to change if desired.

Testing Done

With stats updates, all regressions pass

Issue Summary

Description	From	Last Updated	Status
Aren't portIDs small integers? Wouldn't a vector suffice, with NULL entries for ports that aren't waiting? Or maybe that's just ...	Steve Reinhardt	March 26, 2013, 1:43 a.m.	Resolved

src/mem/bus.hh (Diff revision 1)

A instead of An?

Andreas Hansson March 15, 2013, 3 a.m. (March 15, 2013, 3 a.m.)
```
Consider it done
```

Description:

~		Changeset 9589:4eb79191ee1f
	~	Changeset 9590:fc9c766646dd

		mem: Separate waiting for the bus and waiting for a peer

		This patch splits the retryList into a list of ports that are waiting
		for the bus itself to become available, and a map that tracks the
		ports where forwarding failed due to a peer not accepting the
		packet. Thus, when a retry reaches the bus, it can be sent to the
		appropriate port that initiated that transaction.

		As a consequence of this patch, only ports that are really ready to go
		will get a retry, thus reducing the amount of redundant failed
		attempts. This patch also makes it easier to reason about the order of
		servicing requests as the ports waiting for the bus are now clearly
		FIFO and much easier to change if desired.

Diff:

Revision 2 (+121 -115)

Show changes

	src/mem/bus.hh
	src/mem/bus.cc
	src/mem/coherent_bus.hh
	src/mem/coherent_bus.cc
	src/mem/noncoherent_bus.hh
	src/mem/noncoherent_bus.cc

Looks good to me... just one minor optional issue below.

src/mem/bus.hh (Diff revision 2)

Aren't portIDs small integers?  Wouldn't a vector suffice, with NULL entries for ports that aren't waiting?

Or maybe that's just the old C programmer in me showing through...

Show all issues

Andreas Hansson March 25, 2013, 8:03 p.m. (March 25, 2013, 8:03 p.m.)

Very valid comment. We do know the size of the PortIDs in question, and a vector would probably be more efficient. Let me give it a shot.

Description:

~		Changeset 9590:fc9c766646dd
	~	Changeset 9612:7c0917e48c0f

		mem: Separate waiting for the bus and waiting for a peer

		This patch splits the retryList into a list of ports that are waiting
		for the bus itself to become available, and a map that tracks the
		ports where forwarding failed due to a peer not accepting the
		packet. Thus, when a retry reaches the bus, it can be sent to the
		appropriate port that initiated that transaction.

		As a consequence of this patch, only ports that are really ready to go
		will get a retry, thus reducing the amount of redundant failed
		attempts. This patch also makes it easier to reason about the order of
		servicing requests as the ports waiting for the bus are now clearly
		FIFO and much easier to change if desired.

Diff:

Revision 3 (+137 -123)

Show changes

	src/mem/bus.hh
	src/mem/bus.cc
	src/mem/coherent_bus.hh
	src/mem/coherent_bus.cc
	src/mem/noncoherent_bus.hh
	src/mem/noncoherent_bus.cc

Ship It!

You have a pending review.

Review Board 2.0.15

Andreas Hansson got review request #1771!

This change has been marked as submitted.

mem: Separate waiting for the bus and waiting for a peer

Screenshots

Files

Issue Summary

Description:

Changeset 9589:4eb79191ee1f

Changeset 9590:fc9c766646dd

Diff:

Description:

Changeset 9590:fc9c766646dd

Changeset 9612:7c0917e48c0f

Diff:

Status: Closed (submitted)