From mboxrd@z Thu Jan 1 00:00:00 1970 From: Benjamin Herrenschmidt Date: Wed, 24 Jul 2013 23:13:44 +0000 Subject: Re: [PATCH 04/10] powerpc: Prepare to support kernel handling of IOMMU map/unmap Message-Id: <1374707624.6142.16.camel@pasglop> List-Id: References: <1373936045-22653-1-git-send-email-aik@ozlabs.ru> <1373936045-22653-5-git-send-email-aik@ozlabs.ru> <51EDE903.6010608@ozlabs.ru> <20130724154301.2af75867c51870fc0c32819b@linux-foundation.org> In-Reply-To: <20130724154301.2af75867c51870fc0c32819b@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Andrew Morton Cc: Alexey Kardashevskiy , linuxppc-dev@lists.ozlabs.org, David Gibson , Paul Mackerras , Alexander Graf , Alex Williamson , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, kvm-ppc@vger.kernel.org, linux-mm@kvack.org, Yasuaki Ishimatsu , Martin Schwidefsky , Andrea Arcangeli , Mel Gorman On Wed, 2013-07-24 at 15:43 -0700, Andrew Morton wrote: > For what? The three lines of comment in page-flags.h? ack :) > > Manipulating page->_count directly is considered poor form. Don't > blame us if we break your code ;) > > Actually, the manipulation in realmode_get_page() duplicates the > existing get_page_unless_zero() and the one in realmode_put_page() > could perhaps be placed in mm.h with a suitable name and some > documentation. That would improve your form and might protect the code > from getting broken later on. Yes, this stuff makes me really nervous :-) If it didn't provide an order of magnitude performance improvement in KVM I would avoid it but heh... Alexey, I like having that stuff in generic code. However the meaning of the words "real mode" can be ambiguous accross architectures, it might be best to then name it "mmu_off_put_page" to make things a bit clearer, along with a comment explaining that this is called in a context where none of the virtual mappings are accessible (vmalloc, vmemmap, IOs, ...), and that in the case of sparsemem vmemmap the caller must have taken care of getting the physical address of the struct page and of ensuring it isn't split accross two vmemmap blocks. Cheers, Ben. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 555EA2C007C for ; Thu, 25 Jul 2013 09:14:14 +1000 (EST) Message-ID: <1374707624.6142.16.camel@pasglop> Subject: Re: [PATCH 04/10] powerpc: Prepare to support kernel handling of IOMMU map/unmap From: Benjamin Herrenschmidt To: Andrew Morton Date: Thu, 25 Jul 2013 09:13:44 +1000 In-Reply-To: <20130724154301.2af75867c51870fc0c32819b@linux-foundation.org> References: <1373936045-22653-1-git-send-email-aik@ozlabs.ru> <1373936045-22653-5-git-send-email-aik@ozlabs.ru> <51EDE903.6010608@ozlabs.ru> <20130724154301.2af75867c51870fc0c32819b@linux-foundation.org> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Cc: Andrea Arcangeli , kvm@vger.kernel.org, Yasuaki Ishimatsu , Alexey Kardashevskiy , Alexander Graf , kvm-ppc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Alex Williamson , Paul Mackerras , Mel Gorman , Martin Schwidefsky , linuxppc-dev@lists.ozlabs.org, David Gibson List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Wed, 2013-07-24 at 15:43 -0700, Andrew Morton wrote: > For what? The three lines of comment in page-flags.h? ack :) > > Manipulating page->_count directly is considered poor form. Don't > blame us if we break your code ;) > > Actually, the manipulation in realmode_get_page() duplicates the > existing get_page_unless_zero() and the one in realmode_put_page() > could perhaps be placed in mm.h with a suitable name and some > documentation. That would improve your form and might protect the code > from getting broken later on. Yes, this stuff makes me really nervous :-) If it didn't provide an order of magnitude performance improvement in KVM I would avoid it but heh... Alexey, I like having that stuff in generic code. However the meaning of the words "real mode" can be ambiguous accross architectures, it might be best to then name it "mmu_off_put_page" to make things a bit clearer, along with a comment explaining that this is called in a context where none of the virtual mappings are accessible (vmalloc, vmemmap, IOs, ...), and that in the case of sparsemem vmemmap the caller must have taken care of getting the physical address of the struct page and of ensuring it isn't split accross two vmemmap blocks. Cheers, Ben. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Benjamin Herrenschmidt Subject: Re: [PATCH 04/10] powerpc: Prepare to support kernel handling of IOMMU map/unmap Date: Thu, 25 Jul 2013 09:13:44 +1000 Message-ID: <1374707624.6142.16.camel@pasglop> References: <1373936045-22653-1-git-send-email-aik@ozlabs.ru> <1373936045-22653-5-git-send-email-aik@ozlabs.ru> <51EDE903.6010608@ozlabs.ru> <20130724154301.2af75867c51870fc0c32819b@linux-foundation.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: Alexey Kardashevskiy , linuxppc-dev@lists.ozlabs.org, David Gibson , Paul Mackerras , Alexander Graf , Alex Williamson , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, kvm-ppc@vger.kernel.org, linux-mm@kvack.org, Yasuaki Ishimatsu , Martin Schwidefsky , Andrea Arcangeli , Mel Gorman To: Andrew Morton Return-path: In-Reply-To: <20130724154301.2af75867c51870fc0c32819b@linux-foundation.org> Sender: owner-linux-mm@kvack.org List-Id: kvm.vger.kernel.org On Wed, 2013-07-24 at 15:43 -0700, Andrew Morton wrote: > For what? The three lines of comment in page-flags.h? ack :) > > Manipulating page->_count directly is considered poor form. Don't > blame us if we break your code ;) > > Actually, the manipulation in realmode_get_page() duplicates the > existing get_page_unless_zero() and the one in realmode_put_page() > could perhaps be placed in mm.h with a suitable name and some > documentation. That would improve your form and might protect the code > from getting broken later on. Yes, this stuff makes me really nervous :-) If it didn't provide an order of magnitude performance improvement in KVM I would avoid it but heh... Alexey, I like having that stuff in generic code. However the meaning of the words "real mode" can be ambiguous accross architectures, it might be best to then name it "mmu_off_put_page" to make things a bit clearer, along with a comment explaining that this is called in a context where none of the virtual mappings are accessible (vmalloc, vmemmap, IOs, ...), and that in the case of sparsemem vmemmap the caller must have taken care of getting the physical address of the struct page and of ensuring it isn't split accross two vmemmap blocks. Cheers, Ben. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754140Ab3GXXOa (ORCPT ); Wed, 24 Jul 2013 19:14:30 -0400 Received: from gate.crashing.org ([63.228.1.57]:39137 "EHLO gate.crashing.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752418Ab3GXXO2 (ORCPT ); Wed, 24 Jul 2013 19:14:28 -0400 Message-ID: <1374707624.6142.16.camel@pasglop> Subject: Re: [PATCH 04/10] powerpc: Prepare to support kernel handling of IOMMU map/unmap From: Benjamin Herrenschmidt To: Andrew Morton Cc: Alexey Kardashevskiy , linuxppc-dev@lists.ozlabs.org, David Gibson , Paul Mackerras , Alexander Graf , Alex Williamson , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, kvm-ppc@vger.kernel.org, linux-mm@kvack.org, Yasuaki Ishimatsu , Martin Schwidefsky , Andrea Arcangeli , Mel Gorman Date: Thu, 25 Jul 2013 09:13:44 +1000 In-Reply-To: <20130724154301.2af75867c51870fc0c32819b@linux-foundation.org> References: <1373936045-22653-1-git-send-email-aik@ozlabs.ru> <1373936045-22653-5-git-send-email-aik@ozlabs.ru> <51EDE903.6010608@ozlabs.ru> <20130724154301.2af75867c51870fc0c32819b@linux-foundation.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.6.4-0ubuntu1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2013-07-24 at 15:43 -0700, Andrew Morton wrote: > For what? The three lines of comment in page-flags.h? ack :) > > Manipulating page->_count directly is considered poor form. Don't > blame us if we break your code ;) > > Actually, the manipulation in realmode_get_page() duplicates the > existing get_page_unless_zero() and the one in realmode_put_page() > could perhaps be placed in mm.h with a suitable name and some > documentation. That would improve your form and might protect the code > from getting broken later on. Yes, this stuff makes me really nervous :-) If it didn't provide an order of magnitude performance improvement in KVM I would avoid it but heh... Alexey, I like having that stuff in generic code. However the meaning of the words "real mode" can be ambiguous accross architectures, it might be best to then name it "mmu_off_put_page" to make things a bit clearer, along with a comment explaining that this is called in a context where none of the virtual mappings are accessible (vmalloc, vmemmap, IOs, ...), and that in the case of sparsemem vmemmap the caller must have taken care of getting the physical address of the struct page and of ensuring it isn't split accross two vmemmap blocks. Cheers, Ben.