From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrea Arcangeli Subject: Re: [PATCH 2 of 5] add can_dma/post_dma for direct IO Date: Fri, 12 Dec 2008 20:37:43 +0100 Message-ID: <20081212193743.GC30537@random.random> References: <4942B841.6010900@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Gerd Hoffmann , qemu-devel@nongnu.org, kvm@vger.kernel.org, avi@redhat.com, chrisw@redhat.com To: Anthony Liguori Return-path: Received: from mx2.redhat.com ([66.187.237.31]:40202 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751116AbYLLThu (ORCPT ); Fri, 12 Dec 2008 14:37:50 -0500 Content-Disposition: inline In-Reply-To: <4942B841.6010900@codemonkey.ws> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, Dec 12, 2008 at 01:15:13PM -0600, Anthony Liguori wrote: > 1) You attempt to map a physical address. This effectively is a lock or > pin operation. lock or pin for what? There's nothing to pin or lock here. Perhaps one day we'll have to lock or pin against something, dunno, then we'll add whatever pin or lock, otherwise why don't we also thread qcow2 while we're at it, sounds more worth it than adding a lock or pin that isn't actually needed. > a) In the process of this, you get a virtual address that you can > manipulate. That's what can_dma does. If it can, it generates a dma address backed by ram it does. But if it can't, it won't. This only works for direct I/O. > 2) You do your IO to the virtual address This is done by dma.c. > 3) You indicate how much of the memory you have dirtied That is done by the post_dma handler called by dma.c. > 4) You unmap or unlock that memory region. The virtual address is now no > longer valid. Again not needed. > This could correspond to a: > > void *cpu_physical_memory_map(target_phys_addr_t addr, ram_addr_t size, int > is_write); > > void cpu_physical_memory_unmap(target_physical_addr_t addr, ram_addr_t > size, void *mapping, int is_dirty); So you mean renaming can_dma to map and post_dma to unmap and not adding any lock/pin at all that would otherwise gratuitously slow it down? I've no idea why you should ever want to call any unmap for reads though... and calling it with your naming conventions, and not invoking it for reads (there's absolutely no good reason to invoke any unmap methods for reads, and it can only hurt at runtime) would be weird in my view.