2023-09-22 02:00:02

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH 01/10] highmem: Add folio_release_kmap()

This is the folio equivalent of unmap_and_put_page(), which remains as
a wrapper for it.

Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
include/linux/highmem.h | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index 99c474de800d..4cacc0e43b51 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -551,10 +551,24 @@ static inline void folio_zero_range(struct folio *folio,
zero_user_segments(&folio->page, start, start + length, 0, 0);
}

-static inline void unmap_and_put_page(struct page *page, void *addr)
+/**
+ * folio_release_kmap - Unmap a folio and drop a refcount.
+ * @folio: The folio to release.
+ * @addr: The address previously returned by a call to kmap_local_folio().
+ *
+ * It is common, eg in directory handling to kmap a folio. This function
+ * unmaps the folio and drops the refcount that was being held to keep the
+ * folio alive while we accessed it.
+ */
+static inline void folio_release_kmap(struct folio *folio, void *addr)
{
kunmap_local(addr);
- put_page(page);
+ folio_put(folio);
+}
+
+static inline void unmap_and_put_page(struct page *page, void *addr)
+{
+ folio_release_kmap(page_folio(page), addr);
}

#endif /* _LINUX_HIGHMEM_H */
--
2.40.1


2023-09-22 06:21:48

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH 07/10] ext2: Handle large block size directories in ext2_delete_entry()

If the block size is > PAGE_SIZE, we need to calculate these offsets
relative to the start of the folio, not the page.

Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
fs/ext2/dir.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index 2fc910e99234..7e75cfaa709c 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -586,16 +586,20 @@ int ext2_add_link (struct dentry *dentry, struct inode *inode)
*/
int ext2_delete_entry(struct ext2_dir_entry_2 *dir, struct page *page)
{
- struct inode *inode = page->mapping->host;
- char *kaddr = (char *)((unsigned long)dir & PAGE_MASK);
- unsigned from = offset_in_page(dir) & ~(ext2_chunk_size(inode)-1);
- unsigned to = offset_in_page(dir) +
- ext2_rec_len_from_disk(dir->rec_len);
+ struct folio *folio = page_folio(page);
+ struct inode *inode = folio->mapping->host;
+ size_t from, to;
+ char *kaddr;
loff_t pos;
- ext2_dirent *pde = NULL;
- ext2_dirent *de = (ext2_dirent *)(kaddr + from);
+ ext2_dirent *de, *pde = NULL;
int err;

+ from = offset_in_folio(folio, dir);
+ to = from + ext2_rec_len_from_disk(dir->rec_len);
+ kaddr = (char *)dir - from;
+ from &= ~(ext2_chunk_size(inode)-1);
+ de = (ext2_dirent *)(kaddr + from);
+
while ((char*)de < (char*)dir) {
if (de->rec_len == 0) {
ext2_error(inode->i_sb, __func__,
@@ -606,18 +610,18 @@ int ext2_delete_entry(struct ext2_dir_entry_2 *dir, struct page *page)
de = ext2_next_entry(de);
}
if (pde)
- from = offset_in_page(pde);
- pos = page_offset(page) + from;
- lock_page(page);
- err = ext2_prepare_chunk(page, pos, to - from);
+ from = offset_in_folio(folio, pde);
+ pos = folio_pos(folio) + from;
+ folio_lock(folio);
+ err = ext2_prepare_chunk(&folio->page, pos, to - from);
if (err) {
- unlock_page(page);
+ folio_unlock(folio);
return err;
}
if (pde)
pde->rec_len = ext2_rec_len_to_disk(to - from);
dir->inode = 0;
- ext2_commit_chunk(page, pos, to - from);
+ ext2_commit_chunk(&folio->page, pos, to - from);
inode->i_mtime = inode_set_ctime_current(inode);
EXT2_I(inode)->i_flags &= ~EXT2_BTREE_FL;
mark_inode_dirty(inode);
--
2.40.1

2023-09-22 06:26:08

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH 06/10] ext2: Convert ext2_empty_dir() to use a folio

Save two calls to compound_head() by using the folio API.

Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
fs/ext2/dir.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index 31333b23adf3..2fc910e99234 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -669,16 +669,16 @@ int ext2_make_empty(struct inode *inode, struct inode *parent)
/*
* routine to check that the specified directory is empty (for rmdir)
*/
-int ext2_empty_dir (struct inode * inode)
+int ext2_empty_dir(struct inode *inode)
{
- struct page *page;
+ struct folio *folio;
char *kaddr;
unsigned long i, npages = dir_pages(inode);

for (i = 0; i < npages; i++) {
ext2_dirent *de;

- kaddr = ext2_get_page(inode, i, 0, &page);
+ kaddr = ext2_get_folio(inode, i, 0, &folio);
if (IS_ERR(kaddr))
return 0;

@@ -707,12 +707,12 @@ int ext2_empty_dir (struct inode * inode)
}
de = ext2_next_entry(de);
}
- ext2_put_page(page, kaddr);
+ folio_release_kmap(folio, kaddr);
}
return 1;

not_empty:
- ext2_put_page(page, kaddr);
+ folio_release_kmap(folio, kaddr);
return 0;
}

--
2.40.1

2023-09-24 10:32:01

by Matthew Wilcox

[permalink] [raw]
Subject: [PATCH 09/10] ext2: Convert ext2_make_empty() to use a folio

Remove two hidden calls to compound_head() by using the folio API.

Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
---
fs/ext2/dir.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index dad71ef38395..414680bdb170 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -618,21 +618,21 @@ int ext2_delete_entry(struct ext2_dir_entry_2 *dir, struct folio *folio)
*/
int ext2_make_empty(struct inode *inode, struct inode *parent)
{
- struct page *page = grab_cache_page(inode->i_mapping, 0);
+ struct folio *folio = filemap_grab_folio(inode->i_mapping, 0);
unsigned chunk_size = ext2_chunk_size(inode);
struct ext2_dir_entry_2 * de;
int err;
void *kaddr;

- if (!page)
- return -ENOMEM;
+ if (IS_ERR(folio))
+ return PTR_ERR(folio);

- err = ext2_prepare_chunk(page, 0, chunk_size);
+ err = ext2_prepare_chunk(&folio->page, 0, chunk_size);
if (err) {
- unlock_page(page);
+ folio_unlock(folio);
goto fail;
}
- kaddr = kmap_local_page(page);
+ kaddr = kmap_local_folio(folio, 0);
memset(kaddr, 0, chunk_size);
de = (struct ext2_dir_entry_2 *)kaddr;
de->name_len = 1;
@@ -648,10 +648,10 @@ int ext2_make_empty(struct inode *inode, struct inode *parent)
memcpy (de->name, "..\0", 4);
ext2_set_de_type (de, inode);
kunmap_local(kaddr);
- ext2_commit_chunk(page, 0, chunk_size);
+ ext2_commit_chunk(&folio->page, 0, chunk_size);
err = ext2_handle_dirsync(inode);
fail:
- put_page(page);
+ folio_put(folio);
return err;
}

--
2.40.1

2023-10-03 10:41:37

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 01/10] highmem: Add folio_release_kmap()

On Thu 21-09-23 21:07:38, Matthew Wilcox (Oracle) wrote:
> This is the folio equivalent of unmap_and_put_page(), which remains as
> a wrapper for it.
>
> Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>

I'm missing a patch 10/10 in this series (and a coverletter would be nice
as well)... What's there ;)?

Honza

> ---
> include/linux/highmem.h | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> index 99c474de800d..4cacc0e43b51 100644
> --- a/include/linux/highmem.h
> +++ b/include/linux/highmem.h
> @@ -551,10 +551,24 @@ static inline void folio_zero_range(struct folio *folio,
> zero_user_segments(&folio->page, start, start + length, 0, 0);
> }
>
> -static inline void unmap_and_put_page(struct page *page, void *addr)
> +/**
> + * folio_release_kmap - Unmap a folio and drop a refcount.
> + * @folio: The folio to release.
> + * @addr: The address previously returned by a call to kmap_local_folio().
> + *
> + * It is common, eg in directory handling to kmap a folio. This function
> + * unmaps the folio and drops the refcount that was being held to keep the
> + * folio alive while we accessed it.
> + */
> +static inline void folio_release_kmap(struct folio *folio, void *addr)
> {
> kunmap_local(addr);
> - put_page(page);
> + folio_put(folio);
> +}
> +
> +static inline void unmap_and_put_page(struct page *page, void *addr)
> +{
> + folio_release_kmap(page_folio(page), addr);
> }
>
> #endif /* _LINUX_HIGHMEM_H */
> --
> 2.40.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2023-10-03 11:56:06

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH 01/10] highmem: Add folio_release_kmap()

On Tue, Oct 03, 2023 at 12:41:28PM +0200, Jan Kara wrote:
> On Thu 21-09-23 21:07:38, Matthew Wilcox (Oracle) wrote:
> > This is the folio equivalent of unmap_and_put_page(), which remains as
> > a wrapper for it.
> >
> > Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
>
> I'm missing a patch 10/10 in this series (and a coverletter would be nice
> as well)... What's there ;)?

Email sucks! https://www.infradead.org/~willy/linux/ext2-dir-20230921/

2023-10-25 18:18:29

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 07/10] ext2: Handle large block size directories in ext2_delete_entry()

On Thu 21-09-23 21:07:44, Matthew Wilcox (Oracle) wrote:
> If the block size is > PAGE_SIZE, we need to calculate these offsets
> relative to the start of the folio, not the page.
>
> Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>

So I had a fresh look at the patches and I like them so I'll take them into
my tree for the merge window. Just this patch has somewhat surprising
subject and changelog - I guess it should be standard "convert to folios"
kind of thing, shouldn't it? Because a lot of dir code is not prepared for
"large block size" so it is strange to speak about it in
ext2_delete_entry(). I can fix it up on commit.

Honza

> ---
> fs/ext2/dir.c | 30 +++++++++++++++++-------------
> 1 file changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
> index 2fc910e99234..7e75cfaa709c 100644
> --- a/fs/ext2/dir.c
> +++ b/fs/ext2/dir.c
> @@ -586,16 +586,20 @@ int ext2_add_link (struct dentry *dentry, struct inode *inode)
> */
> int ext2_delete_entry(struct ext2_dir_entry_2 *dir, struct page *page)
> {
> - struct inode *inode = page->mapping->host;
> - char *kaddr = (char *)((unsigned long)dir & PAGE_MASK);
> - unsigned from = offset_in_page(dir) & ~(ext2_chunk_size(inode)-1);
> - unsigned to = offset_in_page(dir) +
> - ext2_rec_len_from_disk(dir->rec_len);
> + struct folio *folio = page_folio(page);
> + struct inode *inode = folio->mapping->host;
> + size_t from, to;
> + char *kaddr;
> loff_t pos;
> - ext2_dirent *pde = NULL;
> - ext2_dirent *de = (ext2_dirent *)(kaddr + from);
> + ext2_dirent *de, *pde = NULL;
> int err;
>
> + from = offset_in_folio(folio, dir);
> + to = from + ext2_rec_len_from_disk(dir->rec_len);
> + kaddr = (char *)dir - from;
> + from &= ~(ext2_chunk_size(inode)-1);
> + de = (ext2_dirent *)(kaddr + from);
> +
> while ((char*)de < (char*)dir) {
> if (de->rec_len == 0) {
> ext2_error(inode->i_sb, __func__,
> @@ -606,18 +610,18 @@ int ext2_delete_entry(struct ext2_dir_entry_2 *dir, struct page *page)
> de = ext2_next_entry(de);
> }
> if (pde)
> - from = offset_in_page(pde);
> - pos = page_offset(page) + from;
> - lock_page(page);
> - err = ext2_prepare_chunk(page, pos, to - from);
> + from = offset_in_folio(folio, pde);
> + pos = folio_pos(folio) + from;
> + folio_lock(folio);
> + err = ext2_prepare_chunk(&folio->page, pos, to - from);
> if (err) {
> - unlock_page(page);
> + folio_unlock(folio);
> return err;
> }
> if (pde)
> pde->rec_len = ext2_rec_len_to_disk(to - from);
> dir->inode = 0;
> - ext2_commit_chunk(page, pos, to - from);
> + ext2_commit_chunk(&folio->page, pos, to - from);
> inode->i_mtime = inode_set_ctime_current(inode);
> EXT2_I(inode)->i_flags &= ~EXT2_BTREE_FL;
> mark_inode_dirty(inode);
> --
> 2.40.1
>
--
Jan Kara <[email protected]>
SUSE Labs, CR