From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11632 invoked by alias); 6 Aug 2008 06:43:48 -0000 Received: (qmail 11624 invoked by uid 22791); 6 Aug 2008 06:43:47 -0000 X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.33.17) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 06 Aug 2008 06:43:12 +0000 Received: from wpaz37.hot.corp.google.com (wpaz37.hot.corp.google.com [172.24.198.101]) by smtp-out.google.com with ESMTP id m766h7Uq018146 for ; Wed, 6 Aug 2008 07:43:07 +0100 Received: from localhost (ruffy.corp.google.com [172.18.118.116]) by wpaz37.hot.corp.google.com with ESMTP id m766h6ls017485 for ; Tue, 5 Aug 2008 23:43:07 -0700 Received: by localhost (Postfix, from userid 67641) id 300831C77BE; Tue, 5 Aug 2008 23:43:06 -0700 (PDT) To: gdb@sourceware.org Subject: dcache.c write behaviour Message-Id: <20080806064306.300831C77BE@localhost> Date: Wed, 06 Aug 2008 06:43:00 -0000 From: dje@google.com (Doug Evans) X-IsSubscribed: yes Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org X-SW-Source: 2008-08/txt/msg00092.txt.bz2 Hi. dcache.c's support for writes seems busted and methinks the right fix is to throw out all handling of dirty lines until someone actually implements this: /* FIXME: There may be some benefit from moving the cache writeback to a higher layer, as it could occur after a sequence of smaller writes have been completed (as when a stack frame is constructed for an inferior function call). Note that only moving it up one level to target_xfer_memory() (also target_xfer_memory_partial()) is not sufficent, since we want to coalesce memory transfers that are "logically" connected but not actually a single call to one of the memory transfer functions. */ Consider: (gdb) set *(char*) 0 = 0 Cannot access memory at address 0x0 (gdb) p *(char*) 0 Cannot access memory at address 0x0 (gdb) mem 0 0xffffffff rw 8 cache (gdb) set *(char*) 0 = 0 (gdb) Where's the error from the second write? It's gone because the return code of dcache_writeback is ignore here: if (should_write) dcache_writeback (dcache); Even if the return code wasn't ignored one couldn't return a proper value which is the number of bytes actually written because the information isn't available at this point. Furthermore, the dcache now contains a valid value for *(char*) 0: (gdb) p *(char*) 0 $1 = 0 '\0' (gdb) There is this comment at the top of the file: The ENTRY_DIRTY state is necessary because GDB likes to write large lumps of memory in small bits. If the caching mechanism didn't maintain the DIRTY information, then something like a two byte write would mean that the entire cache line would have to be read, the two bytes modified and then written out again. The alternative would be to not read in the cache line in the first place, and just write the two bytes directly into target memory. The trouble with that is that it really nails performance, because of the remote protocol overhead. This way, all those little writes are bundled up into an entire cache line write in one go, without having to read the cache line in the first place. I understand the attempt at speeding up writes, but given that the cache is currently flushed after every write, there currently is no win for writes. I think caching of reads is far more valuable than caching of writes (at least until caching of writes works), so I propose fixing the above missing error by having dcache_xfer_memory first write to the target and then record in the cache only the bytes that were successfully written. Doing this means there's no such thing as a dirty line. We _could_ keep the code that tracks dirty lines, but it'd be easy to bring back said code if/when the above FIXME is implemented and until then the code would be easier to understand if we removed it. Comments?