[PATCH 0/4] convert create_page_buffers to folio_create_buffers

[PATCH 0/4] convert create_page_buffers to folio_create_buffers

[Pankaj Raghav]

One of the first kernel panic we hit when we try to increase the
block size > 4k is inside create_page_buffers()[1]. Even though buffer.c
function do not support large folios (folios > PAGE_SIZE) at the moment,
these changes are required when we want to remove that constraint.

Willy had already mentioned that he wanted to convert create_page_buffers to
folio_create_buffers but didn't get to it yet, so I decided to take a
shot.

No functional changes introduced.

Changes since RFC[2]:
- Renaming the helpers with folio_*
- Calling folio_* helper version inside the *page* helper.

[1] https://lore.kernel.org/all/ZBnGc4WbBOlnRUgd@casper.infradead.org/
[2]https://lore.kernel.org/lkml/20230414110821.21548-1-p.raghav@samsung.com/

Pankaj Raghav (4):
  fs/buffer: add folio_set_bh helper
  buffer: add folio_alloc_buffers() helper
  fs/buffer: add folio_create_empty_buffers helper
  fs/buffer: convert create_page_buffers to folio_create_buffers

 fs/buffer.c                 | 89 +++++++++++++++++++++++++------------
 include/linux/buffer_head.h |  6 +++
 2 files changed, 66 insertions(+), 29 deletions(-)

-- 
2.34.1

[PATCH 1/4] fs/buffer: add folio_set_bh helper

[Pankaj Raghav]

The folio version of set_bh_page(). This is required to convert
create_page_buffers() to folio_create_buffers() later in the series.

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 fs/buffer.c                 | 15 +++++++++++++++
 include/linux/buffer_head.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/fs/buffer.c b/fs/buffer.c
index b3eb905f87d6..7e74bd77a81b 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1484,6 +1484,21 @@ void set_bh_page(struct buffer_head *bh,
 }
 EXPORT_SYMBOL(set_bh_page);
 
+void folio_set_bh(struct buffer_head *bh, struct folio *folio,
+		  unsigned long offset)
+{
+	bh->b_folio = folio;
+	BUG_ON(offset >= folio_size(folio));
+	if (folio_test_highmem(folio))
+		/*
+		 * This catches illegal uses and preserves the offset:
+		 */
+		bh->b_data = (char *)(0 + offset);
+	else
+		bh->b_data = folio_address(folio) + offset;
+}
+EXPORT_SYMBOL(folio_set_bh);
+
 /*
  * Called when truncating a buffer on a page completely.
  */
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 8f14dca5fed7..7e92d23f4782 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -196,6 +196,8 @@ void mark_buffer_write_io_error(struct buffer_head *bh);
 void touch_buffer(struct buffer_head *bh);
 void set_bh_page(struct buffer_head *bh,
 		struct page *page, unsigned long offset);
+void folio_set_bh(struct buffer_head *bh, struct folio *folio,
+		  unsigned long offset);
 bool try_to_free_buffers(struct folio *);
 struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size,
 		bool retry);
-- 
2.34.1

Re: [PATCH 1/4] fs/buffer: add folio_set_bh helper

[Matthew Wilcox]

On Mon, Apr 17, 2023 at 02:36:15PM +0200, Pankaj Raghav wrote:
> The folio version of set_bh_page(). This is required to convert
> create_page_buffers() to folio_create_buffers() later in the series.
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>

Yep, not worth making this a wrapper for set_bh_page().

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Re: [PATCH 1/4] fs/buffer: add folio_set_bh helper

[Hannes Reinecke]

On 4/17/23 14:36, Pankaj Raghav wrote:
> The folio version of set_bh_page(). This is required to convert
> create_page_buffers() to folio_create_buffers() later in the series.
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>   fs/buffer.c                 | 15 +++++++++++++++
>   include/linux/buffer_head.h |  2 ++
>   2 files changed, 17 insertions(+)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes

[PATCH 2/4] buffer: add folio_alloc_buffers() helper

[Pankaj Raghav]

Folio version of alloc_page_buffers() helper. This is required to convert
create_page_buffers() to folio_create_buffers() later in the series.

