[PATCH -v1] MIPS: fix pfn_valid() for FLATMEM

[PATCH -v1] MIPS: fix pfn_valid() for FLATMEM

[Wu Zhangjin]

When CONFIG_FLATMEM enabled, STD/Hiberation will fail on YeeLoong
laptop, This patch fixes it:

if pfn is between min_low_pfn and max_mapnr, the old pfn_valid() will
return TRUE, but if the memory is not continuous, for example:

$ cat /proc/iomem | grep "System RAM"
00000000-0fffffff : System RAM
90000000-bfffffff : System RAM

as we can see, it is not continuous, so, some of the memory is not
valid, and at last make STD/Hibernate fail when shrinking a too large
number of invalid memory.

the "invalid" memory here include the memory space we never used.

10000000-3fffffff
80000000-8fffffff

and also include the meory space we have mapped into pci space.

40000000-7fffffff : pci memory space

Here, we fix it via checking pfn is in the "System RAM" or not. and
Seems pfn_valid() is not called in assembly code, we move it to
"!__ASSEMBLY__" to ensure we can simply declare it via "extern int
pfn_valid(unsigned long)" without Compiling Error.

(This -v1 version incorporates feedback from Pavel Machek <pavel@ucw.cz>
 and Sergei Shtylyov <sshtylyov@ru.mvista.com> and Ralf)

Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
---
 arch/mips/include/asm/page.h |   50 ++++++++++++++++++-----------------------
 arch/mips/mm/page.c          |   18 +++++++++++++++
 2 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
index f266295..dc28d0a 100644
--- a/arch/mips/include/asm/page.h
+++ b/arch/mips/include/asm/page.h
@@ -146,36 +146,9 @@ typedef struct { unsigned long pgprot; } pgprot_t;
  */
 #define ptep_buddy(x)	((pte_t *)((unsigned long)(x) ^ sizeof(pte_t)))
 
-#endif /* !__ASSEMBLY__ */
-
-/*
- * __pa()/__va() should be used only during mem init.
- */
-#ifdef CONFIG_64BIT
-#define __pa(x)								\
-({									\
-    unsigned long __x = (unsigned long)(x);				\
-    __x < CKSEG0 ? XPHYSADDR(__x) : CPHYSADDR(__x);			\
-})
-#else
-#define __pa(x)								\
-    ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
-#endif
-#define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
-#define __pa_symbol(x)	__pa(RELOC_HIDE((unsigned long)(x), 0))
-
-#define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
-
 #ifdef CONFIG_FLATMEM
 
-#define pfn_valid(pfn)							\
-({									\
-	unsigned long __pfn = (pfn);					\
-	/* avoid <linux/bootmem.h> include hell */			\
-	extern unsigned long min_low_pfn;				\
-									\
-	__pfn >= min_low_pfn && __pfn < max_mapnr;			\
-})
+extern int pfn_valid(unsigned long);
 
 #elif defined(CONFIG_SPARSEMEM)
 
@@ -194,6 +167,27 @@ typedef struct { unsigned long pgprot; } pgprot_t;
 
 #endif
 
+
+#endif /* !__ASSEMBLY__ */
+
+/*
+ * __pa()/__va() should be used only during mem init.
+ */
+#ifdef CONFIG_64BIT
+#define __pa(x)								\
+({									\
+    unsigned long __x = (unsigned long)(x);				\
+    __x < CKSEG0 ? XPHYSADDR(__x) : CPHYSADDR(__x);			\
+})
+#else
+#define __pa(x)								\
+    ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
+#endif
+#define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
+#define __pa_symbol(x)	__pa(RELOC_HIDE((unsigned long)(x), 0))
+
+#define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
+
 #define virt_to_page(kaddr)	pfn_to_page(PFN_DOWN(virt_to_phys(kaddr)))
 #define virt_addr_valid(kaddr)	pfn_valid(PFN_DOWN(virt_to_phys(kaddr)))
 
diff --git a/arch/mips/mm/page.c b/arch/mips/mm/page.c
index f5c7375..203d805 100644
--- a/arch/mips/mm/page.c
+++ b/arch/mips/mm/page.c
@@ -689,3 +689,21 @@ void copy_page(void *to, void *from)
 }
 
 #endif /* CONFIG_SIBYTE_DMA_PAGEOPS */
+
+#ifdef CONFIG_FLATMEM
+int pfn_valid(unsigned long pfn)
+{
+	int i;
+
+	for (i = 0; i < boot_mem_map.nr_map; i++) {
+		if ((boot_mem_map.map[i].type == BOOT_MEM_RAM) ||
+			(boot_mem_map.map[i].type == BOOT_MEM_ROM_DATA)) {
+			if ((pfn >= PFN_DOWN(boot_mem_map.map[i].addr)) &&
+				(pfn < PFN_UP(boot_mem_map.map[i].addr +
+					boot_mem_map.map[i].size)))
+				return 1;
+		}
+	}
+	return 0;
+}
+#endif
-- 
1.6.2.1

Re: [PATCH -v1] MIPS: fix pfn_valid() for FLATMEM

[Wu Zhangjin]

On Sat, 2009-10-10 at 00:15 +0800, Wu Zhangjin wrote:
> When CONFIG_FLATMEM enabled, STD/Hiberation will fail on YeeLoong
> laptop, This patch fixes it:
> 
> if pfn is between min_low_pfn and max_mapnr, the old pfn_valid() will
> return TRUE, but if the memory is not continuous, for example:
> 
> $ cat /proc/iomem | grep "System RAM"
> 00000000-0fffffff : System RAM
> 90000000-bfffffff : System RAM
> 
> as we can see, it is not continuous, so, some of the memory is not
> valid, and at last make STD/Hibernate fail when shrinking a too large
> number of invalid memory.
> 
> the "invalid" memory here include the memory space we never used.
> 
> 10000000-3fffffff
> 80000000-8fffffff
> 
> and also include the meory space we have mapped into pci space.
> 
> 40000000-7fffffff : pci memory space
> 

what about the "pci memory space"?

> Here, we fix it via checking pfn is in the "System RAM" or not. and
> Seems pfn_valid() is not called in assembly code, we move it to
> "!__ASSEMBLY__" to ensure we can simply declare it via "extern int
> pfn_valid(unsigned long)" without Compiling Error.
> 
> (This -v1 version incorporates feedback from Pavel Machek <pavel@ucw.cz>
>  and Sergei Shtylyov <sshtylyov@ru.mvista.com> and Ralf)
> 
> Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
> ---
>  arch/mips/include/asm/page.h |   50 ++++++++++++++++++-----------------------
>  arch/mips/mm/page.c          |   18 +++++++++++++++
>  2 files changed, 40 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
> index f266295..dc28d0a 100644
> --- a/arch/mips/include/asm/page.h
> +++ b/arch/mips/include/asm/page.h
> @@ -146,36 +146,9 @@ typedef struct { unsigned long pgprot; } pgprot_t;
>   */
>  #define ptep_buddy(x)	((pte_t *)((unsigned long)(x) ^ sizeof(pte_t)))
>  
> -#endif /* !__ASSEMBLY__ */
> -
> -/*
> - * __pa()/__va() should be used only during mem init.
> - */
> -#ifdef CONFIG_64BIT
> -#define __pa(x)								\
> -({									\
> -    unsigned long __x = (unsigned long)(x);				\
> -    __x < CKSEG0 ? XPHYSADDR(__x) : CPHYSADDR(__x);			\
> -})
> -#else
> -#define __pa(x)								\
> -    ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
> -#endif
> -#define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
> -#define __pa_symbol(x)	__pa(RELOC_HIDE((unsigned long)(x), 0))
> -
> -#define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
> -
>  #ifdef CONFIG_FLATMEM
>  
> -#define pfn_valid(pfn)							\
> -({									\
> -	unsigned long __pfn = (pfn);					\
> -	/* avoid <linux/bootmem.h> include hell */			\
> -	extern unsigned long min_low_pfn;				\
> -									\
> -	__pfn >= min_low_pfn && __pfn < max_mapnr;			\
> -})
> +extern int pfn_valid(unsigned long);
>  
>  #elif defined(CONFIG_SPARSEMEM)
>  
> @@ -194,6 +167,27 @@ typedef struct { unsigned long pgprot; } pgprot_t;
>  
>  #endif
>  
> +
> +#endif /* !__ASSEMBLY__ */
> +
> +/*
> + * __pa()/__va() should be used only during mem init.
> + */
> +#ifdef CONFIG_64BIT
> +#define __pa(x)								\
> +({									\
> +    unsigned long __x = (unsigned long)(x);				\
> +    __x < CKSEG0 ? XPHYSADDR(__x) : CPHYSADDR(__x);			\
> +})
> +#else
> +#define __pa(x)								\
> +    ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
> +#endif
> +#define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
> +#define __pa_symbol(x)	__pa(RELOC_HIDE((unsigned long)(x), 0))
> +
> +#define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
> +
>  #define virt_to_page(kaddr)	pfn_to_page(PFN_DOWN(virt_to_phys(kaddr)))
>  #define virt_addr_valid(kaddr)	pfn_valid(PFN_DOWN(virt_to_phys(kaddr)))
>  
> diff --git a/arch/mips/mm/page.c b/arch/mips/mm/page.c
> index f5c7375..203d805 100644
> --- a/arch/mips/mm/page.c
> +++ b/arch/mips/mm/page.c
> @@ -689,3 +689,21 @@ void copy_page(void *to, void *from)
>  }
>  
>  #endif /* CONFIG_SIBYTE_DMA_PAGEOPS */
> +
> +#ifdef CONFIG_FLATMEM
> +int pfn_valid(unsigned long pfn)
> +{
> +	int i;
> +
> +	for (i = 0; i < boot_mem_map.nr_map; i++) {
> +		if ((boot_mem_map.map[i].type == BOOT_MEM_RAM) ||
> +			(boot_mem_map.map[i].type == BOOT_MEM_ROM_DATA)) {
> +			if ((pfn >= PFN_DOWN(boot_mem_map.map[i].addr)) &&
> +				(pfn < PFN_UP(boot_mem_map.map[i].addr +
> +					boot_mem_map.map[i].size)))
> +				return 1;
> +		}
> +	}
> +	return 0;
> +}
> +#endif

Re: [PATCH -v1] MIPS: fix pfn_valid() for FLATMEM

[Wu Zhangjin]

Hello, all

Please ignore This Patch, And I have tried to register_nosave_region
from 20000000 to 7fffffff(include the reserved page and pci memory
space), it will make the wifi driver not work(read/write_nic_word fail,
DMA relative, something bad with pci memory space), So I choose the last
choice, come back to the oldest patch I have sent: "Hibernation: only
save pages in system ram".

