> On April 18, 2016, 5:06 a.m., Steve Reinhardt wrote: > > src/mem/cache/write_queue_entry.hh, line 80 > > <http://reviews.gem5.org/r/3407/diff/2/?file=54859#file54859line80> > > > > Is this 'pkt' field still used? I'd think not, in which case it should > > of course be deleted. If it is still used, I'd be curious how, since I'm > > leery of both forwarding the packet down to the next level and keeping a > > copy of the pointer ourselves---makes the ownership situation ambiguous. > > Andreas Hansson wrote: > Yes it is still used. It is used for functional accesses where the > packets in the queue needs to be updated, and it is used for changing the > properties of these packets if snoops hit in the write queue. > > Steve Reinhardt wrote: > But this only applies in the window between when the packet is received > and when it is forwarded on to the next level, correct? Do we set the pkt > field to nullptr once it's sent? (Obviously not, now that I see the 'const' > field there.) I'm just a little uncomfortable with the notion that every > write queue all the way down the hierarchy is sitting with a pointer to the > same packet, as if each cache owns that packet. When the packet gets updated > on a snoop, for instance, does only one cache do the update? Or are we just > getting lucky that once one cache updates it, it no longer matches the > criteria for other caches to do the update? > > Obviously we don't need to hang on to the packet pointer anymore once > we've forwarded it, since it will be coming back to us in the response. I'd > feel better if we overwrote that field with nullptr when we forward the > packet on.
I'm trying to wrap my head around what it is you want to achieve here. When we send a normal cacheable write downstream we delete the entire WriteQueueEntry once it is sent. It is only for uncacheable accesses (that are "invisible" when looking up queue entries) that we keep the pkt pointer in the WriteQueueEntry until the response comes back. I'd really prefer to not make this more complicated. Makes sense? - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3407/#review8240 ----------------------------------------------------------- On April 9, 2016, 4:20 p.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3407/ > ----------------------------------------------------------- > > (Updated April 9, 2016, 4:20 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11442:338a5d23f371 > --------------------------- > mem: Simplify cache packet handling for uncacheable writes > > Cosmetic change following up on the recent cache queue > modifications. This patch aligns the handling of uncacheable writes in > that no new packets are created, similar to how write evictions are > dealt with. By not allocating copies of the packet the code is > simplified, and we avoid unecessary memory management. > > > Diffs > ----- > > src/mem/cache/cache.cc 0edcf757b6a2 > src/mem/cache/write_queue_entry.hh 0edcf757b6a2 > > Diff: http://reviews.gem5.org/r/3407/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