alloc_page_buffers() has been modified to call folio_alloc_buffers()
which adds one call to compound_head() but folio_alloc_buffers() removes
one call to compound_head() compared to the existing alloc_page_buffers()
implementation.

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 fs/buffer.c                 | 23 +++++++++++++++--------
 include/linux/buffer_head.h |  2 ++
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 7e74bd77a81b..75415170e286 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -842,7 +842,7 @@ int remove_inode_buffers(struct inode *inode)
 }
 
 /*
- * Create the appropriate buffers when given a page for data area and
+ * Create the appropriate buffers when given a folio for data area and
  * the size of each buffer.. Use the bh->b_this_page linked list to
  * follow the buffers created.  Return NULL if unable to create more
  * buffers.
@@ -850,8 +850,8 @@ int remove_inode_buffers(struct inode *inode)
  * The retry flag is used to differentiate async IO (paging, swapping)
  * which may not fail from ordinary buffer allocations.
  */
-struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size,
-		bool retry)
+struct buffer_head *folio_alloc_buffers(struct folio *folio, unsigned long size,
+					bool retry)
 {
 	struct buffer_head *bh, *head;
 	gfp_t gfp = GFP_NOFS | __GFP_ACCOUNT;
@@ -861,12 +861,12 @@ struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size,
 	if (retry)
 		gfp |= __GFP_NOFAIL;
 
-	/* The page lock pins the memcg */
-	memcg = page_memcg(page);
+	/* The folio lock pins the memcg */
+	memcg = folio_memcg(folio);
 	old_memcg = set_active_memcg(memcg);
 
 	head = NULL;
-	offset = PAGE_SIZE;
+	offset = folio_size(folio);
 	while ((offset -= size) >= 0) {
 		bh = alloc_buffer_head(gfp);
 		if (!bh)
@@ -878,8 +878,8 @@ struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size,
 
 		bh->b_size = size;
 
-		/* Link the buffer to its page */
-		set_bh_page(bh, page, offset);
+		/* Link the buffer to its folio */
+		folio_set_bh(bh, folio, offset);
 	}
 out:
 	set_active_memcg(old_memcg);
@@ -898,6 +898,13 @@ struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size,
 
 	goto out;
 }
+EXPORT_SYMBOL_GPL(folio_alloc_buffers);
+
+struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size,
+				       bool retry)
+{
+	return folio_alloc_buffers(page_folio(page), size, retry);
+}
 EXPORT_SYMBOL_GPL(alloc_page_buffers);
 
 static inline void
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 7e92d23f4782..0b14eab41bd1 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -199,6 +199,8 @@ void set_bh_page(struct buffer_head *bh,
 void folio_set_bh(struct buffer_head *bh, struct folio *folio,
 		  unsigned long offset);
 bool try_to_free_buffers(struct folio *);
+struct buffer_head *folio_alloc_buffers(struct folio *folio, unsigned long size,
+					bool retry);
 struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size,
 		bool retry);
 void create_empty_buffers(struct page *, unsigned long,
-- 
2.34.1

Re: [PATCH 2/4] buffer: add folio_alloc_buffers() helper

[Matthew Wilcox]

On Mon, Apr 17, 2023 at 02:36:16PM +0200, Pankaj Raghav wrote:
> Folio version of alloc_page_buffers() helper. This is required to convert
> create_page_buffers() to folio_create_buffers() later in the series.
> 
> alloc_page_buffers() has been modified to call folio_alloc_buffers()
> which adds one call to compound_head() but folio_alloc_buffers() removes
> one call to compound_head() compared to the existing alloc_page_buffers()
> implementation.
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Re: [PATCH 2/4] buffer: add folio_alloc_buffers() helper

[Hannes Reinecke]

On 4/17/23 14:36, Pankaj Raghav wrote:
> Folio version of alloc_page_buffers() helper. This is required to convert
> create_page_buffers() to folio_create_buffers() later in the series.
> 
> alloc_page_buffers() has been modified to call folio_alloc_buffers()
> which adds one call to compound_head() but folio_alloc_buffers() removes
> one call to compound_head() compared to the existing alloc_page_buffers()
> implementation.
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>   fs/buffer.c                 | 23 +++++++++++++++--------
>   include/linux/buffer_head.h |  2 ++
>   2 files changed, 17 insertions(+), 8 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes

[PATCH 3/4] fs/buffer: add folio_create_empty_buffers helper

[Pankaj Raghav]

Folio version of create_empty_buffers(). This is required to convert
create_page_buffers() to folio_create_buffers() later in the series.

It removes several calls to compound_head() as it works directly on folio
compared to create_empty_buffers(). Hence, create_empty_buffers() has
been modified to call folio_create_empty_buffers().

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 fs/buffer.c                 | 28 +++++++++++++++++-----------
 include/linux/buffer_head.h |  2 ++
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 75415170e286..13724ef7eec7 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1593,18 +1593,17 @@ void block_invalidate_folio(struct folio *folio, size_t offset, size_t length)
 }
 EXPORT_SYMBOL(block_invalidate_folio);
 
