[PATCH 0/8] More GFS2 folio conversions

[PATCH 0/8] More GFS2 folio conversions

[Matthew Wilcox (Oracle)]

I think this may be the last batch of patches to gfs2 for folio
conversions.  The only remaining references to struct page that I see are
for filesystem metadata that isn't stored in the page cache, so those are
fine to continue using struct page.  The only mild improvement would be if
we could have different bio completion handlers for gfs2_end_log_write()
when it's using mempool pages vs folio pages, but that may not even be
feasible and I like the current solution well enough.

This all seems fairly straightforward to me, but as usual only
compile-tested.  I don't anticipate the change to buffer_head.h to have
any conflicts; removing the last user of page_buffers() is not on the
cards for the next merge window.

Matthew Wilcox (Oracle) (8):
  gfs2: Use b_folio in gfs2_log_write_bh()
  gfs2: Use b_folio in gfs2_trans_add_meta()
  gfs2: Use b_folio in gfs2_submit_bhs()
  gfs2: Use b_folio in gfs2_check_magic()
  gfs2: Convert gfs2_jhead_pg_srch() to gfs2_jhead_folio_srch()
  gfs2: Convert gfs2_find_jhead() to use a folio
  gfs2: Convert gfs2_end_log_write_bh() to work on a folio
  gfs2: Convert gfs2_meta_read_endio() to use a folio

 fs/gfs2/lops.c              | 69 ++++++++++++++++++-------------------
 fs/gfs2/meta_io.c           | 15 ++++----
 fs/gfs2/trans.c             |  4 +--
 include/linux/buffer_head.h |  1 -
 4 files changed, 43 insertions(+), 46 deletions(-)

-- 
2.47.2

[PATCH 1/8] gfs2: Use b_folio in gfs2_log_write_bh()

[Matthew Wilcox (Oracle)]

We are preparing to remove bh->b_page.  gfs2_log_write() should continue
to operate on pages as some of the memory being logged does not come
from folios, so convert from folio to page in this function.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/gfs2/lops.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 314ec2a70167..d27f34688ff5 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -359,8 +359,8 @@ static void gfs2_log_write_bh(struct gfs2_sbd *sdp, struct buffer_head *bh)
 
 	dblock = gfs2_log_bmap(sdp->sd_jdesc, sdp->sd_log_flush_head);
 	gfs2_log_incr_head(sdp);
