ruby: RubyPort delete snoop requests

Information
Submitter:	Joel Hestness
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 11144:f3dcf1be17c4
---------------------------
ruby: RubyPort delete snoop requests

In RubyPort::ruby_eviction_callback, prior changes fixed a memory leak caused
by instantiating separate packets for each port that the eviction was forwarded
to. That change, however, left the instantiated request to also leak. Allocate
it on the stack to avoid the leak.

Testing Done

Compiled gem5.debug with --without-tcmalloc. Ran large tests with Valgrind.

Issue Summary

Description	From	Last Updated	Status
Do we know for sure that the receiving end is not going to delete the request?	Nilay Vaish	Sept. 16, 2015, 9:08 p.m.	Dropped
Why call new here in the first place?	Andreas Hansson	Sept. 25, 2015, 5:12 p.m.	Resolved
Can we make the size a sensible value (a cacheline size)?	Andreas Hansson	Sept. 27, 2015, 9:52 a.m.	Resolved

src/mem/ruby/system/RubyPort.cc (Diff revision 1)

Do we know for sure that the receiving end is not going to delete the request?

Show all issues

Joel Hestness Sept. 17, 2015, 6:49 a.m. (Sept. 17, 2015, 6:49 a.m.)

Yes, there are no instances of recvTimingSnoopReq that delete packets in mainline. This seems to indicate that it is the responsibility of the requester to delete snoop packets/requests.

src/mem/ruby/system/RubyPort.cc (Diff revision 1)

Why call new here in the first place?

Show all issues

Change Summary:

Per Andreas' suggestion, allocate request on stack.

Description:

~		Changeset 11092:03402924ac1e
	~	Changeset 11140:34b60f0ef0b5

		ruby: RubyPort delete snoop requests

		In RubyPort::ruby_eviction_callback, prior changes fixed a memory leak caused
		by instantiating separate packets for each port that the eviction was forwarded
~		to. That change, however, left the instantiated request to also leak. Delete
~		the request.
	~	to. That change, however, left the instantiated request to also leak. Allocate
	~	it on the stack to avoid the leak.

Diff:

Revision 2 (+2 -2)

Show changes

src/mem/ruby/system/RubyPort.cc

src/mem/ruby/system/RubyPort.cc (Diff revision 2)

Can we make the size a sensible value (a cacheline size)?

Show all issues

Description:

~		Changeset 11140:34b60f0ef0b5
	~	Changeset 11144:f3dcf1be17c4

		ruby: RubyPort delete snoop requests

		In RubyPort::ruby_eviction_callback, prior changes fixed a memory leak caused
		by instantiating separate packets for each port that the eviction was forwarded
		to. That change, however, left the instantiated request to also leak. Allocate
		it on the stack to avoid the leak.

Diff:

Revision 3 (+3 -2)

Show changes

src/mem/ruby/system/RubyPort.cc

Could you update the comments in the code? Also, the patch description is somewhat misleading after the changes. No need to re-post.

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Issue Summary

Change Summary:

Description:

Changeset 11092:03402924ac1e

Changeset 11140:34b60f0ef0b5

Diff:

Description:

Changeset 11140:34b60f0ef0b5

Changeset 11144:f3dcf1be17c4

Diff:

Status: Closed (submitted)