diff mbox series

mm/madvise: elevate page refcount while isolating in process_madvise()

Message ID 1639571019-30043-1-git-send-email-quic_charante@quicinc.com (mailing list archive)
State New
Headers show
Series mm/madvise: elevate page refcount while isolating in process_madvise() | expand

Commit Message

Charan Teja Kalla Dec. 15, 2021, 12:23 p.m. UTC
The documentation of isolate_lru_page() says that, "it must be called
with an elevated refcount on the page", which is not followed while
isolating pages in process_madvise() system call with advise
MADV_PAGEOUT. Fix it.

Signed-off-by: Charan Teja Reddy <quic_charante@quicinc.com>
---
 mm/madvise.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Matthew Wilcox Dec. 15, 2021, 1:55 p.m. UTC | #1
On Wed, Dec 15, 2021 at 05:53:39PM +0530, Charan Teja Reddy wrote:
> The documentation of isolate_lru_page() says that, "it must be called
> with an elevated refcount on the page", which is not followed while
> isolating pages in process_madvise() system call with advise
> MADV_PAGEOUT. Fix it.

We hold the mmap_lock over the call to this function, so the reference
to the page from the page tables cannot go away.  There's no need to
grab an extra reference here.
Charan Teja Kalla Dec. 15, 2021, 3:39 p.m. UTC | #2
On 12/15/2021 7:25 PM, Matthew Wilcox wrote:
> On Wed, Dec 15, 2021 at 05:53:39PM +0530, Charan Teja Reddy wrote:
>> The documentation of isolate_lru_page() says that, "it must be called
>> with an elevated refcount on the page", which is not followed while
>> isolating pages in process_madvise() system call with advise
>> MADV_PAGEOUT. Fix it.
> 
> We hold the mmap_lock over the call to this function, so the reference
> to the page from the page tables cannot go away.  There's no need to
> grab an extra reference here.

Thanks Matthew for clearing such fundamentals.

>
diff mbox series

Patch

diff --git a/mm/madvise.c b/mm/madvise.c
index 0734db8..4c4a8e9 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -317,6 +317,7 @@  static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
 	pte_t *orig_pte, *pte, ptent;
 	spinlock_t *ptl;
 	struct page *page = NULL;
+	int ret;
 	LIST_HEAD(page_list);
 
 	if (fatal_signal_pending(current))
@@ -373,12 +374,15 @@  static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
 		ClearPageReferenced(page);
 		test_and_clear_page_young(page);
 		if (pageout) {
-			if (!isolate_lru_page(page)) {
+			get_page(page);
+			ret = isolate_lru_page(page);
+			if (!ret) {
 				if (PageUnevictable(page))
 					putback_lru_page(page);
 				else
 					list_add(&page->lru, &page_list);
 			}
+			put_page(page);
 		} else
 			deactivate_page(page);
 huge_unlock:
@@ -459,12 +463,15 @@  static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
 		ClearPageReferenced(page);
 		test_and_clear_page_young(page);
 		if (pageout) {
-			if (!isolate_lru_page(page)) {
+			get_page(page);
+			ret = isolate_lru_page(page);
+			if (!ret) {
 				if (PageUnevictable(page))
 					putback_lru_page(page);
 				else
 					list_add(&page->lru, &page_list);
 			}
+			put_page(page);
 		} else
 			deactivate_page(page);
 	}