Re: [PATCH v2 13/13] mm/gup: move private gup FOLL_ flags to internal.h

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Jan 26, 2023 at 04:39:02PM +0100, Claudio Imbrenda wrote:

> I can tell you that the original goal of that function is to make sure
> that there are no extra references. in particular, we want to prevent
> I/O of any kind to be ongoing while the page becomes secure. (the I/O
> will fail and, depending on which device it was, the whole system might
> end up in a rather unhappy state)

Sure, but if there is concurrent IO you just try again right? It
doesn't wait for refs to drop for instance.

So make the lock_page work the same way:

diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
index 9f18a4af9c1319..847ee50b8672c6 100644
--- a/arch/s390/kernel/uv.c
+++ b/arch/s390/kernel/uv.c
@@ -193,20 +193,11 @@ static int expected_page_refs(struct page *page)
 }
 
 static int make_secure_pte(pte_t *ptep, unsigned long addr,
-			   struct page *exp_page, struct uv_cb_header *uvcb)
+			   struct page *page, struct uv_cb_header *uvcb)
 {
 	pte_t entry = READ_ONCE(*ptep);
-	struct page *page;
 	int expected, cc = 0;
 
-	if (!pte_present(entry))
-		return -ENXIO;
-	if (pte_val(entry) & _PAGE_INVALID)
-		return -ENXIO;
-
-	page = pte_page(entry);
-	if (page != exp_page)
-		return -ENXIO;
 	if (PageWriteback(page))
 		return -EAGAIN;
 	expected = expected_page_refs(page);
@@ -304,17 +295,25 @@ int gmap_make_secure(struct gmap *gmap, unsigned long gaddr, void *uvcb)
 		goto out;
 
 	rc = -ENXIO;
-	page = follow_page(vma, uaddr, FOLL_WRITE);
-	if (IS_ERR_OR_NULL(page))
-		goto out;
-
-	lock_page(page);
 	ptep = get_locked_pte(gmap->mm, uaddr, &ptelock);
+
+	if (!pte_present(entry))
+		goto out_unlock_pte;
+	if (pte_val(entry) & _PAGE_INVALID)
+		goto out_unlock_pte;
+	page = pte_page(entry);
+
+	if (!trylock_page(page)) {
+		rc = -EAGAIN;
+		goto out_unlock_pte;
+	}
+
 	if (should_export_before_import(uvcb, gmap->mm))
 		uv_convert_from_secure(page_to_phys(page));
 	rc = make_secure_pte(ptep, uaddr, page, uvcb);
-	pte_unmap_unlock(ptep, ptelock);
 	unlock_page(page);
+out_unlock_pte:
+	pte_unmap_unlock(ptep, ptelock);
 out:
 	mmap_read_unlock(gmap->mm);