This stack is kind of funky... I'm not sure why Stq::initiateAcc would
call into TimingSimpleCPU::completeIfetch.

I went back and looked at that ExecEnable bug discussion though, and I
recall that problem... sad to see that after all the discussion we had
almost a year ago the bug is still not fixed (as far as I can tell).
This patch you made here does look like it could be addressing the
same or a similar issue.

Steve

On Thu, Mar 18, 2010 at 6:56 PM, Beckmann, Brad <[email protected]> wrote:
> I don't have exact Valgrind stack anymore, but below is the approximate call 
> stack (it is actually a subset of another call stack that I wasn't able to 
> completely solve...more on that later).  Basically if I remember correctly, 
> the request is deleted within sendData.
>
> TimingSimpleCPU::sendData(RefCountingPtr<FaultBase>, Request*, unsigned 
> char*, unsigned long*, bool) ==27646==    by 
> TimingSimpleCPU::DataTranslation::finish(RefCountingPtr<FaultBase>, Request*, 
> ThreadContext*, BaseTLB::Mode) (/
> tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.hh:139)
> AlphaISA::TLB::translateTiming(Request*, ThreadContext*, 
> BaseTLB::Translation*, BaseTLB::Mode) (/tmp/bbeckman/m
> 5/build/ALPHA_SE_MOESI_hammer/arch/alpha/tlb.cc:603)
> RefCountingPtr<FaultBase> TimingSimpleCPU::write<unsigned long>(unsigned 
> long, unsigned long, unsigned, unsigne
> d long*) 
> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:599)
> AlphaISAInst::Stq::initiateAcc(TimingSimpleCPU*, Trace::InstRecord*) const 
> (/tmp/bbeckman/m5/build/ALPHA_SE_MOE
> SI_hammer/arch/alpha/timing_simple_cpu_exec.cc:1953)
> TimingSimpleCPU::completeIfetch(Packet*) 
> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:769)
> TimingSimpleCPU::IcachePort::recvTiming(Packet*) 
> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/cpu/simple/timing.cc:832)
> Port::sendTiming(Packet*) 
> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/mem/port.hh:186)
> Bus::recvTiming(Packet*) 
> (/tmp/bbeckman/m5/build/ALPHA_SE_MOESI_hammer/mem/bus.cc:243)
>
>
> Now the bug I wasn't able to completely fix involved the bug identified in 
> the old thread: "Memory corruption in m5 dev repository whenusing    
> --trace-flags="ExecEnable"".  That bug as a similar problem where the trace 
> data is deleted within the write function and then referenced later in 
> initiateAcc, however it is even more confusing because it deals with two 
> different pointers to traceData.  I have a patch that has a heavyweight fix 
> for that bug, but I didn't send it out for review because I couldn't figure 
> out how to get it to compile with the O3 cpu model.  If you're interested, I 
> can send that out as well.
>
> Brad
>
>
> -----Original Message-----
> From: [email protected] [mailto:[email protected]] On Behalf Of 
> Steve Reinhardt
> Sent: Thursday, March 18, 2010 4:24 PM
> To: M5 Developer List
> Subject: Re: [m5-dev] [PATCH 07 of 31] m5: Fixed request read bug flagged by 
> Valgrind
>
> It seems very odd that translateTiming would delete the request
> object... can you point out where that happens?  I couldn't find it in
> the alpha or x86 tlb code.
>
> Thanks,
>
> Steve
>
> On Thu, Mar 18, 2010 at 3:46 PM, Brad Beckmann <[email protected]> wrote:
>> # HG changeset patch
>> # User Brad Beckmann <[email protected]>
>> # Date 1268941825 25200
>> # Node ID c2db9da78da715a22d76e92e0cabbced82dcef9f
>> # Parent  d657b9a0875113dbb453986391661fb3c7fa669c
>> m5: Fixed request read bug flagged by Valgrind
>>
>> Previously the recording of an uncached read occurred after the request was
>> possibly deleted within the translateTiming function.
>>
>> diff --git a/src/cpu/simple/timing.cc b/src/cpu/simple/timing.cc
>> --- a/src/cpu/simple/timing.cc
>> +++ b/src/cpu/simple/timing.cc
>> @@ -432,6 +432,10 @@
>>     Addr split_addr = roundDown(addr + data_size - 1, block_size);
>>     assert(split_addr <= addr || split_addr - addr < block_size);
>>
>> +    // This will need a new way to tell if it's hooked up to a cache or not.
>> +    if (req->isUncacheable())
>> +        recordEvent("Uncached Write");
>> +
>>     _status = DTBWaitResponse;
>>     if (split_addr > addr) {
>>         RequestPtr req1, req2;
>> @@ -461,10 +465,6 @@
>>         traceData->setAddr(addr);
>>     }
>>
>> -    // This will need a new way to tell if it has a dcache attached.
>> -    if (req->isUncacheable())
>> -        recordEvent("Uncached Read");
>> -
>>     return NoFault;
>>  }
>>
>> @@ -510,7 +510,6 @@
>>     return read(addr, *(uint32_t*)&data, flags);
>>  }
>>
>> -
>>  template<>
>>  Fault
>>  TimingSimpleCPU::read(Addr addr, int32_t &data, unsigned flags)
>> @@ -555,6 +554,10 @@
>>     Addr split_addr = roundDown(addr + data_size - 1, block_size);
>>     assert(split_addr <= addr || split_addr - addr < block_size);
>>
>> +    // This will need a new way to tell if it's hooked up to a cache or not.
>> +    if (req->isUncacheable())
>> +        recordEvent("Uncached Write");
>> +
>>     T *dataP = new T;
>>     *dataP = TheISA::htog(data);
>>     _status = DTBWaitResponse;
>> @@ -586,10 +589,6 @@
>>         traceData->setData(data);
>>     }
>>
>> -    // This will need a new way to tell if it's hooked up to a cache or not.
>> -    if (req->isUncacheable())
>> -        recordEvent("Uncached Write");
>> -
>>     // If the write needs to have a fault on the access, consider calling
>>     // changeStatus() and changing it to "bad addr write" or something.
>>     return NoFault;
>>
>> _______________________________________________
>> m5-dev mailing list
>> [email protected]
>> http://m5sim.org/mailman/listinfo/m5-dev
>>
> _______________________________________________
> m5-dev mailing list
> [email protected]
> http://m5sim.org/mailman/listinfo/m5-dev
>
>
> _______________________________________________
> m5-dev mailing list
> [email protected]
> http://m5sim.org/mailman/listinfo/m5-dev
>
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev

Reply via email to