[PATCH 0/2] Convert some gmap functions to use folios

[PATCH 0/2] Convert some gmap functions to use folios

[Matthew Wilcox (Oracle)]

struct page is going to lose its refcount (someday) and as part of
that page_ref_freeze() will go away.  s390's ultravisor is one of
the few remaining places that uses it, so convert it over to folios.
From previous interactions, I understand that ultravisor doesn't support
large folios, so this simply declines to make large folios secure.
I think you'd be better off splitting the folio if it is large, but
that's something I'd rather leave to someone who can test it.

These patches do have the effect of making this more efficient; we lose
at least five hidden calls to compound_head().

Matthew Wilcox (Oracle) (2):
  s390: Convert make_page_secure to use a folio
  s390: Convert gmap_make_secure to use a folio

 arch/s390/kernel/uv.c | 50 +++++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 23 deletions(-)

-- 
2.43.0

[PATCH 1/2] s390: Convert make_page_secure to use a folio

[Matthew Wilcox (Oracle)]

These page APIs are deprecated, so convert the incoming page to a folio
and use the folio APIs instead.  The ultravisor API cannot handle large
folios, so return -EINVAL if one has slipped through.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 arch/s390/kernel/uv.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
index fc07bc39e698..0634448698b0 100644
--- a/arch/s390/kernel/uv.c
+++ b/arch/s390/kernel/uv.c
@@ -181,21 +181,21 @@ int uv_convert_owned_from_secure(unsigned long paddr)
 }
 
 /*
- * Calculate the expected ref_count for a page that would otherwise have no
+ * Calculate the expected ref_count for a folio that would otherwise have no
  * further pins. This was cribbed from similar functions in other places in
  * the kernel, but with some slight modifications. We know that a secure
- * page can not be a huge page for example.
+ * folio can not be a large folio, for example.
  */