Regards,
	Wu Zhangjin

On Sat, 2009-10-10 at 00:15 +0800, Wu Zhangjin wrote:
> When CONFIG_FLATMEM enabled, STD/Hiberation will fail on YeeLoong
> laptop, This patch fixes it:
> 
> if pfn is between min_low_pfn and max_mapnr, the old pfn_valid() will
> return TRUE, but if the memory is not continuous, for example:
> 
> $ cat /proc/iomem | grep "System RAM"
> 00000000-0fffffff : System RAM
> 90000000-bfffffff : System RAM
> 
> as we can see, it is not continuous, so, some of the memory is not
> valid, and at last make STD/Hibernate fail when shrinking a too large
> number of invalid memory.
> 
> the "invalid" memory here include the memory space we never used.
> 
> 10000000-3fffffff
> 80000000-8fffffff
> 
> and also include the meory space we have mapped into pci space.
> 
> 40000000-7fffffff : pci memory space
> 
> Here, we fix it via checking pfn is in the "System RAM" or not. and
> Seems pfn_valid() is not called in assembly code, we move it to
> "!__ASSEMBLY__" to ensure we can simply declare it via "extern int
> pfn_valid(unsigned long)" without Compiling Error.
> 
> (This -v1 version incorporates feedback from Pavel Machek <pavel@ucw.cz>
>  and Sergei Shtylyov <sshtylyov@ru.mvista.com> and Ralf)
> 
> Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
> ---
>  arch/mips/include/asm/page.h |   50 ++++++++++++++++++-----------------------
>  arch/mips/mm/page.c          |   18 +++++++++++++++
>  2 files changed, 40 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
> index f266295..dc28d0a 100644
> --- a/arch/mips/include/asm/page.h
> +++ b/arch/mips/include/asm/page.h
> @@ -146,36 +146,9 @@ typedef struct { unsigned long pgprot; } pgprot_t;
>   */
>  #define ptep_buddy(x)	((pte_t *)((unsigned long)(x) ^ sizeof(pte_t)))
>  
> -#endif /* !__ASSEMBLY__ */
> -
> -/*
> - * __pa()/__va() should be used only during mem init.
> - */
> -#ifdef CONFIG_64BIT
> -#define __pa(x)								\
> -({									\
> -    unsigned long __x = (unsigned long)(x);				\
> -    __x < CKSEG0 ? XPHYSADDR(__x) : CPHYSADDR(__x);			\
> -})
> -#else
> -#define __pa(x)								\
> -    ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
> -#endif
> -#define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
> -#define __pa_symbol(x)	__pa(RELOC_HIDE((unsigned long)(x), 0))
> -
> -#define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
> -
>  #ifdef CONFIG_FLATMEM
>  
> -#define pfn_valid(pfn)							\
> -({									\
> -	unsigned long __pfn = (pfn);					\
> -	/* avoid <linux/bootmem.h> include hell */			\
> -	extern unsigned long min_low_pfn;				\
> -									\
> -	__pfn >= min_low_pfn && __pfn < max_mapnr;			\
> -})
> +extern int pfn_valid(unsigned long);
>  
>  #elif defined(CONFIG_SPARSEMEM)
>  
> @@ -194,6 +167,27 @@ typedef struct { unsigned long pgprot; } pgprot_t;
>  
>  #endif
>  
> +
> +#endif /* !__ASSEMBLY__ */
> +
> +/*
> + * __pa()/__va() should be used only during mem init.
> + */
> +#ifdef CONFIG_64BIT
> +#define __pa(x)								\
> +({									\
> +    unsigned long __x = (unsigned long)(x);				\
> +    __x < CKSEG0 ? XPHYSADDR(__x) : CPHYSADDR(__x);			\
> +})
> +#else
> +#define __pa(x)								\
> +    ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
> +#endif
> +#define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
> +#define __pa_symbol(x)	__pa(RELOC_HIDE((unsigned long)(x), 0))
> +
> +#define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
> +
>  #define virt_to_page(kaddr)	pfn_to_page(PFN_DOWN(virt_to_phys(kaddr)))
>  #define virt_addr_valid(kaddr)	pfn_valid(PFN_DOWN(virt_to_phys(kaddr)))
>  
> diff --git a/arch/mips/mm/page.c b/arch/mips/mm/page.c
> index f5c7375..203d805 100644
> --- a/arch/mips/mm/page.c
> +++ b/arch/mips/mm/page.c
> @@ -689,3 +689,21 @@ void copy_page(void *to, void *from)
>  }
>  
>  #endif /* CONFIG_SIBYTE_DMA_PAGEOPS */
> +
> +#ifdef CONFIG_FLATMEM
> +int pfn_valid(unsigned long pfn)
> +{
> +	int i;
> +
> +	for (i = 0; i < boot_mem_map.nr_map; i++) {
> +		if ((boot_mem_map.map[i].type == BOOT_MEM_RAM) ||
> +			(boot_mem_map.map[i].type == BOOT_MEM_ROM_DATA)) {
> +			if ((pfn >= PFN_DOWN(boot_mem_map.map[i].addr)) &&
> +				(pfn < PFN_UP(boot_mem_map.map[i].addr +
> +					boot_mem_map.map[i].size)))
> +				return 1;
> +		}
> +	}
> +	return 0;
> +}
> +#endif

Re: [PATCH -v1] MIPS: fix pfn_valid() for FLATMEM

[Wu Zhangjin]

On Sat, 2009-10-10 at 22:38 +0800, Wu Zhangjin wrote:
> Hello, all
> 
> Please ignore This Patch, And I have tried to register_nosave_region
> from 20000000 to 7fffffff(include the reserved page and pci memory
> space), it will make the wifi driver not work(read/write_nic_word fail,
> DMA relative, something bad with pci memory space), So I choose the last
> choice, come back to the oldest patch I have sent: "Hibernation: only
> save pages in system ram".
> 
Sorry to disturb you again :-)

Just talked with another guy who are also playing with the Hibernation
support of Yeeloong laptop, he told me the bug existed before my
pfn_valid() patch, that is:

After Hibernation and resuming it, the rtl8187b wifi will not work, 

...
[  338.172000] usb 1-1: reset high speed USB device using ehci_hcd and
address 2
[  338.752000] usb 2-1: reset high speed USB device using ehci_hcd and
address 2
[  338.892000] rtl8187: SCI interrupt Methord Will Turn Radio On
[  338.892000] read_nic_byte TimeOut!addr:50, status:ffffff6c
[  338.892000] write_nic_byte TimeOut!addr:50, status:ffffff6c
[  338.892000] read_nic_byte TimeOut!addr:59, status:ffffff6c
[  338.892000] write_nic_byte TimeOut!addr:59, status:ffffff6c
[  338.892000] rtl8187: Now Radio ON!
[  338.892000] write_nic_dword TimeOut!addr:54, status:ffffff6c
[  338.892000] write_nic_dword TimeOut!addr:60, status:ffffff6c
[  338.892000] write_nic_byte TimeOut!addr:85, status:ffffff6c
[  338.892000] read_nic_word TimeOut!addr:80, status:ffffff6c
[  338.892000] read_nic_word TimeOut!addr:84, status:ffffff6c
[  338.892000] write_nic_word TimeOut!addr:84, status:ffffff6c
[  338.892000] write_nic_word TimeOut!addr:80, status:ffffff6c
[  338.892000] write_nic_word TimeOut!addr:80, status:ffffff6c
[  338.892000] write_nic_word TimeOut!addr:80, status:ffffff6c
[  338.892000] write_nic_word TimeOut!addr:84, status:ffffff6c
[  338.892000] write_nic_byte TimeOut!addr:61, status:ffffff6c
[  338.892000] read_nic_byte TimeOut!addr:62, status:ffffff6c
[  338.892000] write_nic_byte TimeOut!addr:62, status:ffffff6c
[  338.892000] write_nic_byte TimeOut!addr:62, status:ffffff6c
[  338.892000] read_nic_byte TimeOut!addr:24e, status:ffffff6c
[  338.892000] write_nic_byte TimeOut!addr:24e, status:ffffff6c
[  338.892000] write_nic_byte TimeOut!addr:59, status:ffffff6c
[  338.892000] write_nic_byte TimeOut!addr:50, status:ffffff6c
[  339.004000] usb 2-4: reset high speed USB device using ehci_hcd and
address 3
[  339.136000] rtl8187 2-4:1.0: no reset_resume for driver rtl8187?
[  339.396000] rtl8187: Now Radio OFF!
[  339.408000] rtl8187: Card successfully reset
[  339.420000] rtl8187: wlan driver removed
[  339.472000] rtl8187: idProduct:0x8189, bcdDevice:0x200
[  339.520000] rtl8187: Channel plan is 0 
[  339.524000] rtl8187: Reported EEPROM chip is a 93c46 (1Kbit)
[  339.824000] rtl8187: Card MAC address is 00:17:c4:5a:1b:f9
[  340.332000] rtl8187: EEPROM Customer ID: 00
[  340.336000] There is a handler installed for event: 48
[  340.336000] rtl8187: SCI interrupt Methord Will Turn Radio Off
[  340.340000] rtl8187: Driver probe completed
[  340.344000] Restarting tasks ... 
[  340.396000] rtl8187: rtl8187_open process failed because radio off
[  340.408000] done.
[  340.476000] rtl8187: rtl8187_open process failed because radio off
...

(I tried to turn on the radio, can not turn it on)

but if he did something like this, it will "fix" the bug.

# before hibernation, remove the r8187 driver 
modprobe -r r8187

# enter into hibernation
echo disk > /sys/power/state

# ????
echo 0 >/sys/bus/usb/drivers_autoprobe
echo 4 >/sys/class/usb_host/usb_host2/companion
echo 1 >/sys/class/usb_host/usb_host1/companion
sleep 1
echo 1 >/sys/bus/usb/drivers_autoprobe
echo -4 >/sys/class/usb_host/usb_host2/companion
echo -1 >/sys/class/usb_host/usb_host1/companion
sleep 1  #wait for some devices stable

# reinsert the r8187 driver
modprobe r8187

So, that pfn_valid() should be okay, it made Hibernation works with
FLATMEM and have no side effect currently.

Regards,
	Wu Zhangjin

> 
> On Sat, 2009-10-10 at 00:15 +0800, Wu Zhangjin wrote:
> > When CONFIG_FLATMEM enabled, STD/Hiberation will fail on YeeLoong
> > laptop, This patch fixes it:
> > 
> > if pfn is between min_low_pfn and max_mapnr, the old pfn_valid() will
> > return TRUE, but if the memory is not continuous, for example:
> > 
> > $ cat /proc/iomem | grep "System RAM"
> > 00000000-0fffffff : System RAM
> > 90000000-bfffffff : System RAM
> > 
> > as we can see, it is not continuous, so, some of the memory is not
> > valid, and at last make STD/Hibernate fail when shrinking a too large
> > number of invalid memory.
> > 
> > the "invalid" memory here include the memory space we never used.
> > 
> > 10000000-3fffffff
> > 80000000-8fffffff
> > 
> > and also include the meory space we have mapped into pci space.
> > 
> > 40000000-7fffffff : pci memory space
> > 
> > Here, we fix it via checking pfn is in the "System RAM" or not. and
> > Seems pfn_valid() is not called in assembly code, we move it to
> > "!__ASSEMBLY__" to ensure we can simply declare it via "extern int
> > pfn_valid(unsigned long)" without Compiling Error.
> > 
> > (This -v1 version incorporates feedback from Pavel Machek <pavel@ucw.cz>
> >  and Sergei Shtylyov <sshtylyov@ru.mvista.com> and Ralf)
> > 
> > Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
> > ---
> >  arch/mips/include/asm/page.h |   50 ++++++++++++++++++-----------------------
> >  arch/mips/mm/page.c          |   18 +++++++++++++++
> >  2 files changed, 40 insertions(+), 28 deletions(-)
> > 
> > diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
> > index f266295..dc28d0a 100644
> > --- a/arch/mips/include/asm/page.h
> > +++ b/arch/mips/include/asm/page.h
> > @@ -146,36 +146,9 @@ typedef struct { unsigned long pgprot; } pgprot_t;
> >   */
> >  #define ptep_buddy(x)	((pte_t *)((unsigned long)(x) ^ sizeof(pte_t)))
> >  
> > -#endif /* !__ASSEMBLY__ */
> > -
> > -/*
> > - * __pa()/__va() should be used only during mem init.
> > - */
> > -#ifdef CONFIG_64BIT
> > -#define __pa(x)								\
> > -({									\
> > -    unsigned long __x = (unsigned long)(x);				\
> > -    __x < CKSEG0 ? XPHYSADDR(__x) : CPHYSADDR(__x);			\
> > -})
> > -#else
> > -#define __pa(x)								\
> > -    ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
> > -#endif
> > -#define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
> > -#define __pa_symbol(x)	__pa(RELOC_HIDE((unsigned long)(x), 0))
> > -
> > -#define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
> > -
> >  #ifdef CONFIG_FLATMEM
> >  
> > -#define pfn_valid(pfn)							\
> > -({									\
> > -	unsigned long __pfn = (pfn);					\
> > -	/* avoid <linux/bootmem.h> include hell */			\
> > -	extern unsigned long min_low_pfn;				\
> > -									\
> > -	__pfn >= min_low_pfn && __pfn < max_mapnr;			\
> > -})
> > +extern int pfn_valid(unsigned long);
> >  
> >  #elif defined(CONFIG_SPARSEMEM)
> >  
> > @@ -194,6 +167,27 @@ typedef struct { unsigned long pgprot; } pgprot_t;
> >  
> >  #endif
> >  
> > +
> > +#endif /* !__ASSEMBLY__ */
> > +
> > +/*
> > + * __pa()/__va() should be used only during mem init.
> > + */
> > +#ifdef CONFIG_64BIT
> > +#define __pa(x)								\
> > +({									\
> > +    unsigned long __x = (unsigned long)(x);				\
> > +    __x < CKSEG0 ? XPHYSADDR(__x) : CPHYSADDR(__x);			\
> > +})
> > +#else
> > +#define __pa(x)								\
> > +    ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
> > +#endif
> > +#define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
> > +#define __pa_symbol(x)	__pa(RELOC_HIDE((unsigned long)(x), 0))
> > +
> > +#define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
> > +
> >  #define virt_to_page(kaddr)	pfn_to_page(PFN_DOWN(virt_to_phys(kaddr)))
> >  #define virt_addr_valid(kaddr)	pfn_valid(PFN_DOWN(virt_to_phys(kaddr)))
> >  
> > diff --git a/arch/mips/mm/page.c b/arch/mips/mm/page.c
> > index f5c7375..203d805 100644
> > --- a/arch/mips/mm/page.c
> > +++ b/arch/mips/mm/page.c
> > @@ -689,3 +689,21 @@ void copy_page(void *to, void *from)
> >  }
> >  
> >  #endif /* CONFIG_SIBYTE_DMA_PAGEOPS */
> > +
> > +#ifdef CONFIG_FLATMEM
> > +int pfn_valid(unsigned long pfn)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < boot_mem_map.nr_map; i++) {
> > +		if ((boot_mem_map.map[i].type == BOOT_MEM_RAM) ||
> > +			(boot_mem_map.map[i].type == BOOT_MEM_ROM_DATA)) {
> > +			if ((pfn >= PFN_DOWN(boot_mem_map.map[i].addr)) &&
> > +				(pfn < PFN_UP(boot_mem_map.map[i].addr +
> > +					boot_mem_map.map[i].size)))
> > +				return 1;
> > +		}
> > +	}
> > +	return 0;
> > +}
> > +#endif

[PATCH -v1] MIPS: fix pfn_valid() for FLATMEM

[Wu Zhangjin]

When CONFIG_FLATMEM enabled, STD/Hiberation will fail on YeeLoong
laptop, This patch fixes it:

if pfn is between min_low_pfn and max_mapnr, the old pfn_valid() will
return TRUE, but in reality, if the memory is not continuous, it should
be false. for example:

