2017-11-27 11:36:16

by JianKang Chen

Subject: [PATCH resend] mm/page_alloc: fix comment is __get_free_pages

From: Jiankang Chen <[email protected]>

__get_free_pages will return an virtual address,
but it is not just 32-bit address, for example a 64-bit system.
And this comment really confuse new bigenner of mm.

reported-by: Hanjun Guo <[email protected]>
Signed-off-by: Jiankang Chen <[email protected]>
---
mm/page_alloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 77e4d3c..5a7c432 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4240,7 +4240,7 @@ unsigned long __get_free_pages(gfp_t gfp_mask, unsigned int order)
struct page *page;

/*
- * __get_free_pages() returns a 32-bit address, which cannot represent
+ * __get_free_pages() returns a virtual address, which cannot represent
* a highmem page
*/
VM_BUG_ON((gfp_mask & __GFP_HIGHMEM) != 0);
--
1.7.12.4


From 1585218883645974346@xxx Mon Nov 27 11:35:35 +0000 2017
X-GM-THRID: 1585019167239466893
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-27 11:35:35

by Michal Hocko

Subject: Re: [PATCH resend] mm/page_alloc: fix comment is __get_free_pages

On Mon 27-11-17 19:09:24, JianKang Chen wrote:
> From: Jiankang Chen <[email protected]>
>
> __get_free_pages will return an virtual address,
> but it is not just 32-bit address, for example a 64-bit system.
> And this comment really confuse new bigenner of mm.

s@bigenner@beginner@

Anyway, do we really need a bug on for this? Has this actually caught
any wrong usage? VM_BUG_ON tends to be enabled these days AFAIK and
panicking the kernel seems like an over-reaction. If there is a real
risk then why don't we simply mask __GFP_HIGHMEM off when calling
alloc_pages?

> reported-by: Hanjun Guo <[email protected]>
> Signed-off-by: Jiankang Chen <[email protected]>
> ---
> mm/page_alloc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 77e4d3c..5a7c432 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4240,7 +4240,7 @@ unsigned long __get_free_pages(gfp_t gfp_mask, unsigned int order)
> struct page *page;
>
> /*
> - * __get_free_pages() returns a 32-bit address, which cannot represent
> + * __get_free_pages() returns a virtual address, which cannot represent
> * a highmem page
> */
> VM_BUG_ON((gfp_mask & __GFP_HIGHMEM) != 0);
> --
> 1.7.12.4
>

--
Michal Hocko
SUSE Labs

From 1585199318397707758@xxx Mon Nov 27 06:24:36 +0000 2017
X-GM-THRID: 1585019167239466893
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-27 06:24:35

by Anshuman Khandual

Subject: Re: [PATCH resend] mm/page_alloc: fix comment is __get_free_pages

On 11/25/2017 12:50 PM, JianKang Chen wrote:
> From: Jiankang Chen <[email protected]>
>
> __get_free_pages will return an 64bit address in 64bit System
> like arm64 or x86_64. And this comment really confuse new bigenner of
> mm.

Normally its not 64 bit virtual address though CPU architecture supports
64 bits. But yes, specifying it as general virtual address without number
of bits is better.


From 1585022330389114456@xxx Sat Nov 25 07:31:27 +0000 2017
X-GM-THRID: 1585019167239466893
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread