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

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

[Matthew Wilcox (Oracle)]

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

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 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

[PATCH 02/10] ext2: Convert ext2_check_page to ext2_check_folio

[Matthew Wilcox (Oracle)]

Support in this function for large folios is limited to supporting
filesystems with block size > PAGE_SIZE.  This new functionality will only
be supported on machines without HIGHMEM, so the problem of kmap_local
only being able to map a single page in the folio can be ignored.
We will not use large folios for ext2 directories on HIGHMEM machines.

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

diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index b335f17f682f..03867381eec2 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -96,19 +96,19 @@ static void ext2_commit_chunk(struct page *page, loff_t pos, unsigned len)
 	unlock_page(page);
 }
 
-static bool ext2_check_page(struct page *page, int quiet, char *kaddr)
+static bool ext2_check_folio(struct folio *folio, int quiet, char *kaddr)
 {
-	struct inode *dir = page->mapping->host;
+	struct inode *dir = folio->mapping->host;
 	struct super_block *sb = dir->i_sb;
 	unsigned chunk_size = ext2_chunk_size(dir);
 	u32 max_inumber = le32_to_cpu(EXT2_SB(sb)->s_es->s_inodes_count);
 	unsigned offs, rec_len;
-	unsigned limit = PAGE_SIZE;
+	unsigned limit = folio_size(folio);
 	ext2_dirent *p;
 	char *error;
 
-	if ((dir->i_size >> PAGE_SHIFT) == page->index) {
-		limit = dir->i_size & ~PAGE_MASK;
+	if (dir->i_size < folio_pos(folio) + limit) {
+		limit = offset_in_folio(folio, dir->i_size);
 		if (limit & (chunk_size - 1))
 			goto Ebadsize;
 		if (!limit)
@@ -132,7 +132,7 @@ static bool ext2_check_page(struct page *page, int quiet, char *kaddr)
 	if (offs != limit)
 		goto Eend;
 out:
-	SetPageChecked(page);
+	folio_set_checked(folio);
 	return true;
 
 	/* Too bad, we had an error */
@@ -160,22 +160,22 @@ static bool ext2_check_page(struct page *page, int quiet, char *kaddr)
 bad_entry:
 	if (!quiet)
 		ext2_error(sb, __func__, "bad entry in directory #%lu: : %s - "
-			"offset=%lu, inode=%lu, rec_len=%d, name_len=%d",
-			dir->i_ino, error, (page->index<<PAGE_SHIFT)+offs,
+			"offset=%llu, inode=%lu, rec_len=%d, name_len=%d",
+			dir->i_ino, error, folio_pos(folio) + offs,
 			(unsigned long) le32_to_cpu(p->inode),
 			rec_len, p->name_len);
 	goto fail;
 Eend:
 	if (!quiet) {
 		p = (ext2_dirent *)(kaddr + offs);
-		ext2_error(sb, "ext2_check_page",
+		ext2_error(sb, "ext2_check_folio",
 			"entry in directory #%lu spans the page boundary"
-			"offset=%lu, inode=%lu",
-			dir->i_ino, (page->index<<PAGE_SHIFT)+offs,
+			"offset=%llu, inode=%lu",
+			dir->i_ino, folio_pos(folio) + offs,
 			(unsigned long) le32_to_cpu(p->inode));
 	}
 fail:
-	SetPageError(page);
+	folio_set_error(folio);
 	return false;
 }
 
@@ -195,9 +195,9 @@ static void *ext2_get_page(struct inode *dir, unsigned long n,
 
 	if (IS_ERR(folio))
 		return ERR_CAST(folio);
-	page_addr = kmap_local_folio(folio, n & (folio_nr_pages(folio) - 1));
+	page_addr = kmap_local_folio(folio, 0);
 	if (unlikely(!folio_test_checked(folio))) {
-		if (!ext2_check_page(&folio->page, quiet, page_addr))
+		if (!ext2_check_folio(folio, quiet, page_addr))
 			goto fail;
 	}
 	*page = &folio->page;
-- 
2.40.1

Re: [PATCH 02/10] ext2: Convert ext2_check_page to ext2_check_folio

[Jan Kara]

On Thu 21-09-23 21:07:39, Matthew Wilcox (Oracle) wrote:
> Support in this function for large folios is limited to supporting
> filesystems with block size > PAGE_SIZE.  This new functionality will only
> be supported on machines without HIGHMEM, so the problem of kmap_local
> only being able to map a single page in the folio can be ignored.
> We will not use large folios for ext2 directories on HIGHMEM machines.

OK, but can we perhaps enforce this with some checks & error messages
instead of a silent failure? Like:

#ifdef CONFIG_HIGHMEM
	if (sb->s_blocksize > PAGE_SIZE)
		bail with error
#endif

somewhere in ext2_fill_super()? Or maybe force allocation of lowmem pages
when blocksize > PAGE_SIZE?

								Honza

> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/ext2/dir.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
> index b335f17f682f..03867381eec2 100644
> --- a/fs/ext2/dir.c
> +++ b/fs/ext2/dir.c
> @@ -96,19 +96,19 @@ static void ext2_commit_chunk(struct page *page, loff_t pos, unsigned len)
>  	unlock_page(page);
>  }
>  
> -static bool ext2_check_page(struct page *page, int quiet, char *kaddr)
> +static bool ext2_check_folio(struct folio *folio, int quiet, char *kaddr)
>  {
> -	struct inode *dir = page->mapping->host;
> +	struct inode *dir = folio->mapping->host;
>  	struct super_block *sb = dir->i_sb;
>  	unsigned chunk_size = ext2_chunk_size(dir);
>  	u32 max_inumber = le32_to_cpu(EXT2_SB(sb)->s_es->s_inodes_count);
>  	unsigned offs, rec_len;
> -	unsigned limit = PAGE_SIZE;
> +	unsigned limit = folio_size(folio);
>  	ext2_dirent *p;
>  	char *error;
>  
> -	if ((dir->i_size >> PAGE_SHIFT) == page->index) {
> -		limit = dir->i_size & ~PAGE_MASK;
> +	if (dir->i_size < folio_pos(folio) + limit) {
> +		limit = offset_in_folio(folio, dir->i_size);
>  		if (limit & (chunk_size - 1))
>  			goto Ebadsize;
>  		if (!limit)
> @@ -132,7 +132,7 @@ static bool ext2_check_page(struct page *page, int quiet, char *kaddr)
>  	if (offs != limit)
>  		goto Eend;
>  out:
> -	SetPageChecked(page);
> +	folio_set_checked(folio);
>  	return true;
>  
>  	/* Too bad, we had an error */
> @@ -160,22 +160,22 @@ static bool ext2_check_page(struct page *page, int quiet, char *kaddr)
>  bad_entry:
>  	if (!quiet)
>  		ext2_error(sb, __func__, "bad entry in directory #%lu: : %s - "
> -			"offset=%lu, inode=%lu, rec_len=%d, name_len=%d",
> -			dir->i_ino, error, (page->index<<PAGE_SHIFT)+offs,
> +			"offset=%llu, inode=%lu, rec_len=%d, name_len=%d",
> +			dir->i_ino, error, folio_pos(folio) + offs,
>  			(unsigned long) le32_to_cpu(p->inode),
>  			rec_len, p->name_len);
>  	goto fail;
>  Eend:
>  	if (!quiet) {
>  		p = (ext2_dirent *)(kaddr + offs);
> -		ext2_error(sb, "ext2_check_page",
> +		ext2_error(sb, "ext2_check_folio",
>  			"entry in directory #%lu spans the page boundary"
> -			"offset=%lu, inode=%lu",
> -			dir->i_ino, (page->index<<PAGE_SHIFT)+offs,
> +			"offset=%llu, inode=%lu",
> +			dir->i_ino, folio_pos(folio) + offs,
>  			(unsigned long) le32_to_cpu(p->inode));
>  	}
>  fail:
> -	SetPageError(page);
> +	folio_set_error(folio);
>  	return false;
>  }
>  
> @@ -195,9 +195,9 @@ static void *ext2_get_page(struct inode *dir, unsigned long n,
>  
>  	if (IS_ERR(folio))
>  		return ERR_CAST(folio);
> -	page_addr = kmap_local_folio(folio, n & (folio_nr_pages(folio) - 1));
> +	page_addr = kmap_local_folio(folio, 0);
>  	if (unlikely(!folio_test_checked(folio))) {
> -		if (!ext2_check_page(&folio->page, quiet, page_addr))
> +		if (!ext2_check_folio(folio, quiet, page_addr))
>  			goto fail;
>  	}
>  	*page = &folio->page;
> -- 
> 2.40.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 02/10] ext2: Convert ext2_check_page to ext2_check_folio

[Jan Kara]

On Tue 03-10-23 12:40:17, Jan Kara wrote:
> On Thu 21-09-23 21:07:39, Matthew Wilcox (Oracle) wrote:
> > Support in this function for large folios is limited to supporting
> > filesystems with block size > PAGE_SIZE.  This new functionality will only
> > be supported on machines without HIGHMEM, so the problem of kmap_local
> > only being able to map a single page in the folio can be ignored.
> > We will not use large folios for ext2 directories on HIGHMEM machines.
> 
> OK, but can we perhaps enforce this with some checks & error messages
> instead of a silent failure? Like:
> 
> #ifdef CONFIG_HIGHMEM
> 	if (sb->s_blocksize > PAGE_SIZE)
> 		bail with error
> #endif
> 
> somewhere in ext2_fill_super()? Or maybe force allocation of lowmem pages
> when blocksize > PAGE_SIZE?
> 
> > @@ -195,9 +195,9 @@ static void *ext2_get_page(struct inode *dir, unsigned long n,
> >  
> >  	if (IS_ERR(folio))
> >  		return ERR_CAST(folio);
> > -	page_addr = kmap_local_folio(folio, n & (folio_nr_pages(folio) - 1));
> > +	page_addr = kmap_local_folio(folio, 0);

Oh, and I think this change breaks the code whenever we get back higher
order folio because the page_addr we get back is not for the page index
'n'.
								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 02/10] ext2: Convert ext2_check_page to ext2_check_folio

[Matthew Wilcox]

On Tue, Oct 03, 2023 at 12:52:24PM +0200, Jan Kara wrote:
> On Tue 03-10-23 12:40:17, Jan Kara wrote:
> > On Thu 21-09-23 21:07:39, Matthew Wilcox (Oracle) wrote:
> > > Support in this function for large folios is limited to supporting
> > > filesystems with block size > PAGE_SIZE.  This new functionality will only
> > > be supported on machines without HIGHMEM, so the problem of kmap_local
> > > only being able to map a single page in the folio can be ignored.
> > > We will not use large folios for ext2 directories on HIGHMEM machines.
> > 
> > OK, but can we perhaps enforce this with some checks & error messages
> > instead of a silent failure? Like:
> > 
> > #ifdef CONFIG_HIGHMEM
> > 	if (sb->s_blocksize > PAGE_SIZE)
> > 		bail with error
> > #endif
> > 
> > somewhere in ext2_fill_super()? Or maybe force allocation of lowmem pages
> > when blocksize > PAGE_SIZE?
> > 
> > > @@ -195,9 +195,9 @@ static void *ext2_get_page(struct inode *dir, unsigned long n,
> > >  
> > >  	if (IS_ERR(folio))
> > >  		return ERR_CAST(folio);
> > > -	page_addr = kmap_local_folio(folio, n & (folio_nr_pages(folio) - 1));
> > > +	page_addr = kmap_local_folio(folio, 0);
> 
> Oh, and I think this change breaks the code whenever we get back higher
> order folio because the page_addr we get back is not for the page index
> 'n'.

> A look like a fool replying to the same patch multiple times ;) but how is
> this supposed to work even without HIGHMEM? Suppose we have a page size 4k
> and block size 16k. Directory entries in a block can then straddle 4k
> boundary so if kmap_local() is mapping only a single page, then we are
> going to have hard time parsing this all?
> 
> Oh, I guess you are pointing to the fact kmap_local_folio() gives you back
> address usable for the whole folio access if !HIGHMEM. But then all the
> iterations (e.g. in ext2_readdir()) has to be folio-size based and not
> page-size based as they currently are? You didn't change these iterations
> in your patches which has confused me...

I'm going to reply to all three emails as one ;-)

The goal for this patchset is simply to do the folio conversion.
Earlier work was focused on "make sure everything behaves as it did
before", but now I've started to give some thought to "How will this
work" without actually doing the higher level work.

Today, ext2 does not permit large folios to be used for directory
inodes.  Whoever adds the call to mapping_set_large_folios() gets the
joy of finding all the places that contain assumptions and fixing them.
I don't particularly want to do that myself; I'm just trying to sort
out the underpinnings so that whoever does it has a somewhat sane
API to work against.

So I'm really just trying to do two things here:

 - Convert uses of struct page -> struct folio
 - Not leave any landmines for whoever comes after me

I had a quick look at converting ext2_readdir() to work for
large folios.  It's a bit intrusive, and I haven't done any
serious testing (let alone with the bs>PS patches).  But it could
look something like this:


diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index 6807df637112..5eeb57ce6a18 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -257,45 +257,45 @@ static inline void ext2_set_de_type(ext2_dirent *de, struct inode *inode)
 static int
 ext2_readdir(struct file *file, struct dir_context *ctx)
 {
-	loff_t pos = ctx->pos;
 	struct inode *inode = file_inode(file);
+	loff_t pos = ctx->pos;
+	loff_t isize = i_size_read(inode);
 	struct super_block *sb = inode->i_sb;
-	unsigned int offset = pos & ~PAGE_MASK;
-	unsigned long n = pos >> PAGE_SHIFT;
-	unsigned long npages = dir_pages(inode);
 	unsigned chunk_mask = ~(ext2_chunk_size(inode)-1);
 	bool need_revalidate = !inode_eq_iversion(inode, file->f_version);
 	bool has_filetype;
 
-	if (pos > inode->i_size - EXT2_DIR_REC_LEN(1))
+	if (pos > isize - EXT2_DIR_REC_LEN(1))
 		return 0;
 
 	has_filetype =
 		EXT2_HAS_INCOMPAT_FEATURE(sb, EXT2_FEATURE_INCOMPAT_FILETYPE);
 
-	for ( ; n < npages; n++, offset = 0) {
+	while (pos < isize) {
 		ext2_dirent *de;
 		struct folio *folio;
-		char *kaddr = ext2_get_folio(inode, n, 0, &folio);
+		char *kaddr = ext2_get_folio(inode, pos / PAGE_SIZE, 0, &folio);
+		size_t offset = offset_in_folio(folio, pos);
 		char *limit;
 
 		if (IS_ERR(kaddr)) {
 			ext2_error(sb, __func__,
 				   "bad page in #%lu",
 				   inode->i_ino);
-			ctx->pos += PAGE_SIZE - offset;
+			ctx->pos += folio_size(folio) - offset;
 			return PTR_ERR(kaddr);
 		}
 		if (unlikely(need_revalidate)) {
 			if (offset) {
 				offset = ext2_validate_entry(kaddr, offset, chunk_mask);
-				ctx->pos = (n<<PAGE_SHIFT) + offset;
+				ctx->pos = folio_pos(folio) + offset;
 			}
 			file->f_version = inode_query_iversion(inode);
 			need_revalidate = false;
 		}
 		de = (ext2_dirent *)(kaddr+offset);
-		limit = kaddr + ext2_last_byte(inode, n) - EXT2_DIR_REC_LEN(1);
+		limit = kaddr + ext2_last_byte(inode, folio->index) -
+				EXT2_DIR_REC_LEN(1);
 		for ( ;(char*)de <= limit; de = ext2_next_entry(de)) {
 			if (de->rec_len == 0) {
 				ext2_error(sb, __func__,
@@ -319,6 +319,7 @@ ext2_readdir(struct file *file, struct dir_context *ctx)
 			ctx->pos += ext2_rec_len_from_disk(de->rec_len);
 		}
 		folio_release_kmap(folio, kaddr);
+		pos = (loff_t)folio_next_index(folio) * PAGE_SIZE;
 	}
 	return 0;
 }

ext2_last_byte() is clearly now incorrect.  It should probably take
isize and folio as inputs and return either the last byte in the folio
or the last byte in the file.  But then we need to convert all the
callers, and I didn't want to do that for this demonstration patch.

Re: [PATCH 02/10] ext2: Convert ext2_check_page to ext2_check_folio

[Jan Kara]

On Thu 21-09-23 21:07:39, Matthew Wilcox (Oracle) wrote:
> Support in this function for large folios is limited to supporting
> filesystems with block size > PAGE_SIZE.  This new functionality will only
> be supported on machines without HIGHMEM, so the problem of kmap_local
> only being able to map a single page in the folio can be ignored.
> We will not use large folios for ext2 directories on HIGHMEM machines.

A look like a fool replying to the same patch multiple times ;) but how is
this supposed to work even without HIGHMEM? Suppose we have a page size 4k
and block size 16k. Directory entries in a block can then straddle 4k
boundary so if kmap_local() is mapping only a single page, then we are
going to have hard time parsing this all?

Oh, I guess you are pointing to the fact kmap_local_folio() gives you back
address usable for the whole folio access if !HIGHMEM. But then all the
iterations (e.g. in ext2_readdir()) has to be folio-size based and not
page-size based as they currently are? You didn't change these iterations
in your patches which has confused me...

								Honza 

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 03/10] ext2: Add ext2_get_folio()

[Matthew Wilcox (Oracle)]

Convert ext2_get_page() into ext2_get_folio() and keep the original
function around as a temporary wrapper.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ext2/dir.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index 03867381eec2..5c1b7bded535 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -180,34 +180,46 @@ static bool ext2_check_folio(struct folio *folio, int quiet, char *kaddr)
 }
 
 /*
- * Calls to ext2_get_page()/ext2_put_page() must be nested according to the
- * rules documented in kmap_local_page()/kunmap_local().
+ * Calls to ext2_get_folio()/folio_release_kmap() must be nested according
+ * to the rules documented in kmap_local_folio()/kunmap_local().
  *
- * NOTE: ext2_find_entry() and ext2_dotdot() act as a call to ext2_get_page()
- * and should be treated as a call to ext2_get_page() for nesting purposes.
+ * NOTE: ext2_find_entry() and ext2_dotdot() act as a call
+ * to folio_release_kmap() and should be treated as a call to
+ * folio_release_kmap() for nesting purposes.
  */
-static void *ext2_get_page(struct inode *dir, unsigned long n,
-				   int quiet, struct page **page)
+static void *ext2_get_folio(struct inode *dir, unsigned long n,
+				   int quiet, struct folio **foliop)
 {
 	struct address_space *mapping = dir->i_mapping;
 	struct folio *folio = read_mapping_folio(mapping, n, NULL);
-	void *page_addr;
+	void *kaddr;
 
 	if (IS_ERR(folio))
 		return ERR_CAST(folio);
-	page_addr = kmap_local_folio(folio, 0);
+	kaddr = kmap_local_folio(folio, 0);
 	if (unlikely(!folio_test_checked(folio))) {
-		if (!ext2_check_folio(folio, quiet, page_addr))
+		if (!ext2_check_folio(folio, quiet, kaddr))
 			goto fail;
 	}
-	*page = &folio->page;
-	return page_addr;
+	*foliop = folio;
+	return kaddr;
 
 fail:
-	ext2_put_page(&folio->page, page_addr);
+	folio_release_kmap(folio, kaddr);
 	return ERR_PTR(-EIO);
 }
 
+static void *ext2_get_page(struct inode *dir, unsigned long n,
+				   int quiet, struct page **pagep)
+{
+	struct folio *folio;
+	void *kaddr = ext2_get_folio(dir, n, quiet, &folio);
+
+	if (!IS_ERR(kaddr))
+		*pagep = &folio->page;
+	return kaddr;
+}
+
 /*
  * NOTE! unlike strncmp, ext2_match returns 1 for success, 0 for failure.
  *
-- 
2.40.1

[PATCH 04/10] ext2: Convert ext2_readdir to use a folio

[Matthew Wilcox (Oracle)]

Saves a hidden call to compound_head().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 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 5c1b7bded535..5a8a02d6be9a 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -286,8 +286,8 @@ ext2_readdir(struct file *file, struct dir_context *ctx)
 
 	for ( ; n < npages; n++, offset = 0) {
 		ext2_dirent *de;
-		struct page *page;
-		char *kaddr = ext2_get_page(inode, n, 0, &page);
+		struct folio *folio;
+		char *kaddr = ext2_get_folio(inode, n, 0, &folio);
 		char *limit;
 
 		if (IS_ERR(kaddr)) {
@@ -311,7 +311,7 @@ ext2_readdir(struct file *file, struct dir_context *ctx)
 			if (de->rec_len == 0) {
 				ext2_error(sb, __func__,
 					"zero-length directory entry");
-				ext2_put_page(page, de);
+				folio_release_kmap(folio, de);
 				return -EIO;
 			}
 			if (de->inode) {
@@ -323,13 +323,13 @@ ext2_readdir(struct file *file, struct dir_context *ctx)
 				if (!dir_emit(ctx, de->name, de->name_len,
 						le32_to_cpu(de->inode),
 						d_type)) {
-					ext2_put_page(page, de);
+					folio_release_kmap(folio, de);
 					return 0;
 				}
 			}
 			ctx->pos += ext2_rec_len_from_disk(de->rec_len);
 		}
-		ext2_put_page(page, kaddr);
+		folio_release_kmap(folio, kaddr);
 	}
 	return 0;
 }
-- 
2.40.1

[PATCH 05/10] ext2: Convert ext2_add_link() to use a folio

[Matthew Wilcox (Oracle)]

Remove five hidden calls to compound_head() and fix a couple of
places that assumed PAGE_SIZE.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ext2/dir.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index 5a8a02d6be9a..31333b23adf3 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -497,7 +497,7 @@ int ext2_add_link (struct dentry *dentry, struct inode *inode)
 	unsigned chunk_size = ext2_chunk_size(dir);
 	unsigned reclen = EXT2_DIR_REC_LEN(namelen);
 	unsigned short rec_len, name_len;
-	struct page *page = NULL;
+	struct folio *folio = NULL;
 	ext2_dirent * de;
 	unsigned long npages = dir_pages(dir);
 	unsigned long n;
@@ -506,19 +506,19 @@ int ext2_add_link (struct dentry *dentry, struct inode *inode)
 
 	/*
 	 * We take care of directory expansion in the same loop.
-	 * This code plays outside i_size, so it locks the page
+	 * This code plays outside i_size, so it locks the folio
 	 * to protect that region.
 	 */
 	for (n = 0; n <= npages; n++) {
-		char *kaddr = ext2_get_page(dir, n, 0, &page);
+		char *kaddr = ext2_get_folio(dir, n, 0, &folio);
 		char *dir_end;
 
 		if (IS_ERR(kaddr))
 			return PTR_ERR(kaddr);
-		lock_page(page);
+		folio_lock(folio);
 		dir_end = kaddr + ext2_last_byte(dir, n);
 		de = (ext2_dirent *)kaddr;
-		kaddr += PAGE_SIZE - reclen;
+		kaddr += folio_size(folio) - reclen;
 		while ((char *)de <= kaddr) {
 			if ((char *)de == dir_end) {
 				/* We hit i_size */
@@ -545,15 +545,15 @@ int ext2_add_link (struct dentry *dentry, struct inode *inode)
 				goto got_it;
 			de = (ext2_dirent *) ((char *) de + rec_len);
 		}
-		unlock_page(page);
-		ext2_put_page(page, kaddr);
+		folio_unlock(folio);
+		folio_release_kmap(folio, kaddr);
 	}
 	BUG();
 	return -EINVAL;
 
 got_it:
-	pos = page_offset(page) + offset_in_page(de);
-	err = ext2_prepare_chunk(page, pos, rec_len);
+	pos = folio_pos(folio) + offset_in_folio(folio, de);
+	err = ext2_prepare_chunk(&folio->page, pos, rec_len);
 	if (err)
 		goto out_unlock;
 	if (de->inode) {
@@ -566,17 +566,17 @@ int ext2_add_link (struct dentry *dentry, struct inode *inode)
 	memcpy(de->name, name, namelen);
 	de->inode = cpu_to_le32(inode->i_ino);
 	ext2_set_de_type (de, inode);
-	ext2_commit_chunk(page, pos, rec_len);
+	ext2_commit_chunk(&folio->page, pos, rec_len);
 	dir->i_mtime = inode_set_ctime_current(dir);
 	EXT2_I(dir)->i_flags &= ~EXT2_BTREE_FL;
 	mark_inode_dirty(dir);
 	err = ext2_handle_dirsync(dir);
 	/* OFFSET_CACHE */
 out_put:
-	ext2_put_page(page, de);
+	folio_release_kmap(folio, de);
 	return err;
 out_unlock:
-	unlock_page(page);
+	folio_unlock(folio);
 	goto out_put;
 }
 
-- 
2.40.1

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

[Matthew Wilcox (Oracle)]

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

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 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

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

[Matthew Wilcox (Oracle)]

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) <willy@infradead.org>
---
 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

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

[Jan Kara]

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) <willy@infradead.org>

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 <jack@suse.com>
SUSE Labs, CR

[PATCH 08/10] ext2: Convert ext2_unlink() and ext2_rename() to use folios

[Matthew Wilcox (Oracle)]

This involves changing ext2_find_entry(), ext2_dotdot(),
ext2_inode_by_name(), ext2_set_link() and ext2_delete_entry() to
take a folio.  These were also the last users of ext2_get_page() and
ext2_put_page(), so remove those at the same time.
---
 fs/ext2/dir.c   | 75 ++++++++++++++++++++-----------------------------
 fs/ext2/ext2.h  | 23 ++++++---------
 fs/ext2/namei.c | 32 ++++++++++-----------
 3 files changed, 55 insertions(+), 75 deletions(-)

diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index 7e75cfaa709c..dad71ef38395 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -209,17 +209,6 @@ static void *ext2_get_folio(struct inode *dir, unsigned long n,
 	return ERR_PTR(-EIO);
 }
 
-static void *ext2_get_page(struct inode *dir, unsigned long n,
-				   int quiet, struct page **pagep)
-{
-	struct folio *folio;
-	void *kaddr = ext2_get_folio(dir, n, quiet, &folio);
-
-	if (!IS_ERR(kaddr))
-		*pagep = &folio->page;
-	return kaddr;
-}
-
 /*
  * NOTE! unlike strncmp, ext2_match returns 1 for success, 0 for failure.
  *
@@ -342,38 +331,35 @@ ext2_readdir(struct file *file, struct dir_context *ctx)
  * and the entry itself. Page is returned mapped and unlocked.
  * Entry is guaranteed to be valid.
  *
- * On Success ext2_put_page() should be called on *res_page.
+ * On Success folio_release_kmap() should be called on *foliop.
  *
- * NOTE: Calls to ext2_get_page()/ext2_put_page() must be nested according to
- * the rules documented in kmap_local_page()/kunmap_local().
+ * NOTE: Calls to ext2_get_folio()/folio_release_kmap() must be nested
+ * according to the rules documented in kmap_local_folio()/kunmap_local().
  *
- * ext2_find_entry() and ext2_dotdot() act as a call to ext2_get_page() and
- * should be treated as a call to ext2_get_page() for nesting purposes.
+ * ext2_find_entry() and ext2_dotdot() act as a call to ext2_get_folio()
+ * and should be treated as a call to ext2_get_folio() for nesting
+ * purposes.
  */
 struct ext2_dir_entry_2 *ext2_find_entry (struct inode *dir,
-			const struct qstr *child, struct page **res_page)
+			const struct qstr *child, struct folio **foliop)
 {
 	const char *name = child->name;
 	int namelen = child->len;
 	unsigned reclen = EXT2_DIR_REC_LEN(namelen);
 	unsigned long start, n;
 	unsigned long npages = dir_pages(dir);
-	struct page *page = NULL;
 	struct ext2_inode_info *ei = EXT2_I(dir);
 	ext2_dirent * de;
 
 	if (npages == 0)
 		goto out;
 
-	/* OFFSET_CACHE */
-	*res_page = NULL;
-
 	start = ei->i_dir_start_lookup;
 	if (start >= npages)
 		start = 0;
 	n = start;
 	do {
-		char *kaddr = ext2_get_page(dir, n, 0, &page);
+		char *kaddr = ext2_get_folio(dir, n, 0, foliop);
 		if (IS_ERR(kaddr))
 			return ERR_CAST(kaddr);
 
@@ -383,18 +369,18 @@ struct ext2_dir_entry_2 *ext2_find_entry (struct inode *dir,
 			if (de->rec_len == 0) {
 				ext2_error(dir->i_sb, __func__,
 					"zero-length directory entry");
-				ext2_put_page(page, de);
+				folio_release_kmap(*foliop, de);
 				goto out;
 			}
 			if (ext2_match(namelen, name, de))
 				goto found;
 			de = ext2_next_entry(de);
 		}
-		ext2_put_page(page, kaddr);
+		folio_release_kmap(*foliop, kaddr);
 
 		if (++n >= npages)
 			n = 0;
-		/* next page is past the blocks we've got */
+		/* next folio is past the blocks we've got */
 		if (unlikely(n > (dir->i_blocks >> (PAGE_SHIFT - 9)))) {
 			ext2_error(dir->i_sb, __func__,
 				"dir %lu size %lld exceeds block count %llu",
@@ -407,7 +393,6 @@ struct ext2_dir_entry_2 *ext2_find_entry (struct inode *dir,
 	return ERR_PTR(-ENOENT);
 
 found:
-	*res_page = page;
 	ei->i_dir_start_lookup = n;
 	return de;
 }
@@ -416,17 +401,18 @@ struct ext2_dir_entry_2 *ext2_find_entry (struct inode *dir,
  * Return the '..' directory entry and the page in which the entry was found
  * (as a parameter - p).
  *
- * On Success ext2_put_page() should be called on *p.
+ * On Success folio_release_kmap() should be called on *foliop.
  *
- * NOTE: Calls to ext2_get_page()/ext2_put_page() must be nested according to
- * the rules documented in kmap_local_page()/kunmap_local().
+ * NOTE: Calls to ext2_get_folio()/folio_release_kmap() must be nested
+ * according to the rules documented in kmap_local_folio()/kunmap_local().
  *
- * ext2_find_entry() and ext2_dotdot() act as a call to ext2_get_page() and
- * should be treated as a call to ext2_get_page() for nesting purposes.
+ * ext2_find_entry() and ext2_dotdot() act as a call to ext2_get_folio()
+ * and should be treated as a call to ext2_get_folio() for nesting
+ * purposes.
  */
-struct ext2_dir_entry_2 *ext2_dotdot(struct inode *dir, struct page **p)
+struct ext2_dir_entry_2 *ext2_dotdot(struct inode *dir, struct folio **foliop)
 {
-	ext2_dirent *de = ext2_get_page(dir, 0, 0, p);
+	ext2_dirent *de = ext2_get_folio(dir, 0, 0, foliop);
 
 	if (!IS_ERR(de))
 		return ext2_next_entry(de);
@@ -436,14 +422,14 @@ struct ext2_dir_entry_2 *ext2_dotdot(struct inode *dir, struct page **p)
 int ext2_inode_by_name(struct inode *dir, const struct qstr *child, ino_t *ino)
 {
 	struct ext2_dir_entry_2 *de;
-	struct page *page;
-	
-	de = ext2_find_entry(dir, child, &page);
+	struct folio *folio;
+
+	de = ext2_find_entry(dir, child, &folio);
 	if (IS_ERR(de))
 		return PTR_ERR(de);
 
 	*ino = le32_to_cpu(de->inode);
-	ext2_put_page(page, de);
+	folio_release_kmap(folio, de);
 	return 0;
 }
 
@@ -464,21 +450,21 @@ static int ext2_handle_dirsync(struct inode *dir)
 }
 
 int ext2_set_link(struct inode *dir, struct ext2_dir_entry_2 *de,
-		struct page *page, struct inode *inode, bool update_times)
+		struct folio *folio, struct inode *inode, bool update_times)
 {
-	loff_t pos = page_offset(page) + offset_in_page(de);
+	loff_t pos = folio_pos(folio) + offset_in_folio(folio, de);
 	unsigned len = ext2_rec_len_from_disk(de->rec_len);
 	int err;
 
-	lock_page(page);
-	err = ext2_prepare_chunk(page, pos, len);
+	folio_lock(folio);
+	err = ext2_prepare_chunk(&folio->page, pos, len);
 	if (err) {
-		unlock_page(page);
+		folio_unlock(folio);
 		return err;
 	}
 	de->inode = cpu_to_le32(inode->i_ino);
 	ext2_set_de_type(de, inode);
-	ext2_commit_chunk(page, pos, len);
+	ext2_commit_chunk(&folio->page, pos, len);
 	if (update_times)
 		dir->i_mtime = inode_set_ctime_current(dir);
 	EXT2_I(dir)->i_flags &= ~EXT2_BTREE_FL;
@@ -584,9 +570,8 @@ int ext2_add_link (struct dentry *dentry, struct inode *inode)
  * ext2_delete_entry deletes a directory entry by merging it with the
  * previous entry. Page is up-to-date.
  */
-int ext2_delete_entry(struct ext2_dir_entry_2 *dir, struct page *page)
+int ext2_delete_entry(struct ext2_dir_entry_2 *dir, struct folio *folio)
 {
-	struct folio *folio = page_folio(page);
 	struct inode *inode = folio->mapping->host;
 	size_t from, to;
 	char *kaddr;
diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 7fdd685c384d..677a9ad45dcb 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -717,22 +717,17 @@ extern void ext2_init_block_alloc_info(struct inode *);
 extern void ext2_rsv_window_add(struct super_block *sb, struct ext2_reserve_window_node *rsv);
 
 /* dir.c */
-extern int ext2_add_link (struct dentry *, struct inode *);
-extern int ext2_inode_by_name(struct inode *dir,
+int ext2_add_link(struct dentry *, struct inode *);
+int ext2_inode_by_name(struct inode *dir,
 			      const struct qstr *child, ino_t *ino);
-extern int ext2_make_empty(struct inode *, struct inode *);
-extern struct ext2_dir_entry_2 *ext2_find_entry(struct inode *, const struct qstr *,
-						struct page **);
-extern int ext2_delete_entry(struct ext2_dir_entry_2 *dir, struct page *page);
-extern int ext2_empty_dir (struct inode *);
-extern struct ext2_dir_entry_2 *ext2_dotdot(struct inode *dir, struct page **p);
+int ext2_make_empty(struct inode *, struct inode *);
+struct ext2_dir_entry_2 *ext2_find_entry(struct inode *, const struct qstr *,
+		struct folio **foliop);
+int ext2_delete_entry(struct ext2_dir_entry_2 *dir, struct folio *folio);
+int ext2_empty_dir(struct inode *);
+struct ext2_dir_entry_2 *ext2_dotdot(struct inode *dir, struct folio **foliop);
 int ext2_set_link(struct inode *dir, struct ext2_dir_entry_2 *de,
-		struct page *page, struct inode *inode, bool update_times);
-static inline void ext2_put_page(struct page *page, void *page_addr)
-{
-	kunmap_local(page_addr);
-	put_page(page);
-}
+		struct folio *folio, struct inode *inode, bool update_times);
 
 /* ialloc.c */
 extern struct inode * ext2_new_inode (struct inode *, umode_t, const struct qstr *);
diff --git a/fs/ext2/namei.c b/fs/ext2/namei.c
index 059517068adc..65f702b1da5b 100644
--- a/fs/ext2/namei.c
+++ b/fs/ext2/namei.c
@@ -273,21 +273,21 @@ static int ext2_unlink(struct inode *dir, struct dentry *dentry)
 {
 	struct inode *inode = d_inode(dentry);
 	struct ext2_dir_entry_2 *de;
-	struct page *page;
+	struct folio *folio;
 	int err;
 
 	err = dquot_initialize(dir);
 	if (err)
 		goto out;
 
-	de = ext2_find_entry(dir, &dentry->d_name, &page);
+	de = ext2_find_entry(dir, &dentry->d_name, &folio);
 	if (IS_ERR(de)) {
 		err = PTR_ERR(de);
 		goto out;
 	}
 
-	err = ext2_delete_entry(de, page);
-	ext2_put_page(page, de);
+	err = ext2_delete_entry(de, folio);
+	folio_release_kmap(folio, de);
 	if (err)
 		goto out;
 
@@ -321,9 +321,9 @@ static int ext2_rename (struct mnt_idmap * idmap,
 {
 	struct inode * old_inode = d_inode(old_dentry);
 	struct inode * new_inode = d_inode(new_dentry);
-	struct page * dir_page = NULL;
+	struct folio *dir_folio = NULL;
 	struct ext2_dir_entry_2 * dir_de = NULL;
-	struct page * old_page;
+	struct folio * old_folio;
 	struct ext2_dir_entry_2 * old_de;
 	int err;
 
@@ -338,19 +338,19 @@ static int ext2_rename (struct mnt_idmap * idmap,
 	if (err)
 		return err;
 
-	old_de = ext2_find_entry(old_dir, &old_dentry->d_name, &old_page);
+	old_de = ext2_find_entry(old_dir, &old_dentry->d_name, &old_folio);
 	if (IS_ERR(old_de))
 		return PTR_ERR(old_de);
 
 	if (S_ISDIR(old_inode->i_mode)) {
 		err = -EIO;
-		dir_de = ext2_dotdot(old_inode, &dir_page);
+		dir_de = ext2_dotdot(old_inode, &dir_folio);
 		if (!dir_de)
 			goto out_old;
 	}
 
 	if (new_inode) {
-		struct page *new_page;
+		struct folio *new_folio;
 		struct ext2_dir_entry_2 *new_de;
 
 		err = -ENOTEMPTY;
@@ -358,13 +358,13 @@ static int ext2_rename (struct mnt_idmap * idmap,
 			goto out_dir;
 
 		new_de = ext2_find_entry(new_dir, &new_dentry->d_name,
-					 &new_page);
+					 &new_folio);
 		if (IS_ERR(new_de)) {
 			err = PTR_ERR(new_de);
 			goto out_dir;
 		}
-		err = ext2_set_link(new_dir, new_de, new_page, old_inode, true);
-		ext2_put_page(new_page, new_de);
+		err = ext2_set_link(new_dir, new_de, new_folio, old_inode, true);
+		folio_release_kmap(new_folio, new_de);
 		if (err)
 			goto out_dir;
 		inode_set_ctime_current(new_inode);
@@ -386,19 +386,19 @@ static int ext2_rename (struct mnt_idmap * idmap,
 	inode_set_ctime_current(old_inode);
 	mark_inode_dirty(old_inode);
 
-	err = ext2_delete_entry(old_de, old_page);
+	err = ext2_delete_entry(old_de, old_folio);
 	if (!err && dir_de) {
 		if (old_dir != new_dir)
-			err = ext2_set_link(old_inode, dir_de, dir_page,
+			err = ext2_set_link(old_inode, dir_de, dir_folio,
 					    new_dir, false);
 
 		inode_dec_link_count(old_dir);
 	}
 out_dir:
 	if (dir_de)
-		ext2_put_page(dir_page, dir_de);
+		folio_release_kmap(dir_folio, dir_de);
 out_old:
-	ext2_put_page(old_page, old_de);
+	folio_release_kmap(old_folio, old_de);
 	return err;
 }
 
-- 
2.40.1

Re: [PATCH 08/10] ext2: Convert ext2_unlink() and ext2_rename() to use folios

[Jan Kara]

On Thu 21-09-23 21:07:45, Matthew Wilcox (Oracle) wrote:
> This involves changing ext2_find_entry(), ext2_dotdot(),
> ext2_inode_by_name(), ext2_set_link() and ext2_delete_entry() to
> take a folio.  These were also the last users of ext2_get_page() and
> ext2_put_page(), so remove those at the same time.

BTW, missing signed-off-by here. Can I add it?

								Honza

> ---
>  fs/ext2/dir.c   | 75 ++++++++++++++++++++-----------------------------
>  fs/ext2/ext2.h  | 23 ++++++---------
>  fs/ext2/namei.c | 32 ++++++++++-----------
>  3 files changed, 55 insertions(+), 75 deletions(-)
> 
> diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
> index 7e75cfaa709c..dad71ef38395 100644
> --- a/fs/ext2/dir.c
> +++ b/fs/ext2/dir.c
> @@ -209,17 +209,6 @@ static void *ext2_get_folio(struct inode *dir, unsigned long n,
>  	return ERR_PTR(-EIO);
>  }
>  
> -static void *ext2_get_page(struct inode *dir, unsigned long n,
> -				   int quiet, struct page **pagep)
> -{
> -	struct folio *folio;
> -	void *kaddr = ext2_get_folio(dir, n, quiet, &folio);
> -
> -	if (!IS_ERR(kaddr))
> -		*pagep = &folio->page;
> -	return kaddr;
> -}
> -
>  /*
>   * NOTE! unlike strncmp, ext2_match returns 1 for success, 0 for failure.
>   *
> @@ -342,38 +331,35 @@ ext2_readdir(struct file *file, struct dir_context *ctx)
>   * and the entry itself. Page is returned mapped and unlocked.
>   * Entry is guaranteed to be valid.
>   *
> - * On Success ext2_put_page() should be called on *res_page.
> + * On Success folio_release_kmap() should be called on *foliop.
>   *
> - * NOTE: Calls to ext2_get_page()/ext2_put_page() must be nested according to
> - * the rules documented in kmap_local_page()/kunmap_local().
> + * NOTE: Calls to ext2_get_folio()/folio_release_kmap() must be nested
> + * according to the rules documented in kmap_local_folio()/kunmap_local().
>   *
> - * ext2_find_entry() and ext2_dotdot() act as a call to ext2_get_page() and
> - * should be treated as a call to ext2_get_page() for nesting purposes.
> + * ext2_find_entry() and ext2_dotdot() act as a call to ext2_get_folio()
> + * and should be treated as a call to ext2_get_folio() for nesting
> + * purposes.
>   */
>  struct ext2_dir_entry_2 *ext2_find_entry (struct inode *dir,
> -			const struct qstr *child, struct page **res_page)
> +			const struct qstr *child, struct folio **foliop)
>  {
>  	const char *name = child->name;
>  	int namelen = child->len;
>  	unsigned reclen = EXT2_DIR_REC_LEN(namelen);
>  	unsigned long start, n;
>  	unsigned long npages = dir_pages(dir);
> -	struct page *page = NULL;
>  	struct ext2_inode_info *ei = EXT2_I(dir);
>  	ext2_dirent * de;
>  
>  	if (npages == 0)
>  		goto out;
>  
> -	/* OFFSET_CACHE */
> -	*res_page = NULL;
> -
>  	start = ei->i_dir_start_lookup;
>  	if (start >= npages)
>  		start = 0;
>  	n = start;
>  	do {
> -		char *kaddr = ext2_get_page(dir, n, 0, &page);
> +		char *kaddr = ext2_get_folio(dir, n, 0, foliop);
>  		if (IS_ERR(kaddr))
>  			return ERR_CAST(kaddr);
>  
> @@ -383,18 +369,18 @@ struct ext2_dir_entry_2 *ext2_find_entry (struct inode *dir,
>  			if (de->rec_len == 0) {
>  				ext2_error(dir->i_sb, __func__,
>  					"zero-length directory entry");
> -				ext2_put_page(page, de);
> +				folio_release_kmap(*foliop, de);
>  				goto out;
>  			}
>  			if (ext2_match(namelen, name, de))
>  				goto found;
>  			de = ext2_next_entry(de);
>  		}
> -		ext2_put_page(page, kaddr);
> +		folio_release_kmap(*foliop, kaddr);
>  
>  		if (++n >= npages)
>  			n = 0;
> -		/* next page is past the blocks we've got */
> +		/* next folio is past the blocks we've got */
>  		if (unlikely(n > (dir->i_blocks >> (PAGE_SHIFT - 9)))) {
>  			ext2_error(dir->i_sb, __func__,
>  				"dir %lu size %lld exceeds block count %llu",
> @@ -407,7 +393,6 @@ struct ext2_dir_entry_2 *ext2_find_entry (struct inode *dir,
>  	return ERR_PTR(-ENOENT);
>  
>  found:
> -	*res_page = page;
>  	ei->i_dir_start_lookup = n;
>  	return de;
>  }
> @@ -416,17 +401,18 @@ struct ext2_dir_entry_2 *ext2_find_entry (struct inode *dir,
>   * Return the '..' directory entry and the page in which the entry was found
>   * (as a parameter - p).
>   *
> - * On Success ext2_put_page() should be called on *p.
> + * On Success folio_release_kmap() should be called on *foliop.
>   *
> - * NOTE: Calls to ext2_get_page()/ext2_put_page() must be nested according to
> - * the rules documented in kmap_local_page()/kunmap_local().
> + * NOTE: Calls to ext2_get_folio()/folio_release_kmap() must be nested
> + * according to the rules documented in kmap_local_folio()/kunmap_local().
>   *
> - * ext2_find_entry() and ext2_dotdot() act as a call to ext2_get_page() and
> - * should be treated as a call to ext2_get_page() for nesting purposes.
> + * ext2_find_entry() and ext2_dotdot() act as a call to ext2_get_folio()
> + * and should be treated as a call to ext2_get_folio() for nesting
> + * purposes.
>   */
> -struct ext2_dir_entry_2 *ext2_dotdot(struct inode *dir, struct page **p)
> +struct ext2_dir_entry_2 *ext2_dotdot(struct inode *dir, struct folio **foliop)
>  {
> -	ext2_dirent *de = ext2_get_page(dir, 0, 0, p);
> +	ext2_dirent *de = ext2_get_folio(dir, 0, 0, foliop);
>  
>  	if (!IS_ERR(de))
>  		return ext2_next_entry(de);
> @@ -436,14 +422,14 @@ struct ext2_dir_entry_2 *ext2_dotdot(struct inode *dir, struct page **p)
>  int ext2_inode_by_name(struct inode *dir, const struct qstr *child, ino_t *ino)
>  {
>  	struct ext2_dir_entry_2 *de;
> -	struct page *page;
> -	
> -	de = ext2_find_entry(dir, child, &page);
> +	struct folio *folio;
> +
> +	de = ext2_find_entry(dir, child, &folio);
>  	if (IS_ERR(de))
>  		return PTR_ERR(de);
>  
>  	*ino = le32_to_cpu(de->inode);
> -	ext2_put_page(page, de);
> +	folio_release_kmap(folio, de);
>  	return 0;
>  }
>  
> @@ -464,21 +450,21 @@ static int ext2_handle_dirsync(struct inode *dir)
>  }
>  
>  int ext2_set_link(struct inode *dir, struct ext2_dir_entry_2 *de,
> -		struct page *page, struct inode *inode, bool update_times)
> +		struct folio *folio, struct inode *inode, bool update_times)
>  {
> -	loff_t pos = page_offset(page) + offset_in_page(de);
> +	loff_t pos = folio_pos(folio) + offset_in_folio(folio, de);
>  	unsigned len = ext2_rec_len_from_disk(de->rec_len);
>  	int err;
>  
> -	lock_page(page);
> -	err = ext2_prepare_chunk(page, pos, len);
> +	folio_lock(folio);
> +	err = ext2_prepare_chunk(&folio->page, pos, len);
>  	if (err) {
> -		unlock_page(page);
> +		folio_unlock(folio);
>  		return err;
>  	}
>  	de->inode = cpu_to_le32(inode->i_ino);
>  	ext2_set_de_type(de, inode);
> -	ext2_commit_chunk(page, pos, len);
> +	ext2_commit_chunk(&folio->page, pos, len);
>  	if (update_times)
>  		dir->i_mtime = inode_set_ctime_current(dir);
>  	EXT2_I(dir)->i_flags &= ~EXT2_BTREE_FL;
> @@ -584,9 +570,8 @@ int ext2_add_link (struct dentry *dentry, struct inode *inode)
>   * ext2_delete_entry deletes a directory entry by merging it with the
>   * previous entry. Page is up-to-date.
>   */
> -int ext2_delete_entry(struct ext2_dir_entry_2 *dir, struct page *page)
> +int ext2_delete_entry(struct ext2_dir_entry_2 *dir, struct folio *folio)
>  {
> -	struct folio *folio = page_folio(page);
>  	struct inode *inode = folio->mapping->host;
>  	size_t from, to;
>  	char *kaddr;
> diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
> index 7fdd685c384d..677a9ad45dcb 100644
> --- a/fs/ext2/ext2.h
> +++ b/fs/ext2/ext2.h
> @@ -717,22 +717,17 @@ extern void ext2_init_block_alloc_info(struct inode *);
>  extern void ext2_rsv_window_add(struct super_block *sb, struct ext2_reserve_window_node *rsv);
>  
>  /* dir.c */
> -extern int ext2_add_link (struct dentry *, struct inode *);
> -extern int ext2_inode_by_name(struct inode *dir,
> +int ext2_add_link(struct dentry *, struct inode *);
> +int ext2_inode_by_name(struct inode *dir,
>  			      const struct qstr *child, ino_t *ino);
> -extern int ext2_make_empty(struct inode *, struct inode *);
> -extern struct ext2_dir_entry_2 *ext2_find_entry(struct inode *, const struct qstr *,
> -						struct page **);
> -extern int ext2_delete_entry(struct ext2_dir_entry_2 *dir, struct page *page);
> -extern int ext2_empty_dir (struct inode *);
> -extern struct ext2_dir_entry_2 *ext2_dotdot(struct inode *dir, struct page **p);
> +int ext2_make_empty(struct inode *, struct inode *);
> +struct ext2_dir_entry_2 *ext2_find_entry(struct inode *, const struct qstr *,
> +		struct folio **foliop);
> +int ext2_delete_entry(struct ext2_dir_entry_2 *dir, struct folio *folio);
> +int ext2_empty_dir(struct inode *);
> +struct ext2_dir_entry_2 *ext2_dotdot(struct inode *dir, struct folio **foliop);
>  int ext2_set_link(struct inode *dir, struct ext2_dir_entry_2 *de,
> -		struct page *page, struct inode *inode, bool update_times);
> -static inline void ext2_put_page(struct page *page, void *page_addr)
> -{
> -	kunmap_local(page_addr);
> -	put_page(page);
> -}
> +		struct folio *folio, struct inode *inode, bool update_times);
>  
>  /* ialloc.c */
>  extern struct inode * ext2_new_inode (struct inode *, umode_t, const struct qstr *);
> diff --git a/fs/ext2/namei.c b/fs/ext2/namei.c
> index 059517068adc..65f702b1da5b 100644
> --- a/fs/ext2/namei.c
> +++ b/fs/ext2/namei.c
> @@ -273,21 +273,21 @@ static int ext2_unlink(struct inode *dir, struct dentry *dentry)
>  {
>  	struct inode *inode = d_inode(dentry);
>  	struct ext2_dir_entry_2 *de;
> -	struct page *page;
> +	struct folio *folio;
>  	int err;
>  
>  	err = dquot_initialize(dir);
>  	if (err)
>  		goto out;
>  
> -	de = ext2_find_entry(dir, &dentry->d_name, &page);
> +	de = ext2_find_entry(dir, &dentry->d_name, &folio);
>  	if (IS_ERR(de)) {
>  		err = PTR_ERR(de);
>  		goto out;
>  	}
>  
> -	err = ext2_delete_entry(de, page);
> -	ext2_put_page(page, de);
> +	err = ext2_delete_entry(de, folio);
> +	folio_release_kmap(folio, de);
>  	if (err)
>  		goto out;
>  
> @@ -321,9 +321,9 @@ static int ext2_rename (struct mnt_idmap * idmap,
>  {
>  	struct inode * old_inode = d_inode(old_dentry);
>  	struct inode * new_inode = d_inode(new_dentry);
> -	struct page * dir_page = NULL;
> +	struct folio *dir_folio = NULL;
>  	struct ext2_dir_entry_2 * dir_de = NULL;
> -	struct page * old_page;
> +	struct folio * old_folio;
>  	struct ext2_dir_entry_2 * old_de;
>  	int err;
>  
> @@ -338,19 +338,19 @@ static int ext2_rename (struct mnt_idmap * idmap,
>  	if (err)
>  		return err;
>  
> -	old_de = ext2_find_entry(old_dir, &old_dentry->d_name, &old_page);
> +	old_de = ext2_find_entry(old_dir, &old_dentry->d_name, &old_folio);
>  	if (IS_ERR(old_de))
>  		return PTR_ERR(old_de);
>  
>  	if (S_ISDIR(old_inode->i_mode)) {
>  		err = -EIO;
> -		dir_de = ext2_dotdot(old_inode, &dir_page);
> +		dir_de = ext2_dotdot(old_inode, &dir_folio);
>  		if (!dir_de)
>  			goto out_old;
>  	}
>  
>  	if (new_inode) {
> -		struct page *new_page;
> +		struct folio *new_folio;
>  		struct ext2_dir_entry_2 *new_de;
>  
>  		err = -ENOTEMPTY;
> @@ -358,13 +358,13 @@ static int ext2_rename (struct mnt_idmap * idmap,
>  			goto out_dir;
>  
>  		new_de = ext2_find_entry(new_dir, &new_dentry->d_name,
> -					 &new_page);
> +					 &new_folio);
>  		if (IS_ERR(new_de)) {
>  			err = PTR_ERR(new_de);
>  			goto out_dir;
>  		}
> -		err = ext2_set_link(new_dir, new_de, new_page, old_inode, true);
> -		ext2_put_page(new_page, new_de);
> +		err = ext2_set_link(new_dir, new_de, new_folio, old_inode, true);
> +		folio_release_kmap(new_folio, new_de);
>  		if (err)
>  			goto out_dir;
>  		inode_set_ctime_current(new_inode);
> @@ -386,19 +386,19 @@ static int ext2_rename (struct mnt_idmap * idmap,
>  	inode_set_ctime_current(old_inode);
>  	mark_inode_dirty(old_inode);
>  
> -	err = ext2_delete_entry(old_de, old_page);
> +	err = ext2_delete_entry(old_de, old_folio);
>  	if (!err && dir_de) {
>  		if (old_dir != new_dir)
> -			err = ext2_set_link(old_inode, dir_de, dir_page,
> +			err = ext2_set_link(old_inode, dir_de, dir_folio,
>  					    new_dir, false);
>  
>  		inode_dec_link_count(old_dir);
>  	}
>  out_dir:
>  	if (dir_de)
> -		ext2_put_page(dir_page, dir_de);
> +		folio_release_kmap(dir_folio, dir_de);
>  out_old:
> -	ext2_put_page(old_page, old_de);
> +	folio_release_kmap(old_folio, old_de);
>  	return err;
>  }
>  
> -- 
> 2.40.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

[Matthew Wilcox (Oracle)]

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

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 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

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

[Jan Kara]

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) <willy@infradead.org>

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 <jack@suse.com>
SUSE Labs, CR

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

[Matthew Wilcox]

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) <willy@infradead.org>
> 
> 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/