$ cat /proc/iomem | grep "System RAM"
00000000-0fffffff : System RAM
90000000-bfffffff : System RAM

as we can see, it is not continuous, so, some of the memory is not valid
but regarded as valid by pfn_valid(), and at last make STD/Hibernate
fail when shrinking a too large number of invalid memory.

Here, we fix it via checking pfn is in the "System RAM" or not. and
Seems pfn_valid() is not called in assembly code, we move it to
"!__ASSEMBLY__" to ensure we can simply declare it via "extern int
pfn_valid(unsigned long)" without Compiling Error.

(This -v1 version incorporates feedback from Pavel Machek <pavel@ucw.cz>
 and Sergei Shtylyov <sshtylyov@ru.mvista.com>)

Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
---
 arch/mips/include/asm/page.h |   50 ++++++++++++++++++-----------------------
 arch/mips/mm/page.c          |   17 ++++++++++++++
 2 files changed, 39 insertions(+), 28 deletions(-)

diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
index 4320239..ef43905 100644
--- a/arch/mips/include/asm/page.h
+++ b/arch/mips/include/asm/page.h
@@ -145,36 +145,9 @@ typedef struct { unsigned long pgprot; } pgprot_t;
  */
 #define ptep_buddy(x)	((pte_t *)((unsigned long)(x) ^ sizeof(pte_t)))
 
-#endif /* !__ASSEMBLY__ */
-
-/*
- * __pa()/__va() should be used only during mem init.
- */
-#ifdef CONFIG_64BIT
-#define __pa(x)								\
-({									\
-    unsigned long __x = (unsigned long)(x);				\
-    __x < CKSEG0 ? XPHYSADDR(__x) : CPHYSADDR(__x);			\
-})
-#else
-#define __pa(x)								\
-    ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
-#endif
-#define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
-#define __pa_symbol(x)	__pa(RELOC_HIDE((unsigned long)(x), 0))
-
-#define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
-
 #ifdef CONFIG_FLATMEM
 
-#define pfn_valid(pfn)							\
-({									\
-	unsigned long __pfn = (pfn);					\
-	/* avoid <linux/bootmem.h> include hell */			\
-	extern unsigned long min_low_pfn;				\
-									\
-	__pfn >= min_low_pfn && __pfn < max_mapnr;			\
-})
+extern int pfn_valid(unsigned long);
 
 #elif defined(CONFIG_SPARSEMEM)
 
@@ -193,6 +166,27 @@ typedef struct { unsigned long pgprot; } pgprot_t;
 
 #endif
 
+
+#endif /* !__ASSEMBLY__ */
+
+/*
+ * __pa()/__va() should be used only during mem init.
+ */
+#ifdef CONFIG_64BIT
+#define __pa(x)								\
+({									\
+    unsigned long __x = (unsigned long)(x);				\
+    __x < CKSEG0 ? XPHYSADDR(__x) : CPHYSADDR(__x);			\
+})
+#else
+#define __pa(x)								\
+    ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
+#endif
+#define __va(x)		((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
+#define __pa_symbol(x)	__pa(RELOC_HIDE((unsigned long)(x), 0))
+
+#define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
+
 #define virt_to_page(kaddr)	pfn_to_page(PFN_DOWN(virt_to_phys(kaddr)))
 #define virt_addr_valid(kaddr)	pfn_valid(PFN_DOWN(virt_to_phys(kaddr)))
 
diff --git a/arch/mips/mm/page.c b/arch/mips/mm/page.c
index f5c7375..e0a3f72 100644
--- a/arch/mips/mm/page.c
+++ b/arch/mips/mm/page.c
@@ -689,3 +689,20 @@ void copy_page(void *to, void *from)
 }
 
 #endif /* CONFIG_SIBYTE_DMA_PAGEOPS */
+
+#ifdef CONFIG_FLATMEM
+int pfn_valid(unsigned long pfn)
+{
+	int i;
+
+	for (i = 0; i < boot_mem_map.nr_map; i++) {
+		if (boot_mem_map.map[i].type == BOOT_MEM_RAM) {
+			if ((pfn >= PFN_DOWN(boot_mem_map.map[i].addr)) &&
+				(pfn < PFN_UP(boot_mem_map.map[i].addr +
+					boot_mem_map.map[i].size)))
+				return 1;
+		}
+	}
+	return 0;
+}
+#endif
-- 
1.6.2.1

Re: [PATCH -v1] MIPS: fix pfn_valid() for FLATMEM

[Rafael J. Wysocki]

On Thursday 08 October 2009, Wu Zhangjin wrote:
> When CONFIG_FLATMEM enabled, STD/Hiberation will fail on YeeLoong
> laptop, This patch fixes it:
> 
> if pfn is between min_low_pfn and max_mapnr, the old pfn_valid() will
> return TRUE, but in reality, if the memory is not continuous, it should
> be false. for example:
> 
> $ cat /proc/iomem | grep "System RAM"
> 00000000-0fffffff : System RAM
> 90000000-bfffffff : System RAM
> 
> as we can see, it is not continuous, so, some of the memory is not valid
> but regarded as valid by pfn_valid(), and at last make STD/Hibernate
> fail when shrinking a too large number of invalid memory.
> 
> Here, we fix it via checking pfn is in the "System RAM" or not. and
> Seems pfn_valid() is not called in assembly code, we move it to
> "!__ASSEMBLY__" to ensure we can simply declare it via "extern int
> pfn_valid(unsigned long)" without Compiling Error.
> 
> (This -v1 version incorporates feedback from Pavel Machek <pavel@ucw.cz>
>  and Sergei Shtylyov <sshtylyov@ru.mvista.com>)

Hmm.  What exactly would be wrong with using register_nosave_region() or
register_nosave_region_late() like x86 does?

Thanks,
Rafael

Re: [PATCH -v1] MIPS: fix pfn_valid() for FLATMEM

[Ralf Baechle]

On Thu, Oct 08, 2009 at 10:21:12PM +0200, Rafael J. Wysocki wrote:

> > Here, we fix it via checking pfn is in the "System RAM" or not. and
> > Seems pfn_valid() is not called in assembly code, we move it to
> > "!__ASSEMBLY__" to ensure we can simply declare it via "extern int
> > pfn_valid(unsigned long)" without Compiling Error.
> > 
> > (This -v1 version incorporates feedback from Pavel Machek <pavel@ucw.cz>
> >  and Sergei Shtylyov <sshtylyov@ru.mvista.com>)
> 
> Hmm.  What exactly would be wrong with using register_nosave_region() or
> register_nosave_region_late() like x86 does?

That seems to be more the fix than pfn_valid / PageReserved fiddlery we were
discussing in the other thread.  Thanks!

  Ralf

Re: [PATCH -v1] MIPS: fix pfn_valid() for FLATMEM

[Wu Zhangjin]

On Thu, 2009-10-08 at 22:44 +0200, Ralf Baechle wrote:
> On Thu, Oct 08, 2009 at 10:21:12PM +0200, Rafael J. Wysocki wrote:
> 
> > > Here, we fix it via checking pfn is in the "System RAM" or not. and
> > > Seems pfn_valid() is not called in assembly code, we move it to
> > > "!__ASSEMBLY__" to ensure we can simply declare it via "extern int
> > > pfn_valid(unsigned long)" without Compiling Error.
> > > 
> > > (This -v1 version incorporates feedback from Pavel Machek <pavel@ucw.cz>
> > >  and Sergei Shtylyov <sshtylyov@ru.mvista.com>)
> > 
> > Hmm.  What exactly would be wrong with using register_nosave_region() or
> > register_nosave_region_late() like x86 does?

Have tried to use register_nosave_region(), it works, but seems there is
something else wrong there.

> 
> That seems to be more the fix than pfn_valid / PageReserved fiddlery we were
> discussing in the other thread.  Thanks!

Just checked the arch/mips/loongson/common/mem.c, Seems it did not
register any reserved pages, in reality, two sections of memory are
reserved.

here should be the patch, tested it, works.

diff --git a/arch/mips/loongson/common/mem.c
b/arch/mips/loongson/common/mem.c
index 7c92f79..069d20b 100644
--- a/arch/mips/loongson/common/mem.c
+++ b/arch/mips/loongson/common/mem.c
@@ -15,11 +15,17 @@
 
 void __init prom_init_memory(void)
 {
-    add_memory_region(0x0, (memsize << 20), BOOT_MEM_RAM);
+    add_memory_region(0x0, memsize << 20, BOOT_MEM_RAM);
+
+    add_memory_region(memsize << 20, LOONGSON_PCI_MEM_START - (memsize
<<
+                           20), BOOT_MEM_RESERVED);
 #ifdef CONFIG_64BIT
     if (highmemsize > 0)
        add_memory_region(LOONGSON_HIGHMEM_START,
                highmemsize << 20, BOOT_MEM_RAM);
+
+    add_memory_region(LOONGSON_PCI_MEM_END + 1, LOONGSON_HIGHMEM_START
-
+                   LOONGSON_PCI_MEM_END - 1, BOOT_MEM_RESERVED);
 #endif /* CONFIG_64BIT */
 }

here is latest result:

$ cat /proc/iomem
00000000-0fffffff : System RAM
  00200000-0049ba17 : Kernel code
  0049ba18-005415ff : Kernel data
10000000-3fffffff : reserved ---> reserved page
40000000-7fffffff : pci memory space
  40000000-40ffffff : 0000:00:08.0
  41000000-4101ffff : 0000:00:07.0
  41020000-41020fff : 0000:00:09.0
    41020000-41020fff : ohci_hcd
  41021000-41021fff : 0000:00:0e.4
    41021000-41021fff : ohci_hcd
  41022000-41022fff : 0000:00:0e.5
    41022000-41022fff : ehci_hcd
  41023000-410230ff : 0000:00:07.0
    41023000-410230ff : 8139too
  41023100-410231ff : 0000:00:09.1
    41023100-410231ff : ehci_hcd
80000000-8fffffff : reserved  --> reserved page
90000000-bfffffff : System RAM

and what about pfn_valid(), is there a need to make it "robust"? or the
above patch is enough?

if the above patch is okay, I will send it to you later.

Regards,
	Wu Zhangjin

Re: [PATCH -v1] MIPS: fix pfn_valid() for FLATMEM

[Wu Zhangjin]

