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/[email protected]/
[2]https://lore.kernel.org/lkml/[email protected]/
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
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 <[email protected]>
Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
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 <[email protected]>
> ---
> fs/buffer.c | 28 +++++++++++++++++-----------
> include/linux/buffer_head.h | 2 ++
> 2 files changed, 19 insertions(+), 11 deletions(-)
>
Reviewed-by: Hannes Reinecke <[email protected]>
Cheers,
Hannes
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 <[email protected]>
> ---
> fs/buffer.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
Reviewed-by: Hannes Reinecke <[email protected]>
Cheers,
Hannes