-	gfs2_log_write(sdp, sdp->sd_jdesc, bh->b_page, bh->b_size,
-		       bh_offset(bh), dblock);
+	gfs2_log_write(sdp, sdp->sd_jdesc, folio_page(bh->b_folio, 0),
+			bh->b_size, bh_offset(bh), dblock);
 }
 
 /**
-- 
2.47.2

[PATCH 2/8] gfs2: Use b_folio in gfs2_trans_add_meta()

[Matthew Wilcox (Oracle)]

The lock bit is maintained on the folio, not on the page.  Saves two
calls to compound_head() as well as removing two references to
bh->b_page.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/gfs2/trans.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/gfs2/trans.c b/fs/gfs2/trans.c
index 192213c7359a..f8ae2c666fd6 100644
--- a/fs/gfs2/trans.c
+++ b/fs/gfs2/trans.c
@@ -246,12 +246,12 @@ void gfs2_trans_add_meta(struct gfs2_glock *gl, struct buffer_head *bh)
 	if (bd == NULL) {
 		gfs2_log_unlock(sdp);
 		unlock_buffer(bh);
-		lock_page(bh->b_page);
+		folio_lock(bh->b_folio);
 		if (bh->b_private == NULL)
 			bd = gfs2_alloc_bufdata(gl, bh);
 		else
 			bd = bh->b_private;
-		unlock_page(bh->b_page);
+		folio_unlock(bh->b_folio);
 		lock_buffer(bh);
 		gfs2_log_lock(sdp);
 	}
-- 
2.47.2

[PATCH 3/8] gfs2: Use b_folio in gfs2_submit_bhs()

[Matthew Wilcox (Oracle)]

Remove a reference to bh->b_page which is going to be removed soon.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/gfs2/meta_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
index fea3efcc2f93..66db506a5f7f 100644
--- a/fs/gfs2/meta_io.c
+++ b/fs/gfs2/meta_io.c
@@ -232,7 +232,7 @@ static void gfs2_submit_bhs(blk_opf_t opf, struct buffer_head *bhs[], int num)
 		bio->bi_iter.bi_sector = bh->b_blocknr * (bh->b_size >> 9);
 		while (num > 0) {
 			bh = *bhs;
-			if (!bio_add_page(bio, bh->b_page, bh->b_size, bh_offset(bh))) {
+			if (!bio_add_folio(bio, bh->b_folio, bh->b_size, bh_offset(bh))) {
 				BUG_ON(bio->bi_iter.bi_size == 0);
 				break;
 			}
-- 
2.47.2

[PATCH 4/8] gfs2: Use b_folio in gfs2_check_magic()

[Matthew Wilcox (Oracle)]

We are preparing to remove bh->b_page.  Use kmap_local_folio() instead
of kmap_local_page().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/gfs2/lops.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index d27f34688ff5..4123bfc16680 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -615,15 +615,13 @@ static struct page *gfs2_get_log_desc(struct gfs2_sbd *sdp, u32 ld_type,
 
 static void gfs2_check_magic(struct buffer_head *bh)
 {
-	void *kaddr;
 	__be32 *ptr;
 
 	clear_buffer_escaped(bh);
-	kaddr = kmap_local_page(bh->b_page);
-	ptr = kaddr + bh_offset(bh);
+	ptr = kmap_local_folio(bh->b_folio, bh_offset(bh));
 	if (*ptr == cpu_to_be32(GFS2_MAGIC))
 		set_buffer_escaped(bh);
-	kunmap_local(kaddr);
+	kunmap_local(ptr);
 }
 
 static int blocknr_cmp(void *priv, const struct list_head *a,
-- 
2.47.2

[PATCH 5/8] gfs2: Convert gfs2_jhead_pg_srch() to gfs2_jhead_folio_srch()

[Matthew Wilcox (Oracle)]

Pass in the folio instead of the page.  Add an assert that this is
not a large folio as we'd need a more complex solution if we wanted to
kmap() each page out of a large folio.  Removes a use of folio->page.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/gfs2/lops.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 4123bfc16680..09e4c5915243 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -406,17 +406,16 @@ static void gfs2_end_log_read(struct bio *bio)
 }
 
 /**
- * gfs2_jhead_pg_srch - Look for the journal head in a given page.
+ * gfs2_jhead_folio_srch - Look for the journal head in a given page.
  * @jd: The journal descriptor
  * @head: The journal head to start from
- * @page: The page to look in
+ * @folio: The folio to look in
  *
  * Returns: 1 if found, 0 otherwise.
  */
-
-static bool gfs2_jhead_pg_srch(struct gfs2_jdesc *jd,
+static bool gfs2_jhead_folio_srch(struct gfs2_jdesc *jd,
 			      struct gfs2_log_header_host *head,
-			      struct page *page)
+			      struct folio *folio)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
 	struct gfs2_log_header_host lh;
