xip: fix get_zeroed_page with __GFP_HIGHMEM
Subsystems (2)
PATCH 1/1 xip: fix get_zeroed_page with __GFP_HIGHMEM
The use of get_zeroed_page() with __GFP_HIGHMEM is invalid. Use alloc_page() with __GFP_ZERO instead of invalid get_zeroed_page().
(This patch is only compile tested)
--- mm/filemap_xip.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
mm/filemap_xip.c +5 -4
@@ -25,14 +25,15 @@ static struct page *__xip_sparse_page;static struct page *xip_sparse_page (void ) {if (!__xip_sparse_page) { -unsigned long zeroes =get_zeroed_page (GFP_HIGHUSER ); -if (zeroes) { +struct page *page =alloc_page (GFP_HIGHUSER | __GFP_ZERO); + +if (page) {static DEFINE_SPINLOCK (xip_alloc_lock);spin_lock (&xip_alloc_lock);if (!__xip_sparse_page) - __xip_sparse_page =virt_to_page (zeroes); + __xip_sparse_page = page;else -free_page (zeroes); +__free_page (page);spin_unlock (&xip_alloc_lock); } }
Quoted 7 lines · Akinobu Mita
The use of
get_zeroed_page()with__GFP_HIGHMEMis invalid. Usealloc_page()with__GFP_ZEROinstead of invalidget_zeroed_page().(This patch is only compile tested)
Cc Carsten Otte <[email protected]>Signed-off-by Akinobu Mita <[email protected]>
Good find! You got me very worried, how this escaped testing before. Presumed explanation: it hasn't been needed beyond s390, which has no CONFIG_HIGHMEM; and it has never been tested with CONFIG_DEBUG_VM on.
But I haven't tested it either: let's wait for Carsten to report. I believe Nick has changes on the way which will make it possible for ordinary mortals to test XIP: here's a good argument to bring them on.
May I cross-reference my "prep_zero_page: remove bogus BUG_ON" 09f345da758fca1222b0971b65b2fddbdf78bb83 in 2.6.24-rc: that bogus (actually VM_)BUG_ON would have stood in the way too, so there's no point in backporting this without that. But if only non-HIGHMEM architectures can have been using XIP, a backport is not essential.
Hugh
p.s. Nick's ZERO_PAGE changes, in 2.6.24-rc, actually cancel the need for a special xip_sparse_page distinct from ZERO_PAGE. But let's not become dependent on those: keep this doing it the way it does now.
Quoted 29 lines
--- mm/filemap_xip.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)Index:
2.6-git/mm/filemap_xip.c
===================================================================mm/filemap_xip.c +5
-4@@ -25,14 +25,15 @@ static struct page *__xip_sparse_page;static struct page *xip_sparse_page (void ) {if (!__xip_sparse_page) { -unsigned long zeroes =get_zeroed_page (GFP_HIGHUSER ); -if (zeroes) { +struct page *page =alloc_page (GFP_HIGHUSER | __GFP_ZERO); + +if (page) {static DEFINE_SPINLOCK (xip_alloc_lock);spin_lock (&xip_alloc_lock);if (!__xip_sparse_page) - __xip_sparse_page =virt_to_page (zeroes); + __xip_sparse_page = page;else -free_page (zeroes); +__free_page (page);spin_unlock (&xip_alloc_lock); } }
Quoted 2 lines · Akinobu Mita
The use of
get_zeroed_page()with__GFP_HIGHMEMis invalid. Usealloc_page()with__GFP_ZEROinstead of invalidget_zeroed_page().
*Ouch*