From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Yang, Xiaowei" Subject: Re: [PATCH] Fix domheap structure allocation when NUMA=on Date: Tue, 24 Mar 2009 18:48:31 +0800 Message-ID: <49C8BA7F.5020405@intel.com> References: <49C32413.2030505@intel.com> <49C35EA9.76E4.0078.0@novell.com> <49C35517.7040706@intel.com> <49C36646.76E4.0078.0@novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------050304040408060001070407" Return-path: In-Reply-To: <49C36646.76E4.0078.0@novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich Cc: "xen-devel@lists.xensource.com" , Keir Fraser List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --------------050304040408060001070407 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Jan Beulich wrote: >>>> "Yang, Xiaowei" 20.03.09 09:34 >>> >> Jan Beulich wrote: >>>>>> "Yang, Xiaowei" 20.03.09 06:05 >>> >>>> DIRECTMAP_VIRT_END can't be passed to virt_to_mfn(), as it's just beyond >>>> direct map boundary and triggers ASSERT very early at boot time. >>> While I agree to the analysis, I would think that this >>> >>> + mfn + needed <= virt_to_mfn(DIRECTMAP_VIRT_END - PAGE_SIZE) ) >>> >>> should rather be >>> >>> + mfn + needed <= virt_to_mfn(DIRECTMAP_VIRT_END - 1) + 1 ) >>> >> virt_to_mfn(DIRECTMAP_VIRT_END - 1) is equal to > > Depending on whether DIRECTMAP_VIRT_END is the last byte or the first > following byte. Using "- 1" avoids such a dependency. > >> virt_to_mfn(DIRECTMAP_VIRT_END - PAGE_SIZE). Why +1? We use '<=' here. > > Because on the left side of the comparison we also calculate the first > following mfn, not the last included one. > Jan, Thanks for the clarification! Now I found another potential issue. Since mfn+needed could be equal to virt_to_mfn(DIRECTMAP_VIRT_END-1)+1, it can't be passed to mfn_to_virt() directly - the valid range is [0,DIRECT_MAP_BYTES>>PAGE_SHIFT). Here is the patch to fix it. More assert and 32 bit counterpart are added. Signed-off-by: Xiaowei Yang Thanks, xiaowei --------------050304040408060001070407 Content-Type: text/x-patch; name="mfn_to_virt.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="mfn_to_virt.patch" diff -r 0477f9061c8a xen/common/page_alloc.c --- a/xen/common/page_alloc.c Fri Mar 20 17:42:46 2009 +0000 +++ b/xen/common/page_alloc.c Mon Mar 23 03:04:18 2009 +0800 @@ -302,7 +302,8 @@ static unsigned long init_node_heap(int (mfn + needed) <= (virt_to_mfn(DIRECTMAP_VIRT_END - 1) + 1) ) { _heap[node] = mfn_to_virt(mfn); - avail[node] = mfn_to_virt(mfn + needed) - sizeof(**avail) * NR_ZONES; + avail[node] = mfn_to_virt(mfn + needed - 1) + + PAGE_SIZE - sizeof(**avail) * NR_ZONES; } #endif else if ( get_order_from_bytes(sizeof(**_heap)) == diff -r 0477f9061c8a xen/include/asm-x86/x86_32/page.h --- a/xen/include/asm-x86/x86_32/page.h Fri Mar 20 17:42:46 2009 +0000 +++ b/xen/include/asm-x86/x86_32/page.h Mon Mar 23 03:04:18 2009 +0800 @@ -27,9 +27,6 @@ #define __PAGE_OFFSET (0xFF000000) #define __XEN_VIRT_START __PAGE_OFFSET -#define virt_to_maddr(va) ((unsigned long)(va)-DIRECTMAP_VIRT_START) -#define maddr_to_virt(ma) ((void *)((unsigned long)(ma)+DIRECTMAP_VIRT_START)) - #define VADDR_BITS 32 #define VADDR_MASK (~0UL) @@ -43,6 +40,22 @@ #include #include + +static inline unsigned long __virt_to_maddr(unsigned long va) +{ + ASSERT(va >= DIRECTMAP_VIRT_START && va < DIRECTMAP_VIRT_END); + return va - DIRECTMAP_VIRT_START; +} +#define virt_to_maddr(va) \ + (__virt_to_maddr((unsigned long)(va))) + +static inline void *__maddr_to_virt(unsigned long ma) +{ + ASSERT(ma < DIRECTMAP_VIRT_END - DIRECTMAP_VIRT_START); + return (void *)(ma + DIRECTMAP_VIRT_START); +} +#define maddr_to_virt(ma) \ + (__maddr_to_virt((unsigned long)(ma))) /* read access (should only be used for debug printk's) */ typedef u64 intpte_t; diff -r 0477f9061c8a xen/include/asm-x86/x86_64/page.h --- a/xen/include/asm-x86/x86_64/page.h Fri Mar 20 17:42:46 2009 +0000 +++ b/xen/include/asm-x86/x86_64/page.h Mon Mar 23 03:04:42 2009 +0800 @@ -46,8 +46,14 @@ static inline unsigned long __virt_to_ma } #define virt_to_maddr(va) \ (__virt_to_maddr((unsigned long)(va))) + +static inline void *__maddr_to_virt(unsigned long ma) +{ + ASSERT(ma < DIRECTMAP_VIRT_END - DIRECTMAP_VIRT_START); + return (void *)(ma + DIRECTMAP_VIRT_START); +} #define maddr_to_virt(ma) \ - ((void *)((unsigned long)(ma)+DIRECTMAP_VIRT_START)) + (__maddr_to_virt((unsigned long)(ma))) /* read access (should only be used for debug printk's) */ typedef u64 intpte_t; --------------050304040408060001070407 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --------------050304040408060001070407--