On Fri, 2009-10-09 at 10:08 +0800, Wu Zhangjin wrote:
> On Thu, 2009-10-08 at 22:44 +0200, Ralf Baechle wrote:
> > On Thu, Oct 08, 2009 at 10:21:12PM +0200, Rafael J. Wysocki wrote:
> > 
> > > > Here, we fix it via checking pfn is in the "System RAM" or not. and
> > > > Seems pfn_valid() is not called in assembly code, we move it to
> > > > "!__ASSEMBLY__" to ensure we can simply declare it via "extern int
> > > > pfn_valid(unsigned long)" without Compiling Error.
> > > > 
> > > > (This -v1 version incorporates feedback from Pavel Machek <pavel@ucw.cz>
> > > >  and Sergei Shtylyov <sshtylyov@ru.mvista.com>)
> > > 
> > > Hmm.  What exactly would be wrong with using register_nosave_region() or
> > > register_nosave_region_late() like x86 does?
> 
> Have tried to use register_nosave_region(), it works, but seems there is
> something else wrong there.
> 
> > 
> > That seems to be more the fix than pfn_valid / PageReserved fiddlery we were
> > discussing in the other thread.  Thanks!
> 
> Just checked the arch/mips/loongson/common/mem.c, Seems it did not
> register any reserved pages, in reality, two sections of memory are
> reserved.
> 
> here should be the patch, tested it, works.
> 
> diff --git a/arch/mips/loongson/common/mem.c
> b/arch/mips/loongson/common/mem.c
> index 7c92f79..069d20b 100644
> --- a/arch/mips/loongson/common/mem.c
> +++ b/arch/mips/loongson/common/mem.c
> @@ -15,11 +15,17 @@
>  
>  void __init prom_init_memory(void)
>  {
> -    add_memory_region(0x0, (memsize << 20), BOOT_MEM_RAM);
> +    add_memory_region(0x0, memsize << 20, BOOT_MEM_RAM);
> +
> +    add_memory_region(memsize << 20, LOONGSON_PCI_MEM_START - (memsize
> <<
> +                           20), BOOT_MEM_RESERVED);
>  #ifdef CONFIG_64BIT
>      if (highmemsize > 0)
>         add_memory_region(LOONGSON_HIGHMEM_START,
>                 highmemsize << 20, BOOT_MEM_RAM);
> +
> +    add_memory_region(LOONGSON_PCI_MEM_END + 1, LOONGSON_HIGHMEM_START
> -
> +                   LOONGSON_PCI_MEM_END - 1, BOOT_MEM_RESERVED);
>  #endif /* CONFIG_64BIT */
>  }

The above patch can not fix the problem when enabled FLATMEM in
linux-2.6.32-rc3, the real problem should be that we need register the
"pci memory space" as nosave pages, and also, the above "reserved"(not
memory) pages should be registered as nosave pages. but the simpler
solution should be the pfn_valid() I sent out in this E-mail thread, we
just need to check whether they are "valid", if they are "System
RAM"(BOOT_MEM_RAM or BOOT_MEM_ROM_DATA), they should be valid.

and what's more? should be register "pci memory space" as nosave pages
for all architecture?

Regards,
	Wu Zhangjin

Re: [PATCH -v1] MIPS: fix pfn_valid() for FLATMEM

[Ralf Baechle]

On Sat, Oct 10, 2009 at 12:02:10AM +0800, Wu Zhangjin wrote:

> The above patch can not fix the problem when enabled FLATMEM in
> linux-2.6.32-rc3, the real problem should be that we need register the
> "pci memory space" as nosave pages, and also, the above "reserved"(not
> memory) pages should be registered as nosave pages. but the simpler
> solution should be the pfn_valid() I sent out in this E-mail thread, we
> just need to check whether they are "valid", if they are "System
> RAM"(BOOT_MEM_RAM or BOOT_MEM_ROM_DATA), they should be valid.
> 
> and what's more? should be register "pci memory space" as nosave pages
> for all architecture?

No.  You only see this problem because your PCI memory space is between
the lowest and the highest memory address.  Other systems don't have this
issue because they either use the discontig or sparse memory models.

Btw, for systems that actually have memory in the 90000000-bfffffff range
and are running a 64-bit kernel with 4k ages the flatmem memory model
will waste 28MB of RAM; with 16k pages it's still 7MB.

Time to say gooebye to flatmem?

  Ralf

Re: [PATCH -v1] MIPS: fix pfn_valid() for FLATMEM

[Wu Zhangjin]

On Wed, 2009-10-14 at 00:04 +0200, Ralf Baechle wrote:
> On Sat, Oct 10, 2009 at 12:02:10AM +0800, Wu Zhangjin wrote:
> 
> > The above patch can not fix the problem when enabled FLATMEM in
> > linux-2.6.32-rc3, the real problem should be that we need register the
> > "pci memory space" as nosave pages, and also, the above "reserved"(not
> > memory) pages should be registered as nosave pages. but the simpler
> > solution should be the pfn_valid() I sent out in this E-mail thread, we
> > just need to check whether they are "valid", if they are "System
> > RAM"(BOOT_MEM_RAM or BOOT_MEM_ROM_DATA), they should be valid.
> > 
> > and what's more? should be register "pci memory space" as nosave pages
> > for all architecture?
> 
> No.  You only see this problem because your PCI memory space is between
> the lowest and the highest memory address.  Other systems don't have this
> issue because they either use the discontig or sparse memory models.
> 
> Btw, for systems that actually have memory in the 90000000-bfffffff range
> and are running a 64-bit kernel with 4k ages the flatmem memory model
> will waste 28MB of RAM; with 16k pages it's still 7MB.
> 
> Time to say gooebye to flatmem?

Okay, I will enable SPARSEMEM by default in the defconfig later.

Regards,
	Wu Zhangjin