@@ -424,7 +423,8 @@ static bool gfs2_jhead_pg_srch(struct gfs2_jdesc *jd,
 	unsigned int offset;
 	bool ret = false;
 
-	kaddr = kmap_local_page(page);
+	VM_BUG_ON_FOLIO(folio_test_large(folio), folio);
+	kaddr = kmap_local_folio(folio, 0);
 	for (offset = 0; offset < PAGE_SIZE; offset += sdp->sd_sb.sb_bsize) {
 		if (!__get_log_header(sdp, kaddr + offset, 0, &lh)) {
 			if (lh.lh_sequence >= head->lh_sequence)
@@ -472,7 +472,7 @@ static void gfs2_jhead_process_page(struct gfs2_jdesc *jd, unsigned long index,
 		*done = true;
 
 	if (!*done)
-		*done = gfs2_jhead_pg_srch(jd, head, &folio->page);
+		*done = gfs2_jhead_folio_srch(jd, head, folio);
 
 	/* filemap_get_folio() and the earlier grab_cache_page() */
 	folio_put_refs(folio, 2);
-- 
2.47.2

[PATCH 6/8] gfs2: Convert gfs2_find_jhead() to use a folio

[Matthew Wilcox (Oracle)]

Remove a call to grab_cache_page() by using a folio throughout
this function.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/gfs2/lops.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 09e4c5915243..30597b0f7cc3 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -514,7 +514,7 @@ int gfs2_find_jhead(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head,
 	struct gfs2_journal_extent *je;
 	int sz, ret = 0;
 	struct bio *bio = NULL;
-	struct page *page = NULL;
+	struct folio *folio = NULL;
 	bool done = false;
 	errseq_t since;
 
@@ -527,9 +527,10 @@ int gfs2_find_jhead(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head,
 		u64 dblock = je->dblock;
 
 		for (; block < je->lblock + je->blocks; block++, dblock++) {
-			if (!page) {
-				page = grab_cache_page(mapping, block >> shift);
-				if (!page) {
+			if (!folio) {
+				folio = filemap_grab_folio(mapping,
+						block >> shift);
+				if (!folio) {
 					ret = -ENOMEM;
 					done = true;
 					goto out;
@@ -541,7 +542,7 @@ int gfs2_find_jhead(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head,
 				sector_t sector = dblock << sdp->sd_fsb2bb_shift;
 
 				if (bio_end_sector(bio) == sector) {
-					sz = bio_add_page(bio, page, bsize, off);
+					sz = bio_add_folio(bio, folio, bsize, off);
 					if (sz == bsize)
 						goto block_added;
 				}
@@ -562,12 +563,12 @@ int gfs2_find_jhead(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head,
 			bio = gfs2_log_alloc_bio(sdp, dblock, gfs2_end_log_read);
 			bio->bi_opf = REQ_OP_READ;
 add_block_to_new_bio:
-			sz = bio_add_page(bio, page, bsize, off);
+			sz = bio_add_folio(bio, folio, bsize, off);
 			BUG_ON(sz != bsize);
 block_added:
 			off += bsize;
-			if (off == PAGE_SIZE)
-				page = NULL;
+			if (off == folio_size(folio))
+				folio = NULL;
 			if (blocks_submitted <= blocks_read + max_blocks) {
 				/* Keep at least one bio in flight */
 				continue;
-- 
2.47.2

Re: [PATCH 6/8] gfs2: Convert gfs2_find_jhead() to use a folio

[Andreas Gruenbacher]

On Mon, Feb 10, 2025 at 2:35 PM Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
> Remove a call to grab_cache_page() by using a folio throughout
> this function.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/gfs2/lops.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
> index 09e4c5915243..30597b0f7cc3 100644
> --- a/fs/gfs2/lops.c
> +++ b/fs/gfs2/lops.c
> @@ -514,7 +514,7 @@ int gfs2_find_jhead(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head,
>         struct gfs2_journal_extent *je;
>         int sz, ret = 0;
>         struct bio *bio = NULL;
> -       struct page *page = NULL;
> +       struct folio *folio = NULL;
>         bool done = false;
>         errseq_t since;
>
> @@ -527,9 +527,10 @@ int gfs2_find_jhead(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head,
>                 u64 dblock = je->dblock;
>
>                 for (; block < je->lblock + je->blocks; block++, dblock++) {
> -                       if (!page) {
> -                               page = grab_cache_page(mapping, block >> shift);
> -                               if (!page) {
> +                       if (!folio) {
> +                               folio = filemap_grab_folio(mapping,
> +                                               block >> shift);
> +                               if (!folio) {
>                                         ret = -ENOMEM;
>                                         done = true;
>                                         goto out;
> @@ -541,7 +542,7 @@ int gfs2_find_jhead(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head,
>                                 sector_t sector = dblock << sdp->sd_fsb2bb_shift;
>
>                                 if (bio_end_sector(bio) == sector) {
> -                                       sz = bio_add_page(bio, page, bsize, off);
> +                                       sz = bio_add_folio(bio, folio, bsize, off);

The check below needs adjusting: bio_add_folio() returns whether the
addition was successful, not how many bytes were added.

I'll fix that.

>                                         if (sz == bsize)
>                                                 goto block_added;
>                                 }
> @@ -562,12 +563,12 @@ int gfs2_find_jhead(struct gfs2_jdesc *jd, struct gfs2_log_header_host *head,
>                         bio = gfs2_log_alloc_bio(sdp, dblock, gfs2_end_log_read);
>                         bio->bi_opf = REQ_OP_READ;
>  add_block_to_new_bio:
> -                       sz = bio_add_page(bio, page, bsize, off);
> +                       sz = bio_add_folio(bio, folio, bsize, off);

Likewise here.

>                         BUG_ON(sz != bsize);
>  block_added:
>                         off += bsize;
> -                       if (off == PAGE_SIZE)
> -                               page = NULL;
> +                       if (off == folio_size(folio))
> +                               folio = NULL;
>                         if (blocks_submitted <= blocks_read + max_blocks) {
>                                 /* Keep at least one bio in flight */
>                                 continue;
> --
> 2.47.2
>

Thanks,
Andreas

[PATCH 7/8] gfs2: Convert gfs2_end_log_write_bh() to work on a folio

[Matthew Wilcox (Oracle)]

gfs2_end_log_write() has to handle bios which consist of both pages
which belong to folios and pages which were allocated from a mempool and
do not belong to a folio.  It would be cleaner to have separate endio
handlers which handle each type, but it's not clear to me whether that's
even possible.

This patch is slightly forward-looking in that page_folio() cannot
currently return NULL, but it will return NULL in the future for pages
which do not belong to a folio.

This was the last user of page_has_buffers(), so remove it.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/gfs2/lops.c              | 28 ++++++++++++++--------------
 include/linux/buffer_head.h |  1 -
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 30597b0f7cc3..8b46bd01a448 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -157,7 +157,9 @@ u64 gfs2_log_bmap(struct gfs2_jdesc *jd, unsigned int lblock)
 /**
  * gfs2_end_log_write_bh - end log write of pagecache data with buffers
  * @sdp: The superblock
- * @bvec: The bio_vec
+ * @folio: The folio
+ * @offset: The first byte within the folio that completed
+ * @size: The number of bytes that completed
  * @error: The i/o status
  *
  * This finds the relevant buffers and unlocks them and sets the
@@ -166,17 +168,13 @@ u64 gfs2_log_bmap(struct gfs2_jdesc *jd, unsigned int lblock)
  * that is pinned in the pagecache.
  */
 
-static void gfs2_end_log_write_bh(struct gfs2_sbd *sdp,
-				  struct bio_vec *bvec,
-				  blk_status_t error)
+static void gfs2_end_log_write_bh(struct gfs2_sbd *sdp, struct folio *folio,
+		size_t offset, size_t size, blk_status_t error)
 {
 	struct buffer_head *bh, *next;
-	struct page *page = bvec->bv_page;
-	unsigned size;
 
-	bh = page_buffers(page);
-	size = bvec->bv_len;
-	while (bh_offset(bh) < bvec->bv_offset)
+	bh = folio_buffers(folio);
+	while (bh_offset(bh) < offset)
 		bh = bh->b_this_page;
 	do {
 		if (error)
@@ -186,7 +184,7 @@ static void gfs2_end_log_write_bh(struct gfs2_sbd *sdp,
 		size -= bh->b_size;
 		brelse(bh);
 		bh = next;
-	} while(bh && size);
+	} while (bh && size);
 }
 
 /**
@@ -203,7 +201,6 @@ static void gfs2_end_log_write(struct bio *bio)
 {
 	struct gfs2_sbd *sdp = bio->bi_private;
 	struct bio_vec *bvec;
-	struct page *page;
 	struct bvec_iter_all iter_all;
 
 	if (bio->bi_status) {
@@ -217,9 +214,12 @@ static void gfs2_end_log_write(struct bio *bio)
 	}
 
 	bio_for_each_segment_all(bvec, bio, iter_all) {
-		page = bvec->bv_page;
-		if (page_has_buffers(page))
-			gfs2_end_log_write_bh(sdp, bvec, bio->bi_status);
+		struct page *page = bvec->bv_page;
+		struct folio *folio = page_folio(page);
+
+		if (folio && folio_buffers(folio))
+			gfs2_end_log_write_bh(sdp, folio, bvec->bv_offset,
+					bvec->bv_len, bio->bi_status);
 		else
 			mempool_free(page, gfs2_page_pool);
 	}
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 932139c5d46f..fab70b26e131 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -182,7 +182,6 @@ static inline unsigned long bh_offset(const struct buffer_head *bh)
 		BUG_ON(!PagePrivate(page));			\
 		((struct buffer_head *)page_private(page));	\
 	})
-#define page_has_buffers(page)	PagePrivate(page)
 #define folio_buffers(folio)		folio_get_private(folio)
 
 void buffer_check_dirty_writeback(struct folio *folio,
-- 
2.47.2

Re: [PATCH 7/8] gfs2: Convert gfs2_end_log_write_bh() to work on a folio

[Andreas Gruenbacher]

On Mon, Feb 10, 2025 at 2:35 PM Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
> gfs2_end_log_write() has to handle bios which consist of both pages
> which belong to folios and pages which were allocated from a mempool and
> do not belong to a folio.  It would be cleaner to have separate endio
> handlers which handle each type, but it's not clear to me whether that's
> even possible.
>
> This patch is slightly forward-looking in that page_folio() cannot
> currently return NULL, but it will return NULL in the future for pages
> which do not belong to a folio.
>
> This was the last user of page_has_buffers(), so remove it.

Right now in for-next, ocfs2 is still using page_has_buffers(), so I'm
going to skip this part.

Thanks,
Andreas


> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/gfs2/lops.c              | 28 ++++++++++++++--------------
>  include/linux/buffer_head.h |  1 -
>  2 files changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
> index 30597b0f7cc3..8b46bd01a448 100644
> --- a/fs/gfs2/lops.c
> +++ b/fs/gfs2/lops.c
> @@ -157,7 +157,9 @@ u64 gfs2_log_bmap(struct gfs2_jdesc *jd, unsigned int lblock)
>  /**
>   * gfs2_end_log_write_bh - end log write of pagecache data with buffers
>   * @sdp: The superblock
> - * @bvec: The bio_vec
> + * @folio: The folio
> + * @offset: The first byte within the folio that completed
> + * @size: The number of bytes that completed
>   * @error: The i/o status
>   *
>   * This finds the relevant buffers and unlocks them and sets the
> @@ -166,17 +168,13 @@ u64 gfs2_log_bmap(struct gfs2_jdesc *jd, unsigned int lblock)
>   * that is pinned in the pagecache.
>   */
>
> -static void gfs2_end_log_write_bh(struct gfs2_sbd *sdp,
> -                                 struct bio_vec *bvec,
> -                                 blk_status_t error)
> +static void gfs2_end_log_write_bh(struct gfs2_sbd *sdp, struct folio *folio,
> +               size_t offset, size_t size, blk_status_t error)
>  {
>         struct buffer_head *bh, *next;
> -       struct page *page = bvec->bv_page;
> -       unsigned size;
>
> -       bh = page_buffers(page);
> -       size = bvec->bv_len;
> -       while (bh_offset(bh) < bvec->bv_offset)
> +       bh = folio_buffers(folio);
> +       while (bh_offset(bh) < offset)
>                 bh = bh->b_this_page;
>         do {
>                 if (error)
> @@ -186,7 +184,7 @@ static void gfs2_end_log_write_bh(struct gfs2_sbd *sdp,
>                 size -= bh->b_size;
>                 brelse(bh);
>                 bh = next;
> -       } while(bh && size);
> +       } while (bh && size);
>  }
>
>  /**
> @@ -203,7 +201,6 @@ static void gfs2_end_log_write(struct bio *bio)
>  {
>         struct gfs2_sbd *sdp = bio->bi_private;
>         struct bio_vec *bvec;
> -       struct page *page;
>         struct bvec_iter_all iter_all;
>
>         if (bio->bi_status) {
> @@ -217,9 +214,12 @@ static void gfs2_end_log_write(struct bio *bio)
>         }
>
>         bio_for_each_segment_all(bvec, bio, iter_all) {
> -               page = bvec->bv_page;
> -               if (page_has_buffers(page))
> -                       gfs2_end_log_write_bh(sdp, bvec, bio->bi_status);
> +               struct page *page = bvec->bv_page;
> +               struct folio *folio = page_folio(page);
> +
> +               if (folio && folio_buffers(folio))
> +                       gfs2_end_log_write_bh(sdp, folio, bvec->bv_offset,
> +                                       bvec->bv_len, bio->bi_status);
>                 else
>                         mempool_free(page, gfs2_page_pool);
>         }
> diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
> index 932139c5d46f..fab70b26e131 100644
> --- a/include/linux/buffer_head.h
> +++ b/include/linux/buffer_head.h
> @@ -182,7 +182,6 @@ static inline unsigned long bh_offset(const struct buffer_head *bh)
>                 BUG_ON(!PagePrivate(page));                     \
>                 ((struct buffer_head *)page_private(page));     \
>         })
> -#define page_has_buffers(page) PagePrivate(page)
>  #define folio_buffers(folio)           folio_get_private(folio)
>
>  void buffer_check_dirty_writeback(struct folio *folio,
> --
> 2.47.2
>

Re: [PATCH 7/8] gfs2: Convert gfs2_end_log_write_bh() to work on a folio

[Matthew Wilcox]

On Sun, Mar 09, 2025 at 06:33:34PM +0100, Andreas Gruenbacher wrote:
> On Mon, Feb 10, 2025 at 2:35 PM Matthew Wilcox (Oracle)
> <willy@infradead.org> wrote:
> > gfs2_end_log_write() has to handle bios which consist of both pages
> > which belong to folios and pages which were allocated from a mempool and
> > do not belong to a folio.  It would be cleaner to have separate endio
> > handlers which handle each type, but it's not clear to me whether that's
> > even possible.
> >
> > This patch is slightly forward-looking in that page_folio() cannot
> > currently return NULL, but it will return NULL in the future for pages
> > which do not belong to a folio.
> >
> > This was the last user of page_has_buffers(), so remove it.
> 
> Right now in for-next, ocfs2 is still using page_has_buffers(), so I'm
> going to skip this part.

How odd.  I see it removed in 1b426db11ba8 ecee61651d8f 0fad0a824e5c
414ae0a44033 and all of those commits are in 6.14-rc1.

$ git show v6.14-rc1:fs/ocfs2/aops.c |grep page_has
(no output)

Re: [PATCH 7/8] gfs2: Convert gfs2_end_log_write_bh() to work on a folio

[Andreas Gruenbacher]

On Sun, Mar 9, 2025 at 9:57 PM Matthew Wilcox <willy@infradead.org> wrote:
> On Sun, Mar 09, 2025 at 06:33:34PM +0100, Andreas Gruenbacher wrote:
> > On Mon, Feb 10, 2025 at 2:35 PM Matthew Wilcox (Oracle)
> > <willy@infradead.org> wrote:
> > > gfs2_end_log_write() has to handle bios which consist of both pages
> > > which belong to folios and pages which were allocated from a mempool and
> > > do not belong to a folio.  It would be cleaner to have separate endio
> > > handlers which handle each type, but it's not clear to me whether that's
> > > even possible.
> > >
> > > This patch is slightly forward-looking in that page_folio() cannot
> > > currently return NULL, but it will return NULL in the future for pages
> > > which do not belong to a folio.
> > >
> > > This was the last user of page_has_buffers(), so remove it.
> >
> > Right now in for-next, ocfs2 is still using page_has_buffers(), so I'm
> > going to skip this part.
>
> How odd.  I see it removed in 1b426db11ba8 ecee61651d8f 0fad0a824e5c
> 414ae0a44033 and all of those commits are in 6.14-rc1.
>
> $ git show v6.14-rc1:fs/ocfs2/aops.c |grep page_has
> (no output)

Hmm, you're right, it's only that automatic test that's based on an
older kernel. Sorry for the confusion.

Andreas

Re: [PATCH 7/8] gfs2: Convert gfs2_end_log_write_bh() to work on a folio

[Matthew Wilcox]

On Sun, Mar 09, 2025 at 10:53:29PM +0100, Andreas Gruenbacher wrote:
> On Sun, Mar 9, 2025 at 9:57 PM Matthew Wilcox <willy@infradead.org> wrote:
> > On Sun, Mar 09, 2025 at 06:33:34PM +0100, Andreas Gruenbacher wrote:
> > > On Mon, Feb 10, 2025 at 2:35 PM Matthew Wilcox (Oracle)
> > > <willy@infradead.org> wrote:
> > > > gfs2_end_log_write() has to handle bios which consist of both pages
> > > > which belong to folios and pages which were allocated from a mempool and
> > > > do not belong to a folio.  It would be cleaner to have separate endio
> > > > handlers which handle each type, but it's not clear to me whether that's
> > > > even possible.
> > > >
> > > > This patch is slightly forward-looking in that page_folio() cannot
> > > > currently return NULL, but it will return NULL in the future for pages
> > > > which do not belong to a folio.
> > > >
> > > > This was the last user of page_has_buffers(), so remove it.
> > >
> > > Right now in for-next, ocfs2 is still using page_has_buffers(), so I'm
> > > going to skip this part.
> >
> > How odd.  I see it removed in 1b426db11ba8 ecee61651d8f 0fad0a824e5c
> > 414ae0a44033 and all of those commits are in 6.14-rc1.
> >
> > $ git show v6.14-rc1:fs/ocfs2/aops.c |grep page_has
> > (no output)
> 
> Hmm, you're right, it's only that automatic test that's based on an
> older kernel. Sorry for the confusion.

Looks like your for-next doesn't include v6.14-rc1.

gfs2            104b4d597ff21b923b1e963c5793efcadeae047e

is the entry in SHA1s for next-20250307.  And:

$ git log v6.14-rc1 ^104b4d597ff21b923b1e963c5793efcadeae047e
shows quite a lot of commits (9847 of them).  So I think you didn't pull
from Linus before branching for the v6.15 merge window.  Not sure how
you manage your trees and how you'd like to improce this situation
(do you rebase?  Do you want to bring in a merge commit of some -rc
version?  If so, which one?)

Re: [PATCH 7/8] gfs2: Convert gfs2_end_log_write_bh() to work on a folio

[Andreas Gruenbacher]

On Mon, Mar 10, 2025 at 4:46 AM Matthew Wilcox <willy@infradead.org> wrote:
> On Sun, Mar 09, 2025 at 10:53:29PM +0100, Andreas Gruenbacher wrote:
> > On Sun, Mar 9, 2025 at 9:57 PM Matthew Wilcox <willy@infradead.org> wrote:
> > > On Sun, Mar 09, 2025 at 06:33:34PM +0100, Andreas Gruenbacher wrote:
> > > > On Mon, Feb 10, 2025 at 2:35 PM Matthew Wilcox (Oracle)
> > > > <willy@infradead.org> wrote:
> > > > > gfs2_end_log_write() has to handle bios which consist of both pages
> > > > > which belong to folios and pages which were allocated from a mempool and
> > > > > do not belong to a folio.  It would be cleaner to have separate endio
> > > > > handlers which handle each type, but it's not clear to me whether that's
> > > > > even possible.
> > > > >
> > > > > This patch is slightly forward-looking in that page_folio() cannot
> > > > > currently return NULL, but it will return NULL in the future for pages
> > > > > which do not belong to a folio.
> > > > >
> > > > > This was the last user of page_has_buffers(), so remove it.
> > > >
> > > > Right now in for-next, ocfs2 is still using page_has_buffers(), so I'm
> > > > going to skip this part.
> > >
> > > How odd.  I see it removed in 1b426db11ba8 ecee61651d8f 0fad0a824e5c
> > > 414ae0a44033 and all of those commits are in 6.14-rc1.
> > >
> > > $ git show v6.14-rc1:fs/ocfs2/aops.c |grep page_has
> > > (no output)
> >
> > Hmm, you're right, it's only that automatic test that's based on an
> > older kernel. Sorry for the confusion.
>
> Looks like your for-next doesn't include v6.14-rc1.
>
> gfs2            104b4d597ff21b923b1e963c5793efcadeae047e
>
> is the entry in SHA1s for next-20250307.  And:
>
> $ git log v6.14-rc1 ^104b4d597ff21b923b1e963c5793efcadeae047e
> shows quite a lot of commits (9847 of them).  So I think you didn't pull
> from Linus before branching for the v6.15 merge window.

Right, this is the point at which gfs2-for-6.14 was merged into mainline.

>  Not sure how
> you manage your trees and how you'd like to improve this situation
> (do you rebase?  Do you want to bring in a merge commit of some -rc
> version?  If so, which one?)

I have rebased onto v6.14-rc1 now, so things should be fine.

Thanks,
Andreas

[PATCH 8/8] gfs2: Convert gfs2_meta_read_endio() to use a folio

[Matthew Wilcox (Oracle)]

Switch from bio_for_each_segment_all() to bio_for_each_folio_all()
which removes a call to page_buffers().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/gfs2/meta_io.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
index 66db506a5f7f..198cc7056637 100644
--- a/fs/gfs2/meta_io.c
+++ b/fs/gfs2/meta_io.c
@@ -198,15 +198,14 @@ struct buffer_head *gfs2_meta_new(struct gfs2_glock *gl, u64 blkno)
 
 static void gfs2_meta_read_endio(struct bio *bio)
 {
-	struct bio_vec *bvec;
-	struct bvec_iter_all iter_all;
+	struct folio_iter fi;
 
-	bio_for_each_segment_all(bvec, bio, iter_all) {
-		struct page *page = bvec->bv_page;
-		struct buffer_head *bh = page_buffers(page);
-		unsigned int len = bvec->bv_len;
+	bio_for_each_folio_all(fi, bio) {
+		struct folio *folio = fi.folio;
+		struct buffer_head *bh = folio_buffers(folio);
+		size_t len = fi.length;
 
-		while (bh_offset(bh) < bvec->bv_offset)
+		while (bh_offset(bh) < fi.offset)
 			bh = bh->b_this_page;
 		do {
 			struct buffer_head *next = bh->b_this_page;
-- 
2.47.2

Re: [PATCH 0/8] More GFS2 folio conversions

[Matthew Wilcox]

On Mon, Feb 10, 2025 at 01:34:38PM +0000, Matthew Wilcox (Oracle) wrote:
> I think this may be the last batch of patches to gfs2 for folio
> conversions.  The only remaining references to struct page that I see are
> for filesystem metadata that isn't stored in the page cache, so those are
> fine to continue using struct page.  The only mild improvement would be if
> we could have different bio completion handlers for gfs2_end_log_write()
> when it's using mempool pages vs folio pages, but that may not even be
> feasible and I like the current solution well enough.
> 
> This all seems fairly straightforward to me, but as usual only
> compile-tested.  I don't anticipate the change to buffer_head.h to have
> any conflicts; removing the last user of page_buffers() is not on the
> cards for the next merge window.

ping

Re: [PATCH 0/8] More GFS2 folio conversions

[Andreas Gruenbacher]

Willy,

On Thu, Feb 20, 2025 at 2:51 PM Matthew Wilcox <willy@infradead.org> wrote:
> On Mon, Feb 10, 2025 at 01:34:38PM +0000, Matthew Wilcox (Oracle) wrote:
> > I think this may be the last batch of patches to gfs2 for folio
> > conversions.  The only remaining references to struct page that I see are
> > for filesystem metadata that isn't stored in the page cache, so those are
> > fine to continue using struct page.  The only mild improvement would be if
> > we could have different bio completion handlers for gfs2_end_log_write()
> > when it's using mempool pages vs folio pages, but that may not even be
> > feasible and I like the current solution well enough.
> >
> > This all seems fairly straightforward to me, but as usual only
> > compile-tested.  I don't anticipate the change to buffer_head.h to have
> > any conflicts; removing the last user of page_buffers() is not on the
> > cards for the next merge window.
>
> ping

thanks a lot for these patches; I'm adding them to for-next.

Andreas