-static int expected_page_refs(struct page *page)
+static int expected_folio_refs(struct folio *folio)
 {
 	int res;
 
-	res = page_mapcount(page);
-	if (PageSwapCache(page)) {
+	res = folio_mapcount(folio);
+	if (folio_test_swapcache(folio)) {
 		res++;
-	} else if (page_mapping(page)) {
+	} else if (folio_mapping(folio)) {
 		res++;
-		if (page_has_private(page))
+		if (folio->private)
 			res++;
 	}
 	return res;
@@ -203,14 +203,17 @@ static int expected_page_refs(struct page *page)
 
 static int make_page_secure(struct page *page, struct uv_cb_header *uvcb)
 {
+	struct folio *folio = page_folio(page);
 	int expected, cc = 0;
 
-	if (PageWriteback(page))
+	if (folio_test_large(folio))
+		return -EINVAL;
+	if (folio_test_writeback(folio))
 		return -EAGAIN;
-	expected = expected_page_refs(page);
-	if (!page_ref_freeze(page, expected))
+	expected = expected_folio_refs(folio);
+	if (!folio_ref_freeze(folio, expected))
 		return -EBUSY;
-	set_bit(PG_arch_1, &page->flags);
+	set_bit(PG_arch_1, &folio->flags);
 	/*
 	 * If the UVC does not succeed or fail immediately, we don't want to
 	 * loop for long, or we might get stall notifications.
@@ -220,9 +223,9 @@ static int make_page_secure(struct page *page, struct uv_cb_header *uvcb)
 	 * -EAGAIN and we let the callers deal with it.
 	 */
 	cc = __uv_call(0, (u64)uvcb);
-	page_ref_unfreeze(page, expected);
+	folio_ref_unfreeze(folio, expected);
 	/*
-	 * Return -ENXIO if the page was not mapped, -EINVAL for other errors.
+	 * Return -ENXIO if the folio was not mapped, -EINVAL for other errors.
 	 * If busy or partially completed, return -EAGAIN.
 	 */
 	if (cc == UVC_CC_OK)
-- 
2.43.0

[PATCH 2/2] s390: Convert gmap_make_secure to use a folio

[Matthew Wilcox (Oracle)]

Remove uses of deprecated page APIs, and move the check for large
folios to here to avoid taking the folio lock if the folio is too large.
We could do better here by attempting to split the large folio, but I'll
leave that improvement for someone who can test it.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 arch/s390/kernel/uv.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
index 0634448698b0..2aed5af7c4e5 100644
--- a/arch/s390/kernel/uv.c
+++ b/arch/s390/kernel/uv.c
@@ -201,13 +201,10 @@ static int expected_folio_refs(struct folio *folio)
 	return res;
 }
 
-static int make_page_secure(struct page *page, struct uv_cb_header *uvcb)
+static int make_folio_secure(struct folio *folio, struct uv_cb_header *uvcb)
 {
-	struct folio *folio = page_folio(page);
 	int expected, cc = 0;
 
-	if (folio_test_large(folio))
-		return -EINVAL;
 	if (folio_test_writeback(folio))
 		return -EAGAIN;
 	expected = expected_folio_refs(folio);
@@ -280,7 +277,7 @@ int gmap_make_secure(struct gmap *gmap, unsigned long gaddr, void *uvcb)
 	bool local_drain = false;
 	spinlock_t *ptelock;
 	unsigned long uaddr;
-	struct page *page;
+	struct folio *folio;
 	pte_t *ptep;
 	int rc;
 
@@ -309,15 +306,19 @@ int gmap_make_secure(struct gmap *gmap, unsigned long gaddr, void *uvcb)
 	if (!ptep)
 		goto out;
 	if (pte_present(*ptep) && !(pte_val(*ptep) & _PAGE_INVALID) && pte_write(*ptep)) {
-		page = pte_page(*ptep);
+		folio = page_folio(pte_page(*ptep));
+		rc = -EINVAL;
+		if (folio_test_large(folio))
+			goto unlock;
 		rc = -EAGAIN;
-		if (trylock_page(page)) {
+		if (folio_trylock(folio)) {
 			if (should_export_before_import(uvcb, gmap->mm))
-				uv_convert_from_secure(page_to_phys(page));
-			rc = make_page_secure(page, uvcb);
-			unlock_page(page);
+				uv_convert_from_secure(PFN_PHYS(folio_pfn(folio)));
+			rc = make_folio_secure(folio, uvcb);
+			folio_unlock(folio);
 		}
 	}
+unlock:
 	pte_unmap_unlock(ptep, ptelock);
 out:
 	mmap_read_unlock(gmap->mm);
@@ -327,10 +328,10 @@ int gmap_make_secure(struct gmap *gmap, unsigned long gaddr, void *uvcb)
 		 * If we are here because the UVC returned busy or partial
 		 * completion, this is just a useless check, but it is safe.
 		 */
-		wait_on_page_writeback(page);
+		folio_wait_writeback(folio);
 	} else if (rc == -EBUSY) {
 		/*
-		 * If we have tried a local drain and the page refcount
+		 * If we have tried a local drain and the folio refcount
 		 * still does not match our expected safe value, try with a
 		 * system wide drain. This is needed if the pagevecs holding
 		 * the page are on a different CPU.
@@ -341,7 +342,7 @@ int gmap_make_secure(struct gmap *gmap, unsigned long gaddr, void *uvcb)
 			return -EAGAIN;
 		}
 		/*
-		 * We are here if the page refcount does not match the
+		 * We are here if the folio refcount does not match the
 		 * expected safe value. The main culprits are usually
 		 * pagevecs. With lru_add_drain() we drain the pagevecs
 		 * on the local CPU so that hopefully the refcount will
-- 
2.43.0

Re: [PATCH 0/2] Convert some gmap functions to use folios

[Heiko Carstens]

On Fri, Mar 22, 2024 at 04:11:45PM +0000, Matthew Wilcox (Oracle) wrote:
> struct page is going to lose its refcount (someday) and as part of
> that page_ref_freeze() will go away.  s390's ultravisor is one of
> the few remaining places that uses it, so convert it over to folios.
> From previous interactions, I understand that ultravisor doesn't support
> large folios, so this simply declines to make large folios secure.
> I think you'd be better off splitting the folio if it is large, but
> that's something I'd rather leave to someone who can test it.
> 
> These patches do have the effect of making this more efficient; we lose
> at least five hidden calls to compound_head().
> 
> Matthew Wilcox (Oracle) (2):
>   s390: Convert make_page_secure to use a folio
>   s390: Convert gmap_make_secure to use a folio
> 
>  arch/s390/kernel/uv.c | 50 +++++++++++++++++++++++--------------------
>  1 file changed, 27 insertions(+), 23 deletions(-)

Christian, Janosch, or Claudio, could you have a look at this please;
and provide an ACK if this is ok with you?

Re: [PATCH 0/2] Convert some gmap functions to use folios

[Matthew Wilcox]

On Tue, Mar 26, 2024 at 08:41:27AM +0100, Heiko Carstens wrote:
> On Fri, Mar 22, 2024 at 04:11:45PM +0000, Matthew Wilcox (Oracle) wrote:
> > struct page is going to lose its refcount (someday) and as part of
> > that page_ref_freeze() will go away.  s390's ultravisor is one of
> > the few remaining places that uses it, so convert it over to folios.
> > From previous interactions, I understand that ultravisor doesn't support
> > large folios, so this simply declines to make large folios secure.
> > I think you'd be better off splitting the folio if it is large, but
> > that's something I'd rather leave to someone who can test it.
> 
> Christian, Janosch, or Claudio, could you have a look at this please;
> and provide an ACK if this is ok with you?

Ping?

Re: [PATCH 0/2] Convert some gmap functions to use folios

[Claudio Imbrenda]

On Thu, 4 Apr 2024 14:38:55 +0100
Matthew Wilcox <willy@infradead.org> wrote:

> On Tue, Mar 26, 2024 at 08:41:27AM +0100, Heiko Carstens wrote:
> > On Fri, Mar 22, 2024 at 04:11:45PM +0000, Matthew Wilcox (Oracle) wrote:  
> > > struct page is going to lose its refcount (someday) and as part of
> > > that page_ref_freeze() will go away.  s390's ultravisor is one of
> > > the few remaining places that uses it, so convert it over to folios.
> > > From previous interactions, I understand that ultravisor doesn't support
> > > large folios, so this simply declines to make large folios secure.
> > > I think you'd be better off splitting the folio if it is large, but
> > > that's something I'd rather leave to someone who can test it.  
> > 
> > Christian, Janosch, or Claudio, could you have a look at this please;
> > and provide an ACK if this is ok with you?  
> 
> Ping?

hi,

we've been looking into it and running some tests to see if anything
catches fire. I thought someone had replied to tell you, but apparently
it should have been me, and I forgot :)


you can tack this on:

Acked-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Re: [PATCH 0/2] Convert some gmap functions to use folios

[Alexander Gordeev]

On Fri, Mar 22, 2024 at 04:11:45PM +0000, Matthew Wilcox (Oracle) wrote:
...
> Matthew Wilcox (Oracle) (2):
>   s390: Convert make_page_secure to use a folio
>   s390: Convert gmap_make_secure to use a folio
> 
>  arch/s390/kernel/uv.c | 50 +++++++++++++++++++++++--------------------
>  1 file changed, 27 insertions(+), 23 deletions(-)

Applied, thanks!