Skip to contentSkip to conversation

Patch "memfd: fix F_SEAL_WRITE after shmem huge page allocated" has been added to the 5.4-stable tree

DISCUSSION Hugh Dickins 2 participants 3 messages Mar 6, 2022 1 message not in the archive

Subsystems (3)
lore
Share & tools
lore

Hugh Dickins

author maintainer
#1 lore raw
In reply to a message not in the archive
Quoted 11 lines · On Sat, 5 Mar 2022, [email protected] wrote:

This is a note to let you know that I've just added the patch titled

memfd: fix F_SEAL_WRITE after shmem huge page allocated

to the 5.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
memfd-fix-f_seal_write-after-shmem-huge-page-allocated.patch
and it can be found in the queue-5.4 subdirectory.

Thank you for adding that patch to 5.16, 5.15, 5.10 and 5.4:
please accept the substitute patch below for 4.19 - thanks. A different patch will follow for 4.14 and 4.9.

From f2b277c4d1c63a85127e8aa2588e9cc3bd21cb99 Mon Sep 17 00:00:00 2001
From: Hugh Dickins <[email protected]>
Date: Fri, 4 Mar 2022 20:29:01 -0800
Subject: memfd: fix F_SEAL_WRITE after shmem huge page allocated

From: Hugh Dickins <[email protected]>

commit f2b277c4d1c63a85127e8aa2588e9cc3bd21cb99 upstream.

Wangyong reports: after enabling tmpfs filesystem to support transparent hugepage with the following command:

echo always > /sys/kernel/mm/transparent_hugepage/shmem_enabled

the docker program tries to add F_SEAL_WRITE through the following command, but it fails unexpectedly with errno EBUSY:

fcntl(5, F_ADD_SEALS, F_SEAL_WRITE) = -1.

That is because memfd_tag_pins() and memfd_wait_for_pins() were never updated for shmem huge pages: checking page_mapcount() against page_count() is hopeless on THP subpages - they need to check total_mapcount() against page_count() on THP heads only.

Make memfd_tag_pins() (compared > 1) as strict as memfd_wait_for_pins() (compared != 1): either can be justified, but given the non-atomic total_mapcount() calculation, it is better now to be strict. Bear in mind that total_mapcount() itself scans all of the THP subpages, when choosing to take an XA_CHECK_SCHED latency break.

Also fix the unlikely xa_is_value() case in memfd_wait_for_pins(): if a page has been swapped out since memfd_tag_pins(), then its refcount must have fallen, and so it can safely be untagged.

---
 mm/memfd.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)
mm/memfd.c +22 -8
@@ -34,26 +34,35 @@ static void memfd_tag_pins(struct address_space *mapping)
 	void __rcu **slot;
 	pgoff_t start;
 	struct page *page;
-	unsigned int tagged = 0;
+	int latency = 0;
+	int cache_count;
 
 	lru_add_drain();
 	start = 0;
 
 	xa_lock_irq(&mapping->i_pages);
 	radix_tree_for_each_slot(slot, &mapping->i_pages, &iter, start) {
+		cache_count = 1;
 		page = radix_tree_deref_slot_protected(slot, &mapping->i_pages.xa_lock);
-		if (!page || radix_tree_exception(page)) {
+		if (!page || radix_tree_exception(page) || PageTail(page)) {
 			if (radix_tree_deref_retry(page)) {
 				slot = radix_tree_iter_retry(&iter);
 				continue;
 			}
-		} else if (page_count(page) - page_mapcount(page) > 1) {
-			radix_tree_tag_set(&mapping->i_pages, iter.index,
-					   MEMFD_TAG_PINNED);
+		} else {
+			if (PageTransHuge(page) && !PageHuge(page))
+				cache_count = HPAGE_PMD_NR;
+			if (cache_count !=
+			    page_count(page) - total_mapcount(page)) {
+				radix_tree_tag_set(&mapping->i_pages,
+						iter.index, MEMFD_TAG_PINNED);
+			}
 		}
 
-		if (++tagged % 1024)
+		latency += cache_count;
+		if (latency < 1024)
 			continue;
+		latency = 0;
 
 		slot = radix_tree_iter_resume(slot, &iter);
 		xa_unlock_irq(&mapping->i_pages);
@@ -79,6 +88,7 @@ static int memfd_wait_for_pins(struct address_space *mapping)
 	pgoff_t start;
 	struct page *page;
 	int error, scan;
+	int cache_count;
 
 	memfd_tag_pins(mapping);
 
@@ -107,8 +117,12 @@ static int memfd_wait_for_pins(struct address_space *mapping)
 				page = NULL;
 			}
 
-			if (page &&
-			    page_count(page) - page_mapcount(page) != 1) {
+			cache_count = 1;
+			if (page && PageTransHuge(page) && !PageHuge(page))
+				cache_count = HPAGE_PMD_NR;
+
+			if (page && cache_count !=
+			    page_count(page) - total_mapcount(page)) {
 				if (scan < LAST_SCAN)
 					goto continue_resched;
 
author maintainer
#2 lore raw
In reply to a message not in the archive
Quoted 11 lines · On Sat, 5 Mar 2022, [email protected] wrote:

This is a note to let you know that I've just added the patch titled

memfd: fix F_SEAL_WRITE after shmem huge page allocated

to the 5.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
memfd-fix-f_seal_write-after-shmem-huge-page-allocated.patch
and it can be found in the queue-5.4 subdirectory.

Thank you for adding that patch to 5.16, 5.15, 5.10 and 5.4:
please accept the substitute patch below for 4.14 and 4.9 - thanks. A different patch for 4.19 has been sent separately.

From f2b277c4d1c63a85127e8aa2588e9cc3bd21cb99 Mon Sep 17 00:00:00 2001
From: Hugh Dickins <[email protected]>
Date: Fri, 4 Mar 2022 20:29:01 -0800
Subject: memfd: fix F_SEAL_WRITE after shmem huge page allocated

From: Hugh Dickins <[email protected]>

commit f2b277c4d1c63a85127e8aa2588e9cc3bd21cb99 upstream.

Wangyong reports: after enabling tmpfs filesystem to support transparent hugepage with the following command:

echo always > /sys/kernel/mm/transparent_hugepage/shmem_enabled

the docker program tries to add F_SEAL_WRITE through the following command, but it fails unexpectedly with errno EBUSY:

fcntl(5, F_ADD_SEALS, F_SEAL_WRITE) = -1.

That is because memfd_tag_pins() and memfd_wait_for_pins() were never updated for shmem huge pages: checking page_mapcount() against page_count() is hopeless on THP subpages - they need to check total_mapcount() against page_count() on THP heads only.

Make memfd_tag_pins() (compared > 1) as strict as memfd_wait_for_pins() (compared != 1): either can be justified, but given the non-atomic total_mapcount() calculation, it is better now to be strict. Bear in mind that total_mapcount() itself scans all of the THP subpages, when choosing to take an XA_CHECK_SCHED latency break.

Also fix the unlikely xa_is_value() case in memfd_wait_for_pins(): if a page has been swapped out since memfd_tag_pins(), then its refcount must have fallen, and so it can safely be untagged.

---
 mm/shmem.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
mm/shmem.c +4 -3
@@ -2689,7 +2689,8 @@ static void shmem_tag_pins(struct address_space *mapping)
 				slot = radix_tree_iter_retry(&iter);
 				continue;
 			}
-		} else if (page_count(page) - page_mapcount(page) > 1) {
+		} else if (!PageTail(page) && page_count(page) !=
+			   hpage_nr_pages(page) + total_mapcount(page)) {
 			radix_tree_tag_set(&mapping->page_tree, iter.index,
 					   SHMEM_TAG_PINNED);
 		}
@@ -2749,8 +2750,8 @@ static int shmem_wait_for_pins(struct address_space *mapping)
 				page = NULL;
 			}
 
-			if (page &&
-			    page_count(page) - page_mapcount(page) != 1) {
+			if (page && page_count(page) !=
+			    hpage_nr_pages(page) + total_mapcount(page)) {
 				if (scan < LAST_SCAN)
 					goto continue_resched;
 
maintainer
#3 lore raw
Quoted 9 lines · Hugh Dickins
Quoted 8 lines · On Sat, 5 Mar 2022, [email protected] wrote:

This is a note to let you know that I've just added the patch titled

memfd: fix F_SEAL_WRITE after shmem huge page allocated

to the 5.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

Quoted 7 lines
Quoted 3 lines

The filename of the patch is:
memfd-fix-f_seal_write-after-shmem-huge-page-allocated.patch
and it can be found in the queue-5.4 subdirectory.

Thank you for adding that patch to 5.16, 5.15, 5.10 and 5.4:
please accept the substitute patch below for 4.14 and 4.9 - thanks. A different patch for 4.19 has been sent separately.

All now queued up, thanks for the backports!

greg k-h