-
 /*
  * We attach and possibly dirty the buffers atomically wrt
  * block_dirty_folio() via private_lock.  try_to_free_buffers
- * is already excluded via the page lock.
+ * is already excluded via the folio lock.
  */
-void create_empty_buffers(struct page *page,
-			unsigned long blocksize, unsigned long b_state)
+void folio_create_empty_buffers(struct folio *folio, unsigned long blocksize,
+				unsigned long b_state)
 {
 	struct buffer_head *bh, *head, *tail;
 
-	head = alloc_page_buffers(page, blocksize, true);
+	head = folio_alloc_buffers(folio, blocksize, true);
 	bh = head;
 	do {
 		bh->b_state |= b_state;
@@ -1613,19 +1612,26 @@ void create_empty_buffers(struct page *page,
 	} while (bh);
 	tail->b_this_page = head;
 
-	spin_lock(&page->mapping->private_lock);
-	if (PageUptodate(page) || PageDirty(page)) {
+	spin_lock(&folio->mapping->private_lock);
+	if (folio_test_uptodate(folio) || folio_test_dirty(folio)) {
 		bh = head;
 		do {
-			if (PageDirty(page))
+			if (folio_test_dirty(folio))
 				set_buffer_dirty(bh);
-			if (PageUptodate(page))
+			if (folio_test_uptodate(folio))
 				set_buffer_uptodate(bh);
 			bh = bh->b_this_page;
 		} while (bh != head);
 	}
-	attach_page_private(page, head);
-	spin_unlock(&page->mapping->private_lock);
+	folio_attach_private(folio, head);
+	spin_unlock(&folio->mapping->private_lock);
+}
+EXPORT_SYMBOL(folio_create_empty_buffers);
+
+void create_empty_buffers(struct page *page,
+			unsigned long blocksize, unsigned long b_state)
+{
+	folio_create_empty_buffers(page_folio(page), blocksize, b_state);
 }
 EXPORT_SYMBOL(create_empty_buffers);
 
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 0b14eab41bd1..1520793c72da 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -205,6 +205,8 @@ struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size,
 		bool retry);
 void create_empty_buffers(struct page *, unsigned long,
 			unsigned long b_state);
+void folio_create_empty_buffers(struct folio *folio, unsigned long blocksize,
+				unsigned long b_state);
 void end_buffer_read_sync(struct buffer_head *bh, int uptodate);
 void end_buffer_write_sync(struct buffer_head *bh, int uptodate);
 void end_buffer_async_write(struct buffer_head *bh, int uptodate);
-- 
2.34.1

Re: [PATCH 3/4] fs/buffer: add folio_create_empty_buffers helper

[Matthew Wilcox]

On Mon, Apr 17, 2023 at 02:36:17PM +0200, Pankaj Raghav wrote:
> Folio version of create_empty_buffers(). This is required to convert
> create_page_buffers() to folio_create_buffers() later in the series.
> 
> It removes several calls to compound_head() as it works directly on folio
> compared to create_empty_buffers(). Hence, create_empty_buffers() has
> been modified to call folio_create_empty_buffers().
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Re: [PATCH 3/4] fs/buffer: add folio_create_empty_buffers helper

[Hannes Reinecke]

On 4/17/23 14:36, Pankaj Raghav wrote:
> Folio version of create_empty_buffers(). This is required to convert
> create_page_buffers() to folio_create_buffers() later in the series.
> 
> It removes several calls to compound_head() as it works directly on folio
> compared to create_empty_buffers(). Hence, create_empty_buffers() has
> been modified to call folio_create_empty_buffers().
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>   fs/buffer.c                 | 28 +++++++++++++++++-----------
>   include/linux/buffer_head.h |  2 ++
>   2 files changed, 19 insertions(+), 11 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes

[PATCH 4/4] fs/buffer: convert create_page_buffers to folio_create_buffers

[Pankaj Raghav]

fs/buffer do not support large folios as there are many assumptions on
the folio size to be the host page size. This conversion is one step
towards removing that assumption. Also this conversion will reduce calls
to compound_head() if folio_create_buffers() calls
folio_create_empty_buffers().

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
 fs/buffer.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index 13724ef7eec7..a7fc561758b1 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1722,14 +1722,17 @@ static inline int block_size_bits(unsigned int blocksize)
 	return ilog2(blocksize);
 }
 
-static struct buffer_head *create_page_buffers(struct page *page, struct inode *inode, unsigned int b_state)
+static struct buffer_head *folio_create_buffers(struct folio *folio,
+						struct inode *inode,
+						unsigned int b_state)
 {
-	BUG_ON(!PageLocked(page));
+	BUG_ON(!folio_test_locked(folio));
 
-	if (!page_has_buffers(page))
-		create_empty_buffers(page, 1 << READ_ONCE(inode->i_blkbits),
-				     b_state);
-	return page_buffers(page);
+	if (!folio_buffers(folio))
+		folio_create_empty_buffers(folio,
+					   1 << READ_ONCE(inode->i_blkbits),
+					   b_state);
+	return folio_buffers(folio);
 }
 
 /*
@@ -1773,8 +1776,8 @@ int __block_write_full_page(struct inode *inode, struct page *page,
 	int nr_underway = 0;
 	blk_opf_t write_flags = wbc_to_write_flags(wbc);
 
-	head = create_page_buffers(page, inode,
-					(1 << BH_Dirty)|(1 << BH_Uptodate));
+	head = folio_create_buffers(page_folio(page), inode,
+				    (1 << BH_Dirty) | (1 << BH_Uptodate));
 
 	/*
 	 * Be very careful.  We have no exclusion from block_dirty_folio
@@ -2037,7 +2040,7 @@ int __block_write_begin_int(struct folio *folio, loff_t pos, unsigned len,
 	BUG_ON(to > PAGE_SIZE);
 	BUG_ON(from > to);
 
-	head = create_page_buffers(&folio->page, inode, 0);
+	head = folio_create_buffers(folio, inode, 0);
 	blocksize = head->b_size;
 	bbits = block_size_bits(blocksize);
 
@@ -2323,7 +2326,7 @@ int block_read_full_folio(struct folio *folio, get_block_t *get_block)
 
 	VM_BUG_ON_FOLIO(folio_test_large(folio), folio);
 
-	head = create_page_buffers(&folio->page, inode, 0);
+	head = folio_create_buffers(folio, inode, 0);
 	blocksize = head->b_size;
 	bbits = block_size_bits(blocksize);
 
-- 
2.34.1

Re: [PATCH 4/4] fs/buffer: convert create_page_buffers to folio_create_buffers

[Matthew Wilcox]

On Mon, Apr 17, 2023 at 02:36:18PM +0200, Pankaj Raghav wrote:
> fs/buffer do not support large folios as there are many assumptions on
> the folio size to be the host page size. This conversion is one step
> towards removing that assumption. Also this conversion will reduce calls
> to compound_head() if folio_create_buffers() calls
> folio_create_empty_buffers().
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>

Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Re: [PATCH 4/4] fs/buffer: convert create_page_buffers to folio_create_buffers

[Hannes Reinecke]

On 4/17/23 14:36, Pankaj Raghav wrote:
> fs/buffer do not support large folios as there are many assumptions on
> the folio size to be the host page size. This conversion is one step
> towards removing that assumption. Also this conversion will reduce calls
> to compound_head() if folio_create_buffers() calls
> folio_create_empty_buffers().
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
>   fs/buffer.c | 23 +++++++++++++----------
>   1 file changed, 13 insertions(+), 10 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes