All the callers now have a folio, so pass that in and operate on folios.
Removes four calls to compound_head().
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
fs/ext4/inode.c | 41 +++++++++++++++++++++--------------------
1 file changed, 21 insertions(+), 20 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index dbfc0670de75..507c7f88d737 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1055,12 +1055,12 @@ int do_journal_get_write_access(handle_t *handle, struct inode *inode,
}
#ifdef CONFIG_FS_ENCRYPTION
-static int ext4_block_write_begin(struct page *page, loff_t pos, unsigned len,
+static int ext4_block_write_begin(struct folio *folio, loff_t pos, unsigned len,
get_block_t *get_block)
{
unsigned from = pos & (PAGE_SIZE - 1);
unsigned to = from + len;
- struct inode *inode = page->mapping->host;
+ struct inode *inode = folio->mapping->host;
unsigned block_start, block_end;
sector_t block;
int err = 0;
@@ -1070,22 +1070,24 @@ static int ext4_block_write_begin(struct page *page, loff_t pos, unsigned len,
int nr_wait = 0;
int i;
- BUG_ON(!PageLocked(page));
+ BUG_ON(!folio_test_locked(folio));
BUG_ON(from > PAGE_SIZE);
BUG_ON(to > PAGE_SIZE);
BUG_ON(from > to);
- if (!page_has_buffers(page))
- create_empty_buffers(page, blocksize, 0);
- head = page_buffers(page);
+ head = folio_buffers(folio);
+ if (!head) {
+ create_empty_buffers(&folio->page, blocksize, 0);
+ head = folio_buffers(folio);
+ }
bbits = ilog2(blocksize);
- block = (sector_t)page->index << (PAGE_SHIFT - bbits);
+ block = (sector_t)folio->index << (PAGE_SHIFT - bbits);
for (bh = head, block_start = 0; bh != head || !block_start;
block++, block_start = block_end, bh = bh->b_this_page) {
block_end = block_start + blocksize;
if (block_end <= from || block_start >= to) {
- if (PageUptodate(page)) {
+ if (folio_test_uptodate(folio)) {
set_buffer_uptodate(bh);
}
continue;
@@ -1098,19 +1100,20 @@ static int ext4_block_write_begin(struct page *page, loff_t pos, unsigned len,
if (err)
break;
if (buffer_new(bh)) {
- if (PageUptodate(page)) {
+ if (folio_test_uptodate(folio)) {
clear_buffer_new(bh);
set_buffer_uptodate(bh);
mark_buffer_dirty(bh);
continue;
}
if (block_end > to || block_start < from)
- zero_user_segments(page, to, block_end,
- block_start, from);
+ folio_zero_segments(folio, to,
+ block_end,
+ block_start, from);
continue;
}
}
- if (PageUptodate(page)) {
+ if (folio_test_uptodate(folio)) {
set_buffer_uptodate(bh);
continue;
}
@@ -1130,13 +1133,13 @@ static int ext4_block_write_begin(struct page *page, loff_t pos, unsigned len,
err = -EIO;
}
if (unlikely(err)) {
- page_zero_new_buffers(page, from, to);
+ page_zero_new_buffers(&folio->page, from, to);
} else if (fscrypt_inode_uses_fs_layer_crypto(inode)) {
for (i = 0; i < nr_wait; i++) {
int err2;
- err2 = fscrypt_decrypt_pagecache_blocks(page, blocksize,
- bh_offset(wait[i]));
+ err2 = fscrypt_decrypt_pagecache_blocks(&folio->page,
+ blocksize, bh_offset(wait[i]));
if (err2) {
clear_buffer_uptodate(wait[i]);
err = err2;
@@ -1223,11 +1226,10 @@ static int ext4_write_begin(struct file *file, struct address_space *mapping,
#ifdef CONFIG_FS_ENCRYPTION
if (ext4_should_dioread_nolock(inode))
- ret = ext4_block_write_begin(&folio->page, pos, len,
+ ret = ext4_block_write_begin(folio, pos, len,
ext4_get_block_unwritten);
else
- ret = ext4_block_write_begin(&folio->page, pos, len,
- ext4_get_block);
+ ret = ext4_block_write_begin(folio, pos, len, ext4_get_block);
#else
if (ext4_should_dioread_nolock(inode))
ret = __block_write_begin(&folio->page, pos, len,
@@ -3082,8 +3084,7 @@ static int ext4_da_write_begin(struct file *file, struct address_space *mapping,
folio_wait_stable(folio);
#ifdef CONFIG_FS_ENCRYPTION
- ret = ext4_block_write_begin(&folio->page, pos, len,
- ext4_da_get_block_prep);
+ ret = ext4_block_write_begin(folio, pos, len, ext4_da_get_block_prep);
#else
ret = __block_write_begin(&folio->page, pos, len, ext4_da_get_block_prep);
#endif
--
2.35.1
"Matthew Wilcox (Oracle)" <[email protected]> writes:
> All the callers now have a folio, so pass that in and operate on folios.
> Removes four calls to compound_head().
Why do you say four? Isn't it 3 calls of PageUptodate(page) which
removes calls to compound_head()? Which one did I miss?
>
> Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
> ---
> fs/ext4/inode.c | 41 +++++++++++++++++++++--------------------
> 1 file changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index dbfc0670de75..507c7f88d737 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1055,12 +1055,12 @@ int do_journal_get_write_access(handle_t *handle, struct inode *inode,
> }
>
> #ifdef CONFIG_FS_ENCRYPTION
> -static int ext4_block_write_begin(struct page *page, loff_t pos, unsigned len,
> +static int ext4_block_write_begin(struct folio *folio, loff_t pos, unsigned len,
> get_block_t *get_block)
> {
> unsigned from = pos & (PAGE_SIZE - 1);
> unsigned to = from + len;
> - struct inode *inode = page->mapping->host;
> + struct inode *inode = folio->mapping->host;
> unsigned block_start, block_end;
> sector_t block;
> int err = 0;
> @@ -1070,22 +1070,24 @@ static int ext4_block_write_begin(struct page *page, loff_t pos, unsigned len,
> int nr_wait = 0;
> int i;
>
> - BUG_ON(!PageLocked(page));
> + BUG_ON(!folio_test_locked(folio));
> BUG_ON(from > PAGE_SIZE);
> BUG_ON(to > PAGE_SIZE);
> BUG_ON(from > to);
>
> - if (!page_has_buffers(page))
> - create_empty_buffers(page, blocksize, 0);
> - head = page_buffers(page);
> + head = folio_buffers(folio);
> + if (!head) {
> + create_empty_buffers(&folio->page, blocksize, 0);
> + head = folio_buffers(folio);
> + }
> bbits = ilog2(blocksize);
> - block = (sector_t)page->index << (PAGE_SHIFT - bbits);
> + block = (sector_t)folio->index << (PAGE_SHIFT - bbits);
>
> for (bh = head, block_start = 0; bh != head || !block_start;
> block++, block_start = block_end, bh = bh->b_this_page) {
> block_end = block_start + blocksize;
> if (block_end <= from || block_start >= to) {
> - if (PageUptodate(page)) {
> + if (folio_test_uptodate(folio)) {
> set_buffer_uptodate(bh);
> }
> continue;
> @@ -1098,19 +1100,20 @@ static int ext4_block_write_begin(struct page *page, loff_t pos, unsigned len,
> if (err)
> break;
> if (buffer_new(bh)) {
> - if (PageUptodate(page)) {
> + if (folio_test_uptodate(folio)) {
> clear_buffer_new(bh);
> set_buffer_uptodate(bh);
> mark_buffer_dirty(bh);
> continue;
> }
> if (block_end > to || block_start < from)
> - zero_user_segments(page, to, block_end,
> - block_start, from);
> + folio_zero_segments(folio, to,
> + block_end,
> + block_start, from);
> continue;
> }
> }
> - if (PageUptodate(page)) {
> + if (folio_test_uptodate(folio)) {
> set_buffer_uptodate(bh);
> continue;
> }
> @@ -1130,13 +1133,13 @@ static int ext4_block_write_begin(struct page *page, loff_t pos, unsigned len,
> err = -EIO;
> }
> if (unlikely(err)) {
> - page_zero_new_buffers(page, from, to);
> + page_zero_new_buffers(&folio->page, from, to);
> } else if (fscrypt_inode_uses_fs_layer_crypto(inode)) {
> for (i = 0; i < nr_wait; i++) {
> int err2;
>
> - err2 = fscrypt_decrypt_pagecache_blocks(page, blocksize,
> - bh_offset(wait[i]));
> + err2 = fscrypt_decrypt_pagecache_blocks(&folio->page,
> + blocksize, bh_offset(wait[i]));
folio_decrypt_pagecache_blocks() takes folio as it's argument now.
Other than that it looks good to me. Please feel free to add -
Reviewed-by: Ritesh Harjani (IBM) <[email protected]>
On Mon, Mar 06, 2023 at 12:21:48PM +0530, Ritesh Harjani wrote:
> "Matthew Wilcox (Oracle)" <[email protected]> writes:
>
> > All the callers now have a folio, so pass that in and operate on folios.
> > Removes four calls to compound_head().
>
> Why do you say four? Isn't it 3 calls of PageUptodate(page) which
> removes calls to compound_head()? Which one did I miss?
>
> > - BUG_ON(!PageLocked(page));
> > + BUG_ON(!folio_test_locked(folio));
That one ;-)
> > } else if (fscrypt_inode_uses_fs_layer_crypto(inode)) {
> > for (i = 0; i < nr_wait; i++) {
> > int err2;
> >
> > - err2 = fscrypt_decrypt_pagecache_blocks(page, blocksize,
> > - bh_offset(wait[i]));
> > + err2 = fscrypt_decrypt_pagecache_blocks(&folio->page,
> > + blocksize, bh_offset(wait[i]));
>
> folio_decrypt_pagecache_blocks() takes folio as it's argument now.
>
> Other than that it looks good to me. Please feel free to add -
> Reviewed-by: Ritesh Harjani (IBM) <[email protected]>
Thanks. I'll refresh this patchset next week.
Matthew Wilcox <[email protected]> writes:
> On Mon, Mar 06, 2023 at 12:21:48PM +0530, Ritesh Harjani wrote:
>> "Matthew Wilcox (Oracle)" <[email protected]> writes:
>>
>> > All the callers now have a folio, so pass that in and operate on folios.
>> > Removes four calls to compound_head().
>>
>> Why do you say four? Isn't it 3 calls of PageUptodate(page) which
>> removes calls to compound_head()? Which one did I miss?
>>
>> > - BUG_ON(!PageLocked(page));
>> > + BUG_ON(!folio_test_locked(folio));
>
> That one ;-)
__PAGEFLAG(Locked, locked, PF_NO_TAIL)
#define __PAGEFLAG(uname, lname, policy) \
TESTPAGEFLAG(uname, lname, policy) \
__SETPAGEFLAG(uname, lname, policy) \
__CLEARPAGEFLAG(uname, lname, policy)
#define TESTPAGEFLAG(uname, lname, policy) \
static __always_inline bool folio_test_##lname(struct folio *folio) \
{ return test_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \
static __always_inline int Page##uname(struct page *page) \
{ return test_bit(PG_##lname, &policy(page, 0)->flags); }
How? PageLocked(page) doesn't do any compount_head() calls no?
-ritesh
>
>> > } else if (fscrypt_inode_uses_fs_layer_crypto(inode)) {
>> > for (i = 0; i < nr_wait; i++) {
>> > int err2;
>> >
>> > - err2 = fscrypt_decrypt_pagecache_blocks(page, blocksize,
>> > - bh_offset(wait[i]));
>> > + err2 = fscrypt_decrypt_pagecache_blocks(&folio->page,
>> > + blocksize, bh_offset(wait[i]));
>>
>> folio_decrypt_pagecache_blocks() takes folio as it's argument now.
>>
>> Other than that it looks good to me. Please feel free to add -
>> Reviewed-by: Ritesh Harjani (IBM) <[email protected]>
>
> Thanks. I'll refresh this patchset next week.
Sure. Thanks!
On Mon, Mar 06, 2023 at 08:51:45PM +0530, Ritesh Harjani wrote:
> Matthew Wilcox <[email protected]> writes:
>
> > On Mon, Mar 06, 2023 at 12:21:48PM +0530, Ritesh Harjani wrote:
> >> "Matthew Wilcox (Oracle)" <[email protected]> writes:
> >>
> >> > All the callers now have a folio, so pass that in and operate on folios.
> >> > Removes four calls to compound_head().
> >>
> >> Why do you say four? Isn't it 3 calls of PageUptodate(page) which
> >> removes calls to compound_head()? Which one did I miss?
> >>
> >> > - BUG_ON(!PageLocked(page));
> >> > + BUG_ON(!folio_test_locked(folio));
> >
> > That one ;-)
>
> __PAGEFLAG(Locked, locked, PF_NO_TAIL)
>
> #define __PAGEFLAG(uname, lname, policy) \
> TESTPAGEFLAG(uname, lname, policy) \
> __SETPAGEFLAG(uname, lname, policy) \
> __CLEARPAGEFLAG(uname, lname, policy)
>
> #define TESTPAGEFLAG(uname, lname, policy) \
> static __always_inline bool folio_test_##lname(struct folio *folio) \
> { return test_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \
> static __always_inline int Page##uname(struct page *page) \
> { return test_bit(PG_##lname, &policy(page, 0)->flags); }
>
> How? PageLocked(page) doesn't do any compount_head() calls no?
You missed one piece of the definition ...
#define PF_NO_TAIL(page, enforce) ({ \
VM_BUG_ON_PGFLAGS(enforce && PageTail(page), page); \
PF_POISONED_CHECK(compound_head(page)); })
Matthew Wilcox <[email protected]> writes:
> On Mon, Mar 06, 2023 at 08:51:45PM +0530, Ritesh Harjani wrote:
>> Matthew Wilcox <[email protected]> writes:
>>
>> > On Mon, Mar 06, 2023 at 12:21:48PM +0530, Ritesh Harjani wrote:
>> >> "Matthew Wilcox (Oracle)" <[email protected]> writes:
>> >>
>> >> > All the callers now have a folio, so pass that in and operate on folios.
>> >> > Removes four calls to compound_head().
>> >>
>> >> Why do you say four? Isn't it 3 calls of PageUptodate(page) which
>> >> removes calls to compound_head()? Which one did I miss?
>> >>
>> >> > - BUG_ON(!PageLocked(page));
>> >> > + BUG_ON(!folio_test_locked(folio));
>> >
>> > That one ;-)
>>
>> __PAGEFLAG(Locked, locked, PF_NO_TAIL)
>>
>> #define __PAGEFLAG(uname, lname, policy) \
>> TESTPAGEFLAG(uname, lname, policy) \
>> __SETPAGEFLAG(uname, lname, policy) \
>> __CLEARPAGEFLAG(uname, lname, policy)
>>
>> #define TESTPAGEFLAG(uname, lname, policy) \
>> static __always_inline bool folio_test_##lname(struct folio *folio) \
>> { return test_bit(PG_##lname, folio_flags(folio, FOLIO_##policy)); } \
>> static __always_inline int Page##uname(struct page *page) \
>> { return test_bit(PG_##lname, &policy(page, 0)->flags); }
>>
>> How? PageLocked(page) doesn't do any compount_head() calls no?
>
> You missed one piece of the definition ...
>
> #define PF_NO_TAIL(page, enforce) ({ \
> VM_BUG_ON_PGFLAGS(enforce && PageTail(page), page); \
> PF_POISONED_CHECK(compound_head(page)); })
aah yes, right. Thanks for pointing it.
-ritesh