[PATCH v2 0/5] erofs: get rid of erofs_get_meta_page()

[PATCH v2 0/5] erofs: get rid of erofs_get_meta_page()

[Gao Xiang]

Hi folks,

erofs_get_meta_page() is actually inflexible since it's too
close to the page itself.

In order to prepare for folio and subpage features, introduce
on-stack meta buffer descriptor instead and convert all
erofs_get_meta_page() users to use it.

It can also be used for new potential backends such as fscache or mtd.

Patches are trivial.

Thanks,
Gao Xiang

changes since v1:
 - fix unbalanced put_metabuf in xattrs.

Gao Xiang (5):
  erofs: introduce meta buffer operations
  erofs: use meta buffers for inode operations
  erofs: use meta buffers for super operations
  erofs: use meta buffers for xattr operations
  erofs: use meta buffers for zmap operations

 fs/erofs/data.c     | 102 +++++++++++++++++++++++----------
 fs/erofs/inode.c    |  68 +++++++++++-----------
 fs/erofs/internal.h |  22 +++++++-
 fs/erofs/super.c    | 105 +++++++++-------------------------
 fs/erofs/xattr.c    | 134 +++++++++++++-------------------------------
 fs/erofs/zdata.c    |  23 +++-----
 fs/erofs/zmap.c     |  56 ++++++------------
 7 files changed, 214 insertions(+), 296 deletions(-)

-- 
2.24.4

[PATCH v2 1/5] erofs: introduce meta buffer operations

[Gao Xiang]

In order to support subpage and folio for all uncompressed files,
introduce meta buffer descriptors, which can be effectively stored
on stack, in place of meta page operations.

This converts the uncompressed data path to meta buffers.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/data.c     | 97 +++++++++++++++++++++++++++++++++++----------
 fs/erofs/internal.h | 13 ++++++
 2 files changed, 89 insertions(+), 21 deletions(-)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index 4f98c76ec043..6495e16a50a9 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -22,6 +22,56 @@ struct page *erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr)
 	return page;
 }
 
+void erofs_unmap_metabuf(struct erofs_buf *buf)
+{
+	if (buf->kmap_type == EROFS_KMAP)
+		kunmap(buf->page);
+	else if (buf->kmap_type == EROFS_KMAP_ATOMIC)
+		kunmap_atomic(buf->base);
+	buf->kmap_type = EROFS_NO_KMAP;
+}
+
+void erofs_put_metabuf(struct erofs_buf *buf)
+{
+	if (!buf->page)
+		return;
+	erofs_unmap_metabuf(buf);
+	put_page(buf->page);
+	buf->page = NULL;
+}
+
+void *erofs_read_metabuf(struct erofs_buf *buf, struct super_block *sb,
+			erofs_blk_t blkaddr, enum erofs_kmap_type type)
+{
+	struct address_space *const mapping = sb->s_bdev->bd_inode->i_mapping;
+	erofs_off_t offset = blknr_to_addr(blkaddr);
+	pgoff_t index = offset >> PAGE_SHIFT;
+	struct page *page = buf->page;
+
+	if (!page || page->index != index) {
+		erofs_put_metabuf(buf);
+		page = read_cache_page_gfp(mapping, index,
+				mapping_gfp_constraint(mapping, ~__GFP_FS));
+		if (IS_ERR(page))
+			return page;
+		/* should already be PageUptodate, no need to lock page */
+		buf->page = page;
+	}
+	if (buf->kmap_type == EROFS_NO_KMAP) {
+		if (type == EROFS_KMAP)
+			buf->base = kmap(page);
+		else if (type == EROFS_KMAP_ATOMIC)
+			buf->base = kmap_atomic(page);
+		buf->kmap_type = type;
+	} else if (buf->kmap_type != type) {
+		DBG_BUGON(1);
+		return ERR_PTR(-EFAULT);
+	}
+	if (type == EROFS_NO_KMAP)
+		return NULL;
+	return buf->base + (offset & ~PAGE_MASK);
+}
+
 static int erofs_map_blocks_flatmode(struct inode *inode,
 				     struct erofs_map_blocks *map,
 				     int flags)
@@ -31,7 +81,7 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
 	struct erofs_inode *vi = EROFS_I(inode);
 	bool tailendpacking = (vi->datalayout == EROFS_INODE_FLAT_INLINE);
 
-	nblocks = DIV_ROUND_UP(inode->i_size, PAGE_SIZE);
+	nblocks = DIV_ROUND_UP(inode->i_size, EROFS_BLKSIZ);
 	lastblk = nblocks - tailendpacking;
 
 	/* there is no hole in flatmode */
@@ -72,10 +122,11 @@ static int erofs_map_blocks(struct inode *inode,
 	struct super_block *sb = inode->i_sb;
 	struct erofs_inode *vi = EROFS_I(inode);
 	struct erofs_inode_chunk_index *idx;
-	struct page *page;
+	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
 	u64 chunknr;
 	unsigned int unit;
 	erofs_off_t pos;
+	void *kaddr;
 	int err = 0;
 
 	trace_erofs_map_blocks_enter(inode, map, flags);
@@ -101,9 +152,9 @@ static int erofs_map_blocks(struct inode *inode,
 	pos = ALIGN(iloc(EROFS_SB(sb), vi->nid) + vi->inode_isize +
 		    vi->xattr_isize, unit) + unit * chunknr;
 
-	page = erofs_get_meta_page(inode->i_sb, erofs_blknr(pos));
-	if (IS_ERR(page)) {
-		err = PTR_ERR(page);
+	kaddr = erofs_read_metabuf(&buf, sb, erofs_blknr(pos), EROFS_KMAP);
+	if (IS_ERR(kaddr)) {
+		err = PTR_ERR(kaddr);
 		goto out;
 	}
 	map->m_la = chunknr << vi->chunkbits;
@@ -112,7 +163,7 @@ static int erofs_map_blocks(struct inode *inode,
 
 	/* handle block map */
 	if (!(vi->chunkformat & EROFS_CHUNK_FORMAT_INDEXES)) {
-		__le32 *blkaddr = page_address(page) + erofs_blkoff(pos);
+		__le32 *blkaddr = kaddr + erofs_blkoff(pos);
 
 		if (le32_to_cpu(*blkaddr) == EROFS_NULL_ADDR) {
 			map->m_flags = 0;
@@ -123,7 +174,7 @@ static int erofs_map_blocks(struct inode *inode,
 		goto out_unlock;
 	}
 	/* parse chunk indexes */
-	idx = page_address(page) + erofs_blkoff(pos);
+	idx = kaddr + erofs_blkoff(pos);
 	switch (le32_to_cpu(idx->blkaddr)) {
 	case EROFS_NULL_ADDR:
 		map->m_flags = 0;
@@ -136,8 +187,7 @@ static int erofs_map_blocks(struct inode *inode,
 		break;
 	}
 out_unlock:
-	unlock_page(page);
-	put_page(page);
+	erofs_put_metabuf(&buf);
 out:
 	if (!err)
 		map->m_llen = map->m_plen;
@@ -226,16 +276,16 @@ static int erofs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 	}
 
 	if (map.m_flags & EROFS_MAP_META) {
-		struct page *ipage;
+		void *ptr;
+		struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
 
 		iomap->type = IOMAP_INLINE;
-		ipage = erofs_get_meta_page(inode->i_sb,
-					    erofs_blknr(mdev.m_pa));
-		if (IS_ERR(ipage))
-			return PTR_ERR(ipage);
-		iomap->inline_data = page_address(ipage) +
-					erofs_blkoff(mdev.m_pa);
-		iomap->private = ipage;
+		ptr = erofs_read_metabuf(&buf, inode->i_sb,
+					 erofs_blknr(mdev.m_pa), EROFS_KMAP);
+		if (IS_ERR(ptr))
+			return PTR_ERR(ptr);
+		iomap->inline_data = ptr + erofs_blkoff(mdev.m_pa);
+		iomap->private = buf.base;
 	} else {
 		iomap->type = IOMAP_MAPPED;
 		iomap->addr = mdev.m_pa;
@@ -246,12 +296,17 @@ static int erofs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 static int erofs_iomap_end(struct inode *inode, loff_t pos, loff_t length,
 		ssize_t written, unsigned int flags, struct iomap *iomap)
 {
-	struct page *ipage = iomap->private;
+	void *ptr = iomap->private;
+
+	if (ptr) {
+		struct erofs_buf buf = {
+			.page = kmap_to_page(ptr),
+			.base = ptr,
+			.kmap_type = EROFS_KMAP,
+		};
 
-	if (ipage) {
 		DBG_BUGON(iomap->type != IOMAP_INLINE);
-		unlock_page(ipage);
-		put_page(ipage);
+		erofs_put_metabuf(&buf);
 	} else {
 		DBG_BUGON(iomap->type == IOMAP_INLINE);
 	}
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index fca3747d97be..7053f1c4171d 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -251,6 +251,19 @@ static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp)
 #error erofs cannot be used in this platform
 #endif
 
+enum erofs_kmap_type {
+	EROFS_NO_KMAP,		/* don't map the buffer */
+	EROFS_KMAP,		/* use kmap() to map the buffer */
+	EROFS_KMAP_ATOMIC,	/* use kmap_atomic() to map the buffer */
+};
+
+struct erofs_buf {
+	struct page *page;
+	void *base;
+	enum erofs_kmap_type kmap_type;
+};
+#define __EROFS_BUF_INITIALIZER	((struct erofs_buf){ .page = NULL })
+
 #define ROOT_NID(sb)		((sb)->root_nid)
 
 #define erofs_blknr(addr)       ((addr) / EROFS_BLKSIZ)
-- 
2.24.4

Re: [PATCH v2 1/5] erofs: introduce meta buffer operations

[Liu Bo]

On Sun, Jan 02, 2022 at 12:00:13PM +0800, Gao Xiang wrote:
> In order to support subpage and folio for all uncompressed files,
> introduce meta buffer descriptors, which can be effectively stored
> on stack, in place of meta page operations.
> 
> This converts the uncompressed data path to meta buffers.
>

Reviewed-by: Liu Bo <bo.liu@linux.alibaba.com>

> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
>  fs/erofs/data.c     | 97 +++++++++++++++++++++++++++++++++++----------
>  fs/erofs/internal.h | 13 ++++++
>  2 files changed, 89 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> index 4f98c76ec043..6495e16a50a9 100644
> --- a/fs/erofs/data.c
> +++ b/fs/erofs/data.c
> @@ -22,6 +22,56 @@ struct page *erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr)
>  	return page;
>  }
>  
> +void erofs_unmap_metabuf(struct erofs_buf *buf)
> +{
> +	if (buf->kmap_type == EROFS_KMAP)
> +		kunmap(buf->page);
> +	else if (buf->kmap_type == EROFS_KMAP_ATOMIC)
> +		kunmap_atomic(buf->base);
> +	buf->kmap_type = EROFS_NO_KMAP;
> +}
> +
> +void erofs_put_metabuf(struct erofs_buf *buf)
> +{
> +	if (!buf->page)
> +		return;
> +	erofs_unmap_metabuf(buf);
> +	put_page(buf->page);
> +	buf->page = NULL;
> +}
> +
> +void *erofs_read_metabuf(struct erofs_buf *buf, struct super_block *sb,
> +			erofs_blk_t blkaddr, enum erofs_kmap_type type)
> +{
> +	struct address_space *const mapping = sb->s_bdev->bd_inode->i_mapping;
> +	erofs_off_t offset = blknr_to_addr(blkaddr);
> +	pgoff_t index = offset >> PAGE_SHIFT;
> +	struct page *page = buf->page;
> +
> +	if (!page || page->index != index) {
> +		erofs_put_metabuf(buf);
> +		page = read_cache_page_gfp(mapping, index,
> +				mapping_gfp_constraint(mapping, ~__GFP_FS));
> +		if (IS_ERR(page))
> +			return page;
> +		/* should already be PageUptodate, no need to lock page */
> +		buf->page = page;
> +	}
> +	if (buf->kmap_type == EROFS_NO_KMAP) {
> +		if (type == EROFS_KMAP)
> +			buf->base = kmap(page);
> +		else if (type == EROFS_KMAP_ATOMIC)
> +			buf->base = kmap_atomic(page);
> +		buf->kmap_type = type;
> +	} else if (buf->kmap_type != type) {
> +		DBG_BUGON(1);
> +		return ERR_PTR(-EFAULT);
> +	}
> +	if (type == EROFS_NO_KMAP)
> +		return NULL;
> +	return buf->base + (offset & ~PAGE_MASK);
> +}
> +
>  static int erofs_map_blocks_flatmode(struct inode *inode,
>  				     struct erofs_map_blocks *map,
>  				     int flags)
> @@ -31,7 +81,7 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
>  	struct erofs_inode *vi = EROFS_I(inode);
>  	bool tailendpacking = (vi->datalayout == EROFS_INODE_FLAT_INLINE);
>  
> -	nblocks = DIV_ROUND_UP(inode->i_size, PAGE_SIZE);
> +	nblocks = DIV_ROUND_UP(inode->i_size, EROFS_BLKSIZ);
>  	lastblk = nblocks - tailendpacking;
>  
>  	/* there is no hole in flatmode */
> @@ -72,10 +122,11 @@ static int erofs_map_blocks(struct inode *inode,
>  	struct super_block *sb = inode->i_sb;
>  	struct erofs_inode *vi = EROFS_I(inode);
>  	struct erofs_inode_chunk_index *idx;
> -	struct page *page;
> +	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
>  	u64 chunknr;
>  	unsigned int unit;
>  	erofs_off_t pos;
> +	void *kaddr;
>  	int err = 0;
>  
>  	trace_erofs_map_blocks_enter(inode, map, flags);
> @@ -101,9 +152,9 @@ static int erofs_map_blocks(struct inode *inode,
>  	pos = ALIGN(iloc(EROFS_SB(sb), vi->nid) + vi->inode_isize +
>  		    vi->xattr_isize, unit) + unit * chunknr;
>  
> -	page = erofs_get_meta_page(inode->i_sb, erofs_blknr(pos));
> -	if (IS_ERR(page)) {
> -		err = PTR_ERR(page);
> +	kaddr = erofs_read_metabuf(&buf, sb, erofs_blknr(pos), EROFS_KMAP);
> +	if (IS_ERR(kaddr)) {
> +		err = PTR_ERR(kaddr);
>  		goto out;
>  	}
>  	map->m_la = chunknr << vi->chunkbits;
> @@ -112,7 +163,7 @@ static int erofs_map_blocks(struct inode *inode,
>  
>  	/* handle block map */
>  	if (!(vi->chunkformat & EROFS_CHUNK_FORMAT_INDEXES)) {
> -		__le32 *blkaddr = page_address(page) + erofs_blkoff(pos);
> +		__le32 *blkaddr = kaddr + erofs_blkoff(pos);
>  
>  		if (le32_to_cpu(*blkaddr) == EROFS_NULL_ADDR) {
>  			map->m_flags = 0;
> @@ -123,7 +174,7 @@ static int erofs_map_blocks(struct inode *inode,
>  		goto out_unlock;
>  	}
>  	/* parse chunk indexes */
> -	idx = page_address(page) + erofs_blkoff(pos);
> +	idx = kaddr + erofs_blkoff(pos);
>  	switch (le32_to_cpu(idx->blkaddr)) {
>  	case EROFS_NULL_ADDR:
>  		map->m_flags = 0;
> @@ -136,8 +187,7 @@ static int erofs_map_blocks(struct inode *inode,
>  		break;
>  	}
>  out_unlock:
> -	unlock_page(page);
> -	put_page(page);
> +	erofs_put_metabuf(&buf);
>  out:
>  	if (!err)
>  		map->m_llen = map->m_plen;
> @@ -226,16 +276,16 @@ static int erofs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>  	}
>  
>  	if (map.m_flags & EROFS_MAP_META) {
> -		struct page *ipage;
> +		void *ptr;
> +		struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
>  
>  		iomap->type = IOMAP_INLINE;
> -		ipage = erofs_get_meta_page(inode->i_sb,
> -					    erofs_blknr(mdev.m_pa));
> -		if (IS_ERR(ipage))
> -			return PTR_ERR(ipage);
> -		iomap->inline_data = page_address(ipage) +
> -					erofs_blkoff(mdev.m_pa);
> -		iomap->private = ipage;
> +		ptr = erofs_read_metabuf(&buf, inode->i_sb,
> +					 erofs_blknr(mdev.m_pa), EROFS_KMAP);
> +		if (IS_ERR(ptr))
> +			return PTR_ERR(ptr);
> +		iomap->inline_data = ptr + erofs_blkoff(mdev.m_pa);
> +		iomap->private = buf.base;
>  	} else {
>  		iomap->type = IOMAP_MAPPED;
>  		iomap->addr = mdev.m_pa;
> @@ -246,12 +296,17 @@ static int erofs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>  static int erofs_iomap_end(struct inode *inode, loff_t pos, loff_t length,
>  		ssize_t written, unsigned int flags, struct iomap *iomap)
>  {
> -	struct page *ipage = iomap->private;
> +	void *ptr = iomap->private;
> +
> +	if (ptr) {
> +		struct erofs_buf buf = {
> +			.page = kmap_to_page(ptr),
> +			.base = ptr,
> +			.kmap_type = EROFS_KMAP,
> +		};
>  
> -	if (ipage) {
>  		DBG_BUGON(iomap->type != IOMAP_INLINE);
> -		unlock_page(ipage);
> -		put_page(ipage);
> +		erofs_put_metabuf(&buf);
>  	} else {
>  		DBG_BUGON(iomap->type == IOMAP_INLINE);
>  	}
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index fca3747d97be..7053f1c4171d 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -251,6 +251,19 @@ static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp)
>  #error erofs cannot be used in this platform
>  #endif
>  
> +enum erofs_kmap_type {
> +	EROFS_NO_KMAP,		/* don't map the buffer */
> +	EROFS_KMAP,		/* use kmap() to map the buffer */
> +	EROFS_KMAP_ATOMIC,	/* use kmap_atomic() to map the buffer */
> +};
> +
> +struct erofs_buf {
> +	struct page *page;
> +	void *base;
> +	enum erofs_kmap_type kmap_type;
> +};
> +#define __EROFS_BUF_INITIALIZER	((struct erofs_buf){ .page = NULL })
> +
>  #define ROOT_NID(sb)		((sb)->root_nid)
>  
>  #define erofs_blknr(addr)       ((addr) / EROFS_BLKSIZ)
> -- 
> 2.24.4

Re: [PATCH v2 1/5] erofs: introduce meta buffer operations

[Chao Yu]

On 2022/1/2 12:00, Gao Xiang wrote:
> In order to support subpage and folio for all uncompressed files,
> introduce meta buffer descriptors, which can be effectively stored
> on stack, in place of meta page operations.
> 
> This converts the uncompressed data path to meta buffers.
> 
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
>   fs/erofs/data.c     | 97 +++++++++++++++++++++++++++++++++++----------
>   fs/erofs/internal.h | 13 ++++++
>   2 files changed, 89 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> index 4f98c76ec043..6495e16a50a9 100644
> --- a/fs/erofs/data.c
> +++ b/fs/erofs/data.c
> @@ -22,6 +22,56 @@ struct page *erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr)
>   	return page;
>   }
>   
> +void erofs_unmap_metabuf(struct erofs_buf *buf)
> +{
> +	if (buf->kmap_type == EROFS_KMAP)
> +		kunmap(buf->page);
> +	else if (buf->kmap_type == EROFS_KMAP_ATOMIC)
> +		kunmap_atomic(buf->base);

Once user calls this function, .base should be invalidated.

buf->base = NULL;

Otherwise, it looks good to me.

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

> +	buf->kmap_type = EROFS_NO_KMAP;
> +}
> +
> +void erofs_put_metabuf(struct erofs_buf *buf)
> +{
> +	if (!buf->page)
> +		return;
> +	erofs_unmap_metabuf(buf);
> +	put_page(buf->page);
> +	buf->page = NULL;
> +}
> +
> +void *erofs_read_metabuf(struct erofs_buf *buf, struct super_block *sb,
> +			erofs_blk_t blkaddr, enum erofs_kmap_type type)
> +{
> +	struct address_space *const mapping = sb->s_bdev->bd_inode->i_mapping;
> +	erofs_off_t offset = blknr_to_addr(blkaddr);
> +	pgoff_t index = offset >> PAGE_SHIFT;
> +	struct page *page = buf->page;
> +
> +	if (!page || page->index != index) {
> +		erofs_put_metabuf(buf);
> +		page = read_cache_page_gfp(mapping, index,
> +				mapping_gfp_constraint(mapping, ~__GFP_FS));
> +		if (IS_ERR(page))
> +			return page;
> +		/* should already be PageUptodate, no need to lock page */
> +		buf->page = page;
> +	}
> +	if (buf->kmap_type == EROFS_NO_KMAP) {
> +		if (type == EROFS_KMAP)
> +			buf->base = kmap(page);
> +		else if (type == EROFS_KMAP_ATOMIC)
> +			buf->base = kmap_atomic(page);
> +		buf->kmap_type = type;
> +	} else if (buf->kmap_type != type) {
> +		DBG_BUGON(1);
> +		return ERR_PTR(-EFAULT);
> +	}
> +	if (type == EROFS_NO_KMAP)
> +		return NULL;
> +	return buf->base + (offset & ~PAGE_MASK);
> +}
> +
>   static int erofs_map_blocks_flatmode(struct inode *inode,
>   				     struct erofs_map_blocks *map,
>   				     int flags)
> @@ -31,7 +81,7 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
>   	struct erofs_inode *vi = EROFS_I(inode);
>   	bool tailendpacking = (vi->datalayout == EROFS_INODE_FLAT_INLINE);
>   
> -	nblocks = DIV_ROUND_UP(inode->i_size, PAGE_SIZE);
> +	nblocks = DIV_ROUND_UP(inode->i_size, EROFS_BLKSIZ);
>   	lastblk = nblocks - tailendpacking;
>   
>   	/* there is no hole in flatmode */
> @@ -72,10 +122,11 @@ static int erofs_map_blocks(struct inode *inode,
>   	struct super_block *sb = inode->i_sb;
>   	struct erofs_inode *vi = EROFS_I(inode);
>   	struct erofs_inode_chunk_index *idx;
> -	struct page *page;
> +	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
>   	u64 chunknr;
>   	unsigned int unit;
>   	erofs_off_t pos;
> +	void *kaddr;
>   	int err = 0;
>   
>   	trace_erofs_map_blocks_enter(inode, map, flags);
> @@ -101,9 +152,9 @@ static int erofs_map_blocks(struct inode *inode,
>   	pos = ALIGN(iloc(EROFS_SB(sb), vi->nid) + vi->inode_isize +
>   		    vi->xattr_isize, unit) + unit * chunknr;
>   
> -	page = erofs_get_meta_page(inode->i_sb, erofs_blknr(pos));
> -	if (IS_ERR(page)) {
> -		err = PTR_ERR(page);
> +	kaddr = erofs_read_metabuf(&buf, sb, erofs_blknr(pos), EROFS_KMAP);
> +	if (IS_ERR(kaddr)) {
> +		err = PTR_ERR(kaddr);
>   		goto out;
>   	}
>   	map->m_la = chunknr << vi->chunkbits;
> @@ -112,7 +163,7 @@ static int erofs_map_blocks(struct inode *inode,
>   
>   	/* handle block map */
>   	if (!(vi->chunkformat & EROFS_CHUNK_FORMAT_INDEXES)) {
> -		__le32 *blkaddr = page_address(page) + erofs_blkoff(pos);
> +		__le32 *blkaddr = kaddr + erofs_blkoff(pos);
>   
>   		if (le32_to_cpu(*blkaddr) == EROFS_NULL_ADDR) {
>   			map->m_flags = 0;
> @@ -123,7 +174,7 @@ static int erofs_map_blocks(struct inode *inode,
>   		goto out_unlock;
>   	}
>   	/* parse chunk indexes */
> -	idx = page_address(page) + erofs_blkoff(pos);
> +	idx = kaddr + erofs_blkoff(pos);
>   	switch (le32_to_cpu(idx->blkaddr)) {
>   	case EROFS_NULL_ADDR:
>   		map->m_flags = 0;
> @@ -136,8 +187,7 @@ static int erofs_map_blocks(struct inode *inode,
>   		break;
>   	}
>   out_unlock:
> -	unlock_page(page);
> -	put_page(page);
> +	erofs_put_metabuf(&buf);
>   out:
>   	if (!err)
>   		map->m_llen = map->m_plen;
> @@ -226,16 +276,16 @@ static int erofs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>   	}
>   
>   	if (map.m_flags & EROFS_MAP_META) {
> -		struct page *ipage;
> +		void *ptr;
> +		struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
>   
>   		iomap->type = IOMAP_INLINE;
> -		ipage = erofs_get_meta_page(inode->i_sb,
> -					    erofs_blknr(mdev.m_pa));
> -		if (IS_ERR(ipage))
> -			return PTR_ERR(ipage);
> -		iomap->inline_data = page_address(ipage) +
> -					erofs_blkoff(mdev.m_pa);
> -		iomap->private = ipage;
> +		ptr = erofs_read_metabuf(&buf, inode->i_sb,
> +					 erofs_blknr(mdev.m_pa), EROFS_KMAP);
> +		if (IS_ERR(ptr))
> +			return PTR_ERR(ptr);
> +		iomap->inline_data = ptr + erofs_blkoff(mdev.m_pa);
> +		iomap->private = buf.base;
>   	} else {
>   		iomap->type = IOMAP_MAPPED;
>   		iomap->addr = mdev.m_pa;
> @@ -246,12 +296,17 @@ static int erofs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>   static int erofs_iomap_end(struct inode *inode, loff_t pos, loff_t length,
>   		ssize_t written, unsigned int flags, struct iomap *iomap)
>   {
> -	struct page *ipage = iomap->private;
> +	void *ptr = iomap->private;
> +
> +	if (ptr) {
> +		struct erofs_buf buf = {
> +			.page = kmap_to_page(ptr),
> +			.base = ptr,
> +			.kmap_type = EROFS_KMAP,
> +		};
>   
> -	if (ipage) {
>   		DBG_BUGON(iomap->type != IOMAP_INLINE);
> -		unlock_page(ipage);
> -		put_page(ipage);
> +		erofs_put_metabuf(&buf);
>   	} else {
>   		DBG_BUGON(iomap->type == IOMAP_INLINE);
>   	}
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index fca3747d97be..7053f1c4171d 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -251,6 +251,19 @@ static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp)
>   #error erofs cannot be used in this platform
>   #endif
>   
> +enum erofs_kmap_type {
> +	EROFS_NO_KMAP,		/* don't map the buffer */
> +	EROFS_KMAP,		/* use kmap() to map the buffer */
> +	EROFS_KMAP_ATOMIC,	/* use kmap_atomic() to map the buffer */
> +};
> +
> +struct erofs_buf {
> +	struct page *page;
> +	void *base;
> +	enum erofs_kmap_type kmap_type;
> +};
> +#define __EROFS_BUF_INITIALIZER	((struct erofs_buf){ .page = NULL })
> +
>   #define ROOT_NID(sb)		((sb)->root_nid)
>   
>   #define erofs_blknr(addr)       ((addr) / EROFS_BLKSIZ)

Re: [PATCH v2 1/5] erofs: introduce meta buffer operations

[Gao Xiang]

Hi Chao,

On Tue, Jan 04, 2022 at 03:54:08PM +0800, Chao Yu wrote:
> On 2022/1/2 12:00, Gao Xiang wrote:
> > In order to support subpage and folio for all uncompressed files,
> > introduce meta buffer descriptors, which can be effectively stored
> > on stack, in place of meta page operations.
> > 
> > This converts the uncompressed data path to meta buffers.
> > 
> > Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> > ---
> >   fs/erofs/data.c     | 97 +++++++++++++++++++++++++++++++++++----------
> >   fs/erofs/internal.h | 13 ++++++
> >   2 files changed, 89 insertions(+), 21 deletions(-)
> > 
> > diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> > index 4f98c76ec043..6495e16a50a9 100644
> > --- a/fs/erofs/data.c
> > +++ b/fs/erofs/data.c
> > @@ -22,6 +22,56 @@ struct page *erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr)
> >   	return page;
> >   }
> > +void erofs_unmap_metabuf(struct erofs_buf *buf)
> > +{
> > +	if (buf->kmap_type == EROFS_KMAP)
> > +		kunmap(buf->page);
> > +	else if (buf->kmap_type == EROFS_KMAP_ATOMIC)
> > +		kunmap_atomic(buf->base);
> 
> Once user calls this function, .base should be invalidated.
> 
> buf->base = NULL;

Thanks for your suggestion!

Since buf->kmap_type will be switched to EROFS_NO_KMAP, so buf->base
is completely ignored (IOWs, it's unnecessary to reset buf->base..),
however I'm fine to set buf->base = NULL here either.

Thanks,
Gao Xiang 

> 
> Otherwise, it looks good to me.
> 
> Reviewed-by: Chao Yu <chao@kernel.org>
> 
> Thanks,
> 

[PATCH v2 2/5] erofs: use meta buffers for inode operations

[Gao Xiang]

Get rid of old erofs_get_meta_page() within inode operations by
using on-stack meta buffers in order to prepare subpage and folio
features.

Reviewed-by: Yue Hu <huyue2@yulong.com>
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/inode.c    | 68 +++++++++++++++++++++------------------------
 fs/erofs/internal.h |  3 ++
 2 files changed, 35 insertions(+), 36 deletions(-)

diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
index 2345f1de438e..ff62f84f47d3 100644
--- a/fs/erofs/inode.c
+++ b/fs/erofs/inode.c
@@ -13,8 +13,8 @@
  * the inode payload page if it's an extended inode) in order to fill
  * inline data if possible.
  */
-static struct page *erofs_read_inode(struct inode *inode,
-				     unsigned int *ofs)
+static void *erofs_read_inode(struct erofs_buf *buf,
+			      struct inode *inode, unsigned int *ofs)
 {
 	struct super_block *sb = inode->i_sb;
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
@@ -22,7 +22,7 @@ static struct page *erofs_read_inode(struct inode *inode,
 	const erofs_off_t inode_loc = iloc(sbi, vi->nid);
 
 	erofs_blk_t blkaddr, nblks = 0;
-	struct page *page;
+	void *kaddr;
 	struct erofs_inode_compact *dic;
 	struct erofs_inode_extended *die, *copied = NULL;
 	unsigned int ifmt;
@@ -34,14 +34,14 @@ static struct page *erofs_read_inode(struct inode *inode,
 	erofs_dbg("%s, reading inode nid %llu at %u of blkaddr %u",
 		  __func__, vi->nid, *ofs, blkaddr);
 
-	page = erofs_get_meta_page(sb, blkaddr);
-	if (IS_ERR(page)) {
+	kaddr = erofs_read_metabuf(buf, sb, blkaddr, EROFS_KMAP);
+	if (IS_ERR(kaddr)) {
 		erofs_err(sb, "failed to get inode (nid: %llu) page, err %ld",
-			  vi->nid, PTR_ERR(page));
-		return page;
+			  vi->nid, PTR_ERR(kaddr));
+		return kaddr;
 	}
 
-	dic = page_address(page) + *ofs;
+	dic = kaddr + *ofs;
 	ifmt = le16_to_cpu(dic->i_format);
 
 	if (ifmt & ~EROFS_I_ALL) {
@@ -62,12 +62,12 @@ static struct page *erofs_read_inode(struct inode *inode,
 	switch (erofs_inode_version(ifmt)) {
 	case EROFS_INODE_LAYOUT_EXTENDED:
 		vi->inode_isize = sizeof(struct erofs_inode_extended);
-		/* check if the inode acrosses page boundary */
-		if (*ofs + vi->inode_isize <= PAGE_SIZE) {
+		/* check if the extended inode acrosses block boundary */
+		if (*ofs + vi->inode_isize <= EROFS_BLKSIZ) {
 			*ofs += vi->inode_isize;
 			die = (struct erofs_inode_extended *)dic;
 		} else {
-			const unsigned int gotten = PAGE_SIZE - *ofs;
+			const unsigned int gotten = EROFS_BLKSIZ - *ofs;
 
 			copied = kmalloc(vi->inode_isize, GFP_NOFS);
 			if (!copied) {
@@ -75,18 +75,16 @@ static struct page *erofs_read_inode(struct inode *inode,
 				goto err_out;
 			}
 			memcpy(copied, dic, gotten);
-			unlock_page(page);
-			put_page(page);
-
-			page = erofs_get_meta_page(sb, blkaddr + 1);
-			if (IS_ERR(page)) {
-				erofs_err(sb, "failed to get inode payload page (nid: %llu), err %ld",
-					  vi->nid, PTR_ERR(page));
+			kaddr = erofs_read_metabuf(buf, sb, blkaddr + 1,
+						   EROFS_KMAP);
+			if (IS_ERR(kaddr)) {
+				erofs_err(sb, "failed to get inode payload block (nid: %llu), err %ld",
+					  vi->nid, PTR_ERR(kaddr));
 				kfree(copied);
-				return page;
+				return kaddr;
 			}
 			*ofs = vi->inode_isize - gotten;
-			memcpy((u8 *)copied + gotten, page_address(page), *ofs);
+			memcpy((u8 *)copied + gotten, kaddr, *ofs);
 			die = copied;
 		}
 		vi->xattr_isize = erofs_xattr_ibody_size(die->i_xattr_icount);
@@ -200,7 +198,7 @@ static struct page *erofs_read_inode(struct inode *inode,
 		inode->i_blocks = roundup(inode->i_size, EROFS_BLKSIZ) >> 9;
 	else
 		inode->i_blocks = nblks << LOG_SECTORS_PER_BLOCK;
-	return page;
+	return kaddr;
 
 bogusimode:
 	erofs_err(inode->i_sb, "bogus i_mode (%o) @ nid %llu",
@@ -209,12 +207,11 @@ static struct page *erofs_read_inode(struct inode *inode,
 err_out:
 	DBG_BUGON(1);
 	kfree(copied);
-	unlock_page(page);
-	put_page(page);
+	erofs_put_metabuf(buf);
 	return ERR_PTR(err);
 }
 
-static int erofs_fill_symlink(struct inode *inode, void *data,
+static int erofs_fill_symlink(struct inode *inode, void *kaddr,
 			      unsigned int m_pofs)
 {
 	struct erofs_inode *vi = EROFS_I(inode);
@@ -222,7 +219,7 @@ static int erofs_fill_symlink(struct inode *inode, void *data,
 
 	/* if it cannot be handled with fast symlink scheme */
 	if (vi->datalayout != EROFS_INODE_FLAT_INLINE ||
-	    inode->i_size >= PAGE_SIZE) {
+	    inode->i_size >= EROFS_BLKSIZ) {
 		inode->i_op = &erofs_symlink_iops;
 		return 0;
 	}
@@ -232,8 +229,8 @@ static int erofs_fill_symlink(struct inode *inode, void *data,
 		return -ENOMEM;
 
 	m_pofs += vi->xattr_isize;
-	/* inline symlink data shouldn't cross page boundary as well */
-	if (m_pofs + inode->i_size > PAGE_SIZE) {
+	/* inline symlink data shouldn't cross block boundary */
+	if (m_pofs + inode->i_size > EROFS_BLKSIZ) {
 		kfree(lnk);
 		erofs_err(inode->i_sb,
 			  "inline data cross block boundary @ nid %llu",
@@ -241,8 +238,7 @@ static int erofs_fill_symlink(struct inode *inode, void *data,
 		DBG_BUGON(1);
 		return -EFSCORRUPTED;
 	}
-
-	memcpy(lnk, data + m_pofs, inode->i_size);
+	memcpy(lnk, kaddr + m_pofs, inode->i_size);
 	lnk[inode->i_size] = '\0';
 
 	inode->i_link = lnk;
@@ -253,16 +249,17 @@ static int erofs_fill_symlink(struct inode *inode, void *data,
 static int erofs_fill_inode(struct inode *inode, int isdir)
 {
 	struct erofs_inode *vi = EROFS_I(inode);
-	struct page *page;
+	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
+	void *kaddr;
 	unsigned int ofs;
 	int err = 0;
 
 	trace_erofs_fill_inode(inode, isdir);
 
 	/* read inode base data from disk */
-	page = erofs_read_inode(inode, &ofs);
-	if (IS_ERR(page))
-		return PTR_ERR(page);
+	kaddr = erofs_read_inode(&buf, inode, &ofs);
+	if (IS_ERR(kaddr))
+		return PTR_ERR(kaddr);
 
 	/* setup the new inode */
 	switch (inode->i_mode & S_IFMT) {
@@ -278,7 +275,7 @@ static int erofs_fill_inode(struct inode *inode, int isdir)
 		inode->i_fop = &erofs_dir_fops;
 		break;
 	case S_IFLNK:
-		err = erofs_fill_symlink(inode, page_address(page), ofs);
+		err = erofs_fill_symlink(inode, kaddr, ofs);
 		if (err)
 			goto out_unlock;
 		inode_nohighmem(inode);
@@ -302,8 +299,7 @@ static int erofs_fill_inode(struct inode *inode, int isdir)
 	inode->i_mapping->a_ops = &erofs_raw_access_aops;
 
 out_unlock:
-	unlock_page(page);
-	put_page(page);
+	erofs_put_metabuf(&buf);
 	return err;
 }
 
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 7053f1c4171d..f1e4eb3025f6 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -475,6 +475,9 @@ struct erofs_map_dev {
 /* data.c */
 extern const struct file_operations erofs_file_fops;
 struct page *erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr);
+void erofs_put_metabuf(struct erofs_buf *buf);
+void *erofs_read_metabuf(struct erofs_buf *buf, struct super_block *sb,
+			 erofs_blk_t blkaddr, enum erofs_kmap_type type);
 int erofs_map_dev(struct super_block *sb, struct erofs_map_dev *dev);
 int erofs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 		 u64 start, u64 len);
-- 
2.24.4

Re: [PATCH v2 2/5] erofs: use meta buffers for inode operations

[Liu Bo]

On Sun, Jan 02, 2022 at 12:00:14PM +0800, Gao Xiang wrote:
> Get rid of old erofs_get_meta_page() within inode operations by
> using on-stack meta buffers in order to prepare subpage and folio
> features.
> 

Reviewed-by: Liu Bo <bo.liu@linux.alibaba.com>

> Reviewed-by: Yue Hu <huyue2@yulong.com>
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
>  fs/erofs/inode.c    | 68 +++++++++++++++++++++------------------------
>  fs/erofs/internal.h |  3 ++
>  2 files changed, 35 insertions(+), 36 deletions(-)
> 
> diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
> index 2345f1de438e..ff62f84f47d3 100644
> --- a/fs/erofs/inode.c
> +++ b/fs/erofs/inode.c
> @@ -13,8 +13,8 @@
>   * the inode payload page if it's an extended inode) in order to fill
>   * inline data if possible.
>   */
> -static struct page *erofs_read_inode(struct inode *inode,
> -				     unsigned int *ofs)
> +static void *erofs_read_inode(struct erofs_buf *buf,
> +			      struct inode *inode, unsigned int *ofs)
>  {
>  	struct super_block *sb = inode->i_sb;
>  	struct erofs_sb_info *sbi = EROFS_SB(sb);
> @@ -22,7 +22,7 @@ static struct page *erofs_read_inode(struct inode *inode,
>  	const erofs_off_t inode_loc = iloc(sbi, vi->nid);
>  
>  	erofs_blk_t blkaddr, nblks = 0;
> -	struct page *page;
> +	void *kaddr;
>  	struct erofs_inode_compact *dic;
>  	struct erofs_inode_extended *die, *copied = NULL;
>  	unsigned int ifmt;
> @@ -34,14 +34,14 @@ static struct page *erofs_read_inode(struct inode *inode,
>  	erofs_dbg("%s, reading inode nid %llu at %u of blkaddr %u",
>  		  __func__, vi->nid, *ofs, blkaddr);
>  
> -	page = erofs_get_meta_page(sb, blkaddr);
> -	if (IS_ERR(page)) {
> +	kaddr = erofs_read_metabuf(buf, sb, blkaddr, EROFS_KMAP);
> +	if (IS_ERR(kaddr)) {
>  		erofs_err(sb, "failed to get inode (nid: %llu) page, err %ld",
> -			  vi->nid, PTR_ERR(page));
> -		return page;
> +			  vi->nid, PTR_ERR(kaddr));
> +		return kaddr;
>  	}
>  
> -	dic = page_address(page) + *ofs;
> +	dic = kaddr + *ofs;
>  	ifmt = le16_to_cpu(dic->i_format);
>  
>  	if (ifmt & ~EROFS_I_ALL) {
> @@ -62,12 +62,12 @@ static struct page *erofs_read_inode(struct inode *inode,
>  	switch (erofs_inode_version(ifmt)) {
>  	case EROFS_INODE_LAYOUT_EXTENDED:
>  		vi->inode_isize = sizeof(struct erofs_inode_extended);
> -		/* check if the inode acrosses page boundary */
> -		if (*ofs + vi->inode_isize <= PAGE_SIZE) {
> +		/* check if the extended inode acrosses block boundary */
> +		if (*ofs + vi->inode_isize <= EROFS_BLKSIZ) {
>  			*ofs += vi->inode_isize;
>  			die = (struct erofs_inode_extended *)dic;
>  		} else {
> -			const unsigned int gotten = PAGE_SIZE - *ofs;
> +			const unsigned int gotten = EROFS_BLKSIZ - *ofs;
>  
>  			copied = kmalloc(vi->inode_isize, GFP_NOFS);
>  			if (!copied) {
> @@ -75,18 +75,16 @@ static struct page *erofs_read_inode(struct inode *inode,
>  				goto err_out;
>  			}
>  			memcpy(copied, dic, gotten);
> -			unlock_page(page);
> -			put_page(page);
> -
> -			page = erofs_get_meta_page(sb, blkaddr + 1);
> -			if (IS_ERR(page)) {
> -				erofs_err(sb, "failed to get inode payload page (nid: %llu), err %ld",
> -					  vi->nid, PTR_ERR(page));
> +			kaddr = erofs_read_metabuf(buf, sb, blkaddr + 1,
> +						   EROFS_KMAP);
> +			if (IS_ERR(kaddr)) {
> +				erofs_err(sb, "failed to get inode payload block (nid: %llu), err %ld",
> +					  vi->nid, PTR_ERR(kaddr));
>  				kfree(copied);
> -				return page;
> +				return kaddr;
>  			}
>  			*ofs = vi->inode_isize - gotten;
> -			memcpy((u8 *)copied + gotten, page_address(page), *ofs);
> +			memcpy((u8 *)copied + gotten, kaddr, *ofs);
>  			die = copied;
>  		}
>  		vi->xattr_isize = erofs_xattr_ibody_size(die->i_xattr_icount);
> @@ -200,7 +198,7 @@ static struct page *erofs_read_inode(struct inode *inode,
>  		inode->i_blocks = roundup(inode->i_size, EROFS_BLKSIZ) >> 9;
>  	else
>  		inode->i_blocks = nblks << LOG_SECTORS_PER_BLOCK;
> -	return page;
> +	return kaddr;
>  
>  bogusimode:
>  	erofs_err(inode->i_sb, "bogus i_mode (%o) @ nid %llu",
> @@ -209,12 +207,11 @@ static struct page *erofs_read_inode(struct inode *inode,
>  err_out:
>  	DBG_BUGON(1);
>  	kfree(copied);
> -	unlock_page(page);
> -	put_page(page);
> +	erofs_put_metabuf(buf);
>  	return ERR_PTR(err);
>  }
>  
> -static int erofs_fill_symlink(struct inode *inode, void *data,
> +static int erofs_fill_symlink(struct inode *inode, void *kaddr,
>  			      unsigned int m_pofs)
>  {
>  	struct erofs_inode *vi = EROFS_I(inode);
> @@ -222,7 +219,7 @@ static int erofs_fill_symlink(struct inode *inode, void *data,
>  
>  	/* if it cannot be handled with fast symlink scheme */
>  	if (vi->datalayout != EROFS_INODE_FLAT_INLINE ||
> -	    inode->i_size >= PAGE_SIZE) {
> +	    inode->i_size >= EROFS_BLKSIZ) {
>  		inode->i_op = &erofs_symlink_iops;
>  		return 0;
>  	}
> @@ -232,8 +229,8 @@ static int erofs_fill_symlink(struct inode *inode, void *data,
>  		return -ENOMEM;
>  
>  	m_pofs += vi->xattr_isize;
> -	/* inline symlink data shouldn't cross page boundary as well */
> -	if (m_pofs + inode->i_size > PAGE_SIZE) {
> +	/* inline symlink data shouldn't cross block boundary */
> +	if (m_pofs + inode->i_size > EROFS_BLKSIZ) {
>  		kfree(lnk);
>  		erofs_err(inode->i_sb,
>  			  "inline data cross block boundary @ nid %llu",
> @@ -241,8 +238,7 @@ static int erofs_fill_symlink(struct inode *inode, void *data,
>  		DBG_BUGON(1);
>  		return -EFSCORRUPTED;
>  	}
> -
> -	memcpy(lnk, data + m_pofs, inode->i_size);
> +	memcpy(lnk, kaddr + m_pofs, inode->i_size);
>  	lnk[inode->i_size] = '\0';
>  
>  	inode->i_link = lnk;
> @@ -253,16 +249,17 @@ static int erofs_fill_symlink(struct inode *inode, void *data,
>  static int erofs_fill_inode(struct inode *inode, int isdir)
>  {
>  	struct erofs_inode *vi = EROFS_I(inode);
> -	struct page *page;
> +	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
> +	void *kaddr;
>  	unsigned int ofs;
>  	int err = 0;
>  
>  	trace_erofs_fill_inode(inode, isdir);
>  
>  	/* read inode base data from disk */
> -	page = erofs_read_inode(inode, &ofs);
> -	if (IS_ERR(page))
> -		return PTR_ERR(page);
> +	kaddr = erofs_read_inode(&buf, inode, &ofs);
> +	if (IS_ERR(kaddr))
> +		return PTR_ERR(kaddr);
>  
>  	/* setup the new inode */
>  	switch (inode->i_mode & S_IFMT) {
> @@ -278,7 +275,7 @@ static int erofs_fill_inode(struct inode *inode, int isdir)
>  		inode->i_fop = &erofs_dir_fops;
>  		break;
>  	case S_IFLNK:
> -		err = erofs_fill_symlink(inode, page_address(page), ofs);
> +		err = erofs_fill_symlink(inode, kaddr, ofs);
>  		if (err)
>  			goto out_unlock;
>  		inode_nohighmem(inode);
> @@ -302,8 +299,7 @@ static int erofs_fill_inode(struct inode *inode, int isdir)
>  	inode->i_mapping->a_ops = &erofs_raw_access_aops;
>  
>  out_unlock:
> -	unlock_page(page);
> -	put_page(page);
> +	erofs_put_metabuf(&buf);
>  	return err;
>  }
>  
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 7053f1c4171d..f1e4eb3025f6 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -475,6 +475,9 @@ struct erofs_map_dev {
>  /* data.c */
>  extern const struct file_operations erofs_file_fops;
>  struct page *erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr);
> +void erofs_put_metabuf(struct erofs_buf *buf);
> +void *erofs_read_metabuf(struct erofs_buf *buf, struct super_block *sb,
> +			 erofs_blk_t blkaddr, enum erofs_kmap_type type);
>  int erofs_map_dev(struct super_block *sb, struct erofs_map_dev *dev);
>  int erofs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  		 u64 start, u64 len);
> -- 
> 2.24.4

Re: [PATCH v2 2/5] erofs: use meta buffers for inode operations

[Chao Yu]

On 2022/1/2 12:00, Gao Xiang wrote:
> Get rid of old erofs_get_meta_page() within inode operations by
> using on-stack meta buffers in order to prepare subpage and folio
> features.
> 
> Reviewed-by: Yue Hu <huyue2@yulong.com>
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

[PATCH v2 3/5] erofs: use meta buffers for super operations

[Gao Xiang]

Get rid of old erofs_get_meta_page() within super operations by
using on-stack meta buffers in order to prepare subpage and folio
features.

Reviewed-by: Yue Hu <huyue2@yulong.com>
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/super.c | 105 ++++++++++++-----------------------------------
 1 file changed, 26 insertions(+), 79 deletions(-)

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 0724ad5fd6cf..38305fa2969b 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (C) 2017-2018 HUAWEI, Inc.
  *             https://www.huawei.com/
+ * Copyright (C) 2021, Alibaba Cloud
  */
 #include <linux/module.h>
 #include <linux/buffer_head.h>
@@ -124,80 +125,48 @@ static bool check_layout_compatibility(struct super_block *sb,
 
 #ifdef CONFIG_EROFS_FS_ZIP
 /* read variable-sized metadata, offset will be aligned by 4-byte */
-static void *erofs_read_metadata(struct super_block *sb, struct page **pagep,
+static void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf,
 				 erofs_off_t *offset, int *lengthp)
 {
-	struct page *page = *pagep;
 	u8 *buffer, *ptr;
 	int len, i, cnt;
-	erofs_blk_t blk;
 
 	*offset = round_up(*offset, 4);
-	blk = erofs_blknr(*offset);
+	ptr = erofs_read_metabuf(buf, sb, erofs_blknr(*offset), EROFS_KMAP);
+	if (IS_ERR(ptr))
+		return ptr;
 
-	if (!page || page->index != blk) {
-		if (page) {
-			unlock_page(page);
-			put_page(page);
-		}
-		page = erofs_get_meta_page(sb, blk);
-		if (IS_ERR(page))
-			goto err_nullpage;
-	}
-
-	ptr = kmap(page);
 	len = le16_to_cpu(*(__le16 *)&ptr[erofs_blkoff(*offset)]);
 	if (!len)
 		len = U16_MAX + 1;
 	buffer = kmalloc(len, GFP_KERNEL);
-	if (!buffer) {
-		buffer = ERR_PTR(-ENOMEM);
-		goto out;
-	}
+	if (!buffer)
+		return ERR_PTR(-ENOMEM);
 	*offset += sizeof(__le16);
 	*lengthp = len;
 
 	for (i = 0; i < len; i += cnt) {
 		cnt = min(EROFS_BLKSIZ - (int)erofs_blkoff(*offset), len - i);
-		blk = erofs_blknr(*offset);
-
-		if (!page || page->index != blk) {
-			if (page) {
-				kunmap(page);
-				unlock_page(page);
-				put_page(page);
-			}
-			page = erofs_get_meta_page(sb, blk);
-			if (IS_ERR(page)) {
-				kfree(buffer);
-				goto err_nullpage;
-			}
-			ptr = kmap(page);
-		}
+		ptr = erofs_read_metabuf(buf, sb, erofs_blknr(*offset),
+					 EROFS_KMAP);
+		if (IS_ERR(ptr))
+			return ptr;
 		memcpy(buffer + i, ptr + erofs_blkoff(*offset), cnt);
 		*offset += cnt;
 	}
-out:
-	kunmap(page);
-	*pagep = page;
 	return buffer;
-err_nullpage:
-	*pagep = NULL;
-	return page;
 }
 
 static int erofs_load_compr_cfgs(struct super_block *sb,
 				 struct erofs_super_block *dsb)
 {
-	struct erofs_sb_info *sbi;
-	struct page *page;
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
 	unsigned int algs, alg;
 	erofs_off_t offset;
-	int size, ret;
+	int size, ret = 0;
 
-	sbi = EROFS_SB(sb);
 	sbi->available_compr_algs = le16_to_cpu(dsb->u1.available_compr_algs);
-
 	if (sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS) {
 		erofs_err(sb, "try to load compressed fs with unsupported algorithms %x",
 			  sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS);
@@ -205,21 +174,16 @@ static int erofs_load_compr_cfgs(struct super_block *sb,
 	}
 
 	offset = EROFS_SUPER_OFFSET + sbi->sb_size;
-	page = NULL;
 	alg = 0;
-	ret = 0;
-
 	for (algs = sbi->available_compr_algs; algs; algs >>= 1, ++alg) {
 		void *data;
 
 		if (!(algs & 1))
 			continue;
 
-		data = erofs_read_metadata(sb, &page, &offset, &size);
-		if (IS_ERR(data)) {
-			ret = PTR_ERR(data);
-			goto err;
-		}
+		data = erofs_read_metadata(sb, &buf, &offset, &size);
+		if (IS_ERR(data))
+			return PTR_ERR(data);
 
 		switch (alg) {
 		case Z_EROFS_COMPRESSION_LZ4:
@@ -234,13 +198,9 @@ static int erofs_load_compr_cfgs(struct super_block *sb,
 		}
 		kfree(data);
 		if (ret)
-			goto err;
-	}
-err:
-	if (page) {
-		unlock_page(page);
-		put_page(page);
+			break;
 	}
+	erofs_put_metabuf(&buf);
 	return ret;
 }
 #else
@@ -261,7 +221,7 @@ static int erofs_init_devices(struct super_block *sb,
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
 	unsigned int ondisk_extradevs;
 	erofs_off_t pos;
-	struct page *page = NULL;
+	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
 	struct erofs_device_info *dif;
 	struct erofs_deviceslot *dis;
 	void *ptr;
@@ -285,22 +245,13 @@ static int erofs_init_devices(struct super_block *sb,
 	pos = le16_to_cpu(dsb->devt_slotoff) * EROFS_DEVT_SLOT_SIZE;
 	down_read(&sbi->devs->rwsem);
 	idr_for_each_entry(&sbi->devs->tree, dif, id) {
-		erofs_blk_t blk = erofs_blknr(pos);
 		struct block_device *bdev;
 
-		if (!page || page->index != blk) {
-			if (page) {
-				kunmap(page);
-				unlock_page(page);
-				put_page(page);
-			}
-
-			page = erofs_get_meta_page(sb, blk);
-			if (IS_ERR(page)) {
-				up_read(&sbi->devs->rwsem);
-				return PTR_ERR(page);
-			}
-			ptr = kmap(page);
+		ptr = erofs_read_metabuf(&buf, sb, erofs_blknr(pos),
+					 EROFS_KMAP);
+		if (IS_ERR(ptr)) {
+			up_read(&sbi->devs->rwsem);
+			return PTR_ERR(ptr);
 		}
 		dis = ptr + erofs_blkoff(pos);
 
@@ -320,11 +271,7 @@ static int erofs_init_devices(struct super_block *sb,
 	}
 err_out:
 	up_read(&sbi->devs->rwsem);
-	if (page) {
-		kunmap(page);
-		unlock_page(page);
-		put_page(page);
-	}
+	erofs_put_metabuf(&buf);
 	return err;
 }
 
-- 
2.24.4

Re: [PATCH v2 3/5] erofs: use meta buffers for super operations

[Liu Bo]

On Sun, Jan 02, 2022 at 12:00:15PM +0800, Gao Xiang wrote:
> Get rid of old erofs_get_meta_page() within super operations by
> using on-stack meta buffers in order to prepare subpage and folio
> features.
> 
> Reviewed-by: Yue Hu <huyue2@yulong.com>
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
>  fs/erofs/super.c | 105 ++++++++++++-----------------------------------
>  1 file changed, 26 insertions(+), 79 deletions(-)
> 
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 0724ad5fd6cf..38305fa2969b 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -2,6 +2,7 @@
>  /*
>   * Copyright (C) 2017-2018 HUAWEI, Inc.
>   *             https://www.huawei.com/
> + * Copyright (C) 2021, Alibaba Cloud
>   */
>  #include <linux/module.h>
>  #include <linux/buffer_head.h>
> @@ -124,80 +125,48 @@ static bool check_layout_compatibility(struct super_block *sb,
>  
>  #ifdef CONFIG_EROFS_FS_ZIP
>  /* read variable-sized metadata, offset will be aligned by 4-byte */
> -static void *erofs_read_metadata(struct super_block *sb, struct page **pagep,
> +static void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf,
>  				 erofs_off_t *offset, int *lengthp)
>  {
> -	struct page *page = *pagep;
>  	u8 *buffer, *ptr;
>  	int len, i, cnt;
> -	erofs_blk_t blk;
>  
>  	*offset = round_up(*offset, 4);
> -	blk = erofs_blknr(*offset);
> +	ptr = erofs_read_metabuf(buf, sb, erofs_blknr(*offset), EROFS_KMAP);
> +	if (IS_ERR(ptr))
> +		return ptr;
>  
> -	if (!page || page->index != blk) {
> -		if (page) {
> -			unlock_page(page);
> -			put_page(page);
> -		}
> -		page = erofs_get_meta_page(sb, blk);
> -		if (IS_ERR(page))
> -			goto err_nullpage;
> -	}
> -
> -	ptr = kmap(page);
>  	len = le16_to_cpu(*(__le16 *)&ptr[erofs_blkoff(*offset)]);
>  	if (!len)
>  		len = U16_MAX + 1;
>  	buffer = kmalloc(len, GFP_KERNEL);
> -	if (!buffer) {
> -		buffer = ERR_PTR(-ENOMEM);
> -		goto out;
> -	}
> +	if (!buffer)
> +		return ERR_PTR(-ENOMEM);
>  	*offset += sizeof(__le16);
>  	*lengthp = len;
>  
>  	for (i = 0; i < len; i += cnt) {
>  		cnt = min(EROFS_BLKSIZ - (int)erofs_blkoff(*offset), len - i);
> -		blk = erofs_blknr(*offset);
> -
> -		if (!page || page->index != blk) {
> -			if (page) {
> -				kunmap(page);
> -				unlock_page(page);
> -				put_page(page);
> -			}
> -			page = erofs_get_meta_page(sb, blk);
> -			if (IS_ERR(page)) {
> -				kfree(buffer);
> -				goto err_nullpage;
> -			}
> -			ptr = kmap(page);
> -		}
> +		ptr = erofs_read_metabuf(buf, sb, erofs_blknr(*offset),
> +					 EROFS_KMAP);
> +		if (IS_ERR(ptr))

kfree(buffer) is missing.

> +			return ptr;
>  		memcpy(buffer + i, ptr + erofs_blkoff(*offset), cnt);
>  		*offset += cnt;
>  	}
> -out:
> -	kunmap(page);
> -	*pagep = page;
>  	return buffer;
> -err_nullpage:
> -	*pagep = NULL;
> -	return page;
>  }
>  
>  static int erofs_load_compr_cfgs(struct super_block *sb,
>  				 struct erofs_super_block *dsb)
>  {
> -	struct erofs_sb_info *sbi;
> -	struct page *page;
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
>  	unsigned int algs, alg;
>  	erofs_off_t offset;
> -	int size, ret;
> +	int size, ret = 0;
>  
> -	sbi = EROFS_SB(sb);
>  	sbi->available_compr_algs = le16_to_cpu(dsb->u1.available_compr_algs);
> -
>  	if (sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS) {
>  		erofs_err(sb, "try to load compressed fs with unsupported algorithms %x",
>  			  sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS);
> @@ -205,21 +174,16 @@ static int erofs_load_compr_cfgs(struct super_block *sb,
>  	}
>  
>  	offset = EROFS_SUPER_OFFSET + sbi->sb_size;
> -	page = NULL;
>  	alg = 0;
> -	ret = 0;
> -
>  	for (algs = sbi->available_compr_algs; algs; algs >>= 1, ++alg) {
>  		void *data;
>  
>  		if (!(algs & 1))
>  			continue;
>  
> -		data = erofs_read_metadata(sb, &page, &offset, &size);
> -		if (IS_ERR(data)) {
> -			ret = PTR_ERR(data);
> -			goto err;
> -		}
> +		data = erofs_read_metadata(sb, &buf, &offset, &size);

if PTR_ERR(data) is -ENOMEM, buf needs to be put according to the
current design.

> +		if (IS_ERR(data))
> +			return PTR_ERR(data);
>  
>  		switch (alg) {
>  		case Z_EROFS_COMPRESSION_LZ4:
> @@ -234,13 +198,9 @@ static int erofs_load_compr_cfgs(struct super_block *sb,
>  		}
>  		kfree(data);
>  		if (ret)
> -			goto err;
> -	}
> -err:
> -	if (page) {
> -		unlock_page(page);
> -		put_page(page);
> +			break;
>  	}
> +	erofs_put_metabuf(&buf);
>  	return ret;
>  }
>  #else
> @@ -261,7 +221,7 @@ static int erofs_init_devices(struct super_block *sb,
>  	struct erofs_sb_info *sbi = EROFS_SB(sb);
>  	unsigned int ondisk_extradevs;
>  	erofs_off_t pos;
> -	struct page *page = NULL;
> +	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
>  	struct erofs_device_info *dif;
>  	struct erofs_deviceslot *dis;
>  	void *ptr;
> @@ -285,22 +245,13 @@ static int erofs_init_devices(struct super_block *sb,
>  	pos = le16_to_cpu(dsb->devt_slotoff) * EROFS_DEVT_SLOT_SIZE;
>  	down_read(&sbi->devs->rwsem);
>  	idr_for_each_entry(&sbi->devs->tree, dif, id) {
> -		erofs_blk_t blk = erofs_blknr(pos);
>  		struct block_device *bdev;
>  
> -		if (!page || page->index != blk) {
> -			if (page) {
> -				kunmap(page);
> -				unlock_page(page);
> -				put_page(page);
> -			}
> -
> -			page = erofs_get_meta_page(sb, blk);
> -			if (IS_ERR(page)) {
> -				up_read(&sbi->devs->rwsem);
> -				return PTR_ERR(page);
> -			}
> -			ptr = kmap(page);
> +		ptr = erofs_read_metabuf(&buf, sb, erofs_blknr(pos),
> +					 EROFS_KMAP);
> +		if (IS_ERR(ptr)) {

Ditto.

Thanks,
liubo

> +			up_read(&sbi->devs->rwsem);
> +			return PTR_ERR(ptr);
>  		}
>  		dis = ptr + erofs_blkoff(pos);
>  
> @@ -320,11 +271,7 @@ static int erofs_init_devices(struct super_block *sb,
>  	}
>  err_out:
>  	up_read(&sbi->devs->rwsem);
> -	if (page) {
> -		kunmap(page);
> -		unlock_page(page);
> -		put_page(page);
> -	}
> +	erofs_put_metabuf(&buf);
>  	return err;
>  }
>  
> -- 
> 2.24.4

Re: [PATCH v2 3/5] erofs: use meta buffers for super operations

[Gao Xiang]

Hi Bo,

On Sun, Jan 02, 2022 at 03:23:21PM +0800, Liu Bo wrote:
> On Sun, Jan 02, 2022 at 12:00:15PM +0800, Gao Xiang wrote:

...

> > -			ptr = kmap(page);
> > -		}
> > +		ptr = erofs_read_metabuf(buf, sb, erofs_blknr(*offset),
> > +					 EROFS_KMAP);
> > +		if (IS_ERR(ptr))
> 
> kfree(buffer) is missing.
> 
> > +			return ptr;
> >  		memcpy(buffer + i, ptr + erofs_blkoff(*offset), cnt);
> >  		*offset += cnt;
> >  	}
> > -out:
> > -	kunmap(page);
> > -	*pagep = page;
> >  	return buffer;
> > -err_nullpage:
> > -	*pagep = NULL;
> > -	return page;
> >  }
> >  
> >  static int erofs_load_compr_cfgs(struct super_block *sb,
> >  				 struct erofs_super_block *dsb)
> >  {
> > -	struct erofs_sb_info *sbi;
> > -	struct page *page;
> > +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> > +	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
> >  	unsigned int algs, alg;
> >  	erofs_off_t offset;
> > -	int size, ret;
> > +	int size, ret = 0;
> >  
> > -	sbi = EROFS_SB(sb);
> >  	sbi->available_compr_algs = le16_to_cpu(dsb->u1.available_compr_algs);
> > -
> >  	if (sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS) {
> >  		erofs_err(sb, "try to load compressed fs with unsupported algorithms %x",
> >  			  sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS);
> > @@ -205,21 +174,16 @@ static int erofs_load_compr_cfgs(struct super_block *sb,
> >  	}
> >  
> >  	offset = EROFS_SUPER_OFFSET + sbi->sb_size;
> > -	page = NULL;
> >  	alg = 0;
> > -	ret = 0;
> > -
> >  	for (algs = sbi->available_compr_algs; algs; algs >>= 1, ++alg) {
> >  		void *data;
> >  
> >  		if (!(algs & 1))
> >  			continue;
> >  
> > -		data = erofs_read_metadata(sb, &page, &offset, &size);
> > -		if (IS_ERR(data)) {
> > -			ret = PTR_ERR(data);
> > -			goto err;
> > -		}
> > +		data = erofs_read_metadata(sb, &buf, &offset, &size);
> 
> if PTR_ERR(data) is -ENOMEM, buf needs to be put according to the
> current design.
> 
> > +		if (IS_ERR(data))
> > +			return PTR_ERR(data);
> >  
> >  		switch (alg) {
> >  		case Z_EROFS_COMPRESSION_LZ4:
> > @@ -234,13 +198,9 @@ static int erofs_load_compr_cfgs(struct super_block *sb,
> >  		}
> >  		kfree(data);
> >  		if (ret)
> > -			goto err;
> > -	}
> > -err:
> > -	if (page) {
> > -		unlock_page(page);
> > -		put_page(page);
> > +			break;
> >  	}
> > +	erofs_put_metabuf(&buf);
> >  	return ret;
> >  }
> >  #else
> > @@ -261,7 +221,7 @@ static int erofs_init_devices(struct super_block *sb,
> >  	struct erofs_sb_info *sbi = EROFS_SB(sb);
> >  	unsigned int ondisk_extradevs;
> >  	erofs_off_t pos;
> > -	struct page *page = NULL;
> > +	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
> >  	struct erofs_device_info *dif;
> >  	struct erofs_deviceslot *dis;
> >  	void *ptr;
> > @@ -285,22 +245,13 @@ static int erofs_init_devices(struct super_block *sb,
> >  	pos = le16_to_cpu(dsb->devt_slotoff) * EROFS_DEVT_SLOT_SIZE;
> >  	down_read(&sbi->devs->rwsem);
> >  	idr_for_each_entry(&sbi->devs->tree, dif, id) {
> > -		erofs_blk_t blk = erofs_blknr(pos);
> >  		struct block_device *bdev;
> >  
> > -		if (!page || page->index != blk) {
> > -			if (page) {
> > -				kunmap(page);
> > -				unlock_page(page);
> > -				put_page(page);
> > -			}
> > -
> > -			page = erofs_get_meta_page(sb, blk);
> > -			if (IS_ERR(page)) {
> > -				up_read(&sbi->devs->rwsem);
> > -				return PTR_ERR(page);
> > -			}
> > -			ptr = kmap(page);
> > +		ptr = erofs_read_metabuf(&buf, sb, erofs_blknr(pos),
> > +					 EROFS_KMAP);
> > +		if (IS_ERR(ptr)) {
> 
> Ditto.
> 

Thanks for catching these! I've fixed these as follows,
please help check out again!

Thanks,
Gao Xiang

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 38305fa2969b..9e076303bb24 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -149,8 +149,10 @@ static void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf,
 		cnt = min(EROFS_BLKSIZ - (int)erofs_blkoff(*offset), len - i);
 		ptr = erofs_read_metabuf(buf, sb, erofs_blknr(*offset),
 					 EROFS_KMAP);
-		if (IS_ERR(ptr))
+		if (IS_ERR(ptr)) {
+			kfree(buffer);
 			return ptr;
+		}
 		memcpy(buffer + i, ptr + erofs_blkoff(*offset), cnt);
 		*offset += cnt;
 	}
@@ -182,8 +184,10 @@ static int erofs_load_compr_cfgs(struct super_block *sb,
 			continue;
 
 		data = erofs_read_metadata(sb, &buf, &offset, &size);
-		if (IS_ERR(data))
-			return PTR_ERR(data);
+		if (IS_ERR(data)) {
+			ret = PTR_ERR(data);
+			break;
+		}
 
 		switch (alg) {
 		case Z_EROFS_COMPRESSION_LZ4:
@@ -250,8 +254,8 @@ static int erofs_init_devices(struct super_block *sb,
 		ptr = erofs_read_metabuf(&buf, sb, erofs_blknr(pos),
 					 EROFS_KMAP);
 		if (IS_ERR(ptr)) {
-			up_read(&sbi->devs->rwsem);
-			return PTR_ERR(ptr);
+			err = PTR_ERR(ptr);
+			break;
 		}
 		dis = ptr + erofs_blkoff(pos);
 
@@ -260,7 +264,7 @@ static int erofs_init_devices(struct super_block *sb,
 					  sb->s_type);
 		if (IS_ERR(bdev)) {
 			err = PTR_ERR(bdev);
-			goto err_out;
+			break;
 		}
 		dif->bdev = bdev;
 		dif->dax_dev = fs_dax_get_by_bdev(bdev);

[PATCH v3 3/5] erofs: use meta buffers for super operations

[Gao Xiang]

Get rid of old erofs_get_meta_page() within super operations by
using on-stack meta buffers in order to prepare subpage and folio
features.

Reviewed-by: Yue Hu <huyue2@yulong.com>
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
changes since v2:
 - add a missing kfree(buffer);
 - forget to put_metabuf in erofs_load_compr_cfgs;
 - fotget to put_metabuf in erofs_init_devices.

 fs/erofs/super.c | 104 ++++++++++++-----------------------------------
 1 file changed, 27 insertions(+), 77 deletions(-)

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 0724ad5fd6cf..5c137647fa8a 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (C) 2017-2018 HUAWEI, Inc.
  *             https://www.huawei.com/
+ * Copyright (C) 2021, Alibaba Cloud
  */
 #include <linux/module.h>
 #include <linux/buffer_head.h>
@@ -124,80 +125,50 @@ static bool check_layout_compatibility(struct super_block *sb,
 
 #ifdef CONFIG_EROFS_FS_ZIP
 /* read variable-sized metadata, offset will be aligned by 4-byte */
-static void *erofs_read_metadata(struct super_block *sb, struct page **pagep,
+static void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf,
 				 erofs_off_t *offset, int *lengthp)
 {
-	struct page *page = *pagep;
 	u8 *buffer, *ptr;
 	int len, i, cnt;
-	erofs_blk_t blk;
 
 	*offset = round_up(*offset, 4);
-	blk = erofs_blknr(*offset);
+	ptr = erofs_read_metabuf(buf, sb, erofs_blknr(*offset), EROFS_KMAP);
+	if (IS_ERR(ptr))
+		return ptr;
 
-	if (!page || page->index != blk) {
-		if (page) {
-			unlock_page(page);
-			put_page(page);
-		}
-		page = erofs_get_meta_page(sb, blk);
-		if (IS_ERR(page))
-			goto err_nullpage;
-	}
-
-	ptr = kmap(page);
 	len = le16_to_cpu(*(__le16 *)&ptr[erofs_blkoff(*offset)]);
 	if (!len)
 		len = U16_MAX + 1;
 	buffer = kmalloc(len, GFP_KERNEL);
-	if (!buffer) {
-		buffer = ERR_PTR(-ENOMEM);
-		goto out;
-	}
+	if (!buffer)
+		return ERR_PTR(-ENOMEM);
 	*offset += sizeof(__le16);
 	*lengthp = len;
 
 	for (i = 0; i < len; i += cnt) {
 		cnt = min(EROFS_BLKSIZ - (int)erofs_blkoff(*offset), len - i);
-		blk = erofs_blknr(*offset);
-
-		if (!page || page->index != blk) {
-			if (page) {
-				kunmap(page);
-				unlock_page(page);
-				put_page(page);
-			}
-			page = erofs_get_meta_page(sb, blk);
-			if (IS_ERR(page)) {
-				kfree(buffer);
-				goto err_nullpage;
-			}
-			ptr = kmap(page);
+		ptr = erofs_read_metabuf(buf, sb, erofs_blknr(*offset),
+					 EROFS_KMAP);
+		if (IS_ERR(ptr)) {
+			kfree(buffer);
+			return ptr;
 		}
 		memcpy(buffer + i, ptr + erofs_blkoff(*offset), cnt);
 		*offset += cnt;
 	}
-out:
-	kunmap(page);
-	*pagep = page;
 	return buffer;
-err_nullpage:
-	*pagep = NULL;
-	return page;
 }
 
 static int erofs_load_compr_cfgs(struct super_block *sb,
 				 struct erofs_super_block *dsb)
 {
-	struct erofs_sb_info *sbi;
-	struct page *page;
+	struct erofs_sb_info *sbi = EROFS_SB(sb);
+	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
 	unsigned int algs, alg;
 	erofs_off_t offset;
-	int size, ret;
+	int size, ret = 0;
 
-	sbi = EROFS_SB(sb);
 	sbi->available_compr_algs = le16_to_cpu(dsb->u1.available_compr_algs);
-
 	if (sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS) {
 		erofs_err(sb, "try to load compressed fs with unsupported algorithms %x",
 			  sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS);
@@ -205,20 +176,17 @@ static int erofs_load_compr_cfgs(struct super_block *sb,
 	}
 
 	offset = EROFS_SUPER_OFFSET + sbi->sb_size;
-	page = NULL;
 	alg = 0;
-	ret = 0;
-
 	for (algs = sbi->available_compr_algs; algs; algs >>= 1, ++alg) {
 		void *data;
 
 		if (!(algs & 1))
 			continue;
 
-		data = erofs_read_metadata(sb, &page, &offset, &size);
+		data = erofs_read_metadata(sb, &buf, &offset, &size);
 		if (IS_ERR(data)) {
 			ret = PTR_ERR(data);
-			goto err;
+			break;
 		}
 
 		switch (alg) {
@@ -234,13 +202,9 @@ static int erofs_load_compr_cfgs(struct super_block *sb,
 		}
 		kfree(data);
 		if (ret)
-			goto err;
-	}
-err:
-	if (page) {
-		unlock_page(page);
-		put_page(page);
+			break;
 	}
+	erofs_put_metabuf(&buf);
 	return ret;
 }
 #else
@@ -261,7 +225,7 @@ static int erofs_init_devices(struct super_block *sb,
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
 	unsigned int ondisk_extradevs;
 	erofs_off_t pos;
-	struct page *page = NULL;
+	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
 	struct erofs_device_info *dif;
 	struct erofs_deviceslot *dis;
 	void *ptr;
@@ -285,22 +249,13 @@ static int erofs_init_devices(struct super_block *sb,
 	pos = le16_to_cpu(dsb->devt_slotoff) * EROFS_DEVT_SLOT_SIZE;
 	down_read(&sbi->devs->rwsem);
 	idr_for_each_entry(&sbi->devs->tree, dif, id) {
-		erofs_blk_t blk = erofs_blknr(pos);
 		struct block_device *bdev;
 
-		if (!page || page->index != blk) {
-			if (page) {
-				kunmap(page);
-				unlock_page(page);
-				put_page(page);
-			}
-
-			page = erofs_get_meta_page(sb, blk);
-			if (IS_ERR(page)) {
-				up_read(&sbi->devs->rwsem);
-				return PTR_ERR(page);
-			}
-			ptr = kmap(page);
+		ptr = erofs_read_metabuf(&buf, sb, erofs_blknr(pos),
+					 EROFS_KMAP);
+		if (IS_ERR(ptr)) {
+			err = PTR_ERR(ptr);
+			break;
 		}
 		dis = ptr + erofs_blkoff(pos);
 
@@ -309,7 +264,7 @@ static int erofs_init_devices(struct super_block *sb,
 					  sb->s_type);
 		if (IS_ERR(bdev)) {
 			err = PTR_ERR(bdev);
-			goto err_out;
+			break;
 		}
 		dif->bdev = bdev;
 		dif->dax_dev = fs_dax_get_by_bdev(bdev);
@@ -318,13 +273,8 @@ static int erofs_init_devices(struct super_block *sb,
 		sbi->total_blocks += dif->blocks;
 		pos += EROFS_DEVT_SLOT_SIZE;
 	}
-err_out:
 	up_read(&sbi->devs->rwsem);
-	if (page) {
-		kunmap(page);
-		unlock_page(page);
-		put_page(page);
-	}
+	erofs_put_metabuf(&buf);
 	return err;
 }
 
-- 
2.24.4

Re: [PATCH v3 3/5] erofs: use meta buffers for super operations

[Liu Bo]

On Sun, Jan 02, 2022 at 04:13:17PM +0800, Gao Xiang wrote:
> Get rid of old erofs_get_meta_page() within super operations by
> using on-stack meta buffers in order to prepare subpage and folio
> features.
>

Reviewed-by: Liu Bo <bo.liu@linux.alibaba.com>


> Reviewed-by: Yue Hu <huyue2@yulong.com>
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
> changes since v2:
>  - add a missing kfree(buffer);
>  - forget to put_metabuf in erofs_load_compr_cfgs;
>  - fotget to put_metabuf in erofs_init_devices.
> 
>  fs/erofs/super.c | 104 ++++++++++++-----------------------------------
>  1 file changed, 27 insertions(+), 77 deletions(-)
> 
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 0724ad5fd6cf..5c137647fa8a 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -2,6 +2,7 @@
>  /*
>   * Copyright (C) 2017-2018 HUAWEI, Inc.
>   *             https://www.huawei.com/
> + * Copyright (C) 2021, Alibaba Cloud
>   */
>  #include <linux/module.h>
>  #include <linux/buffer_head.h>
> @@ -124,80 +125,50 @@ static bool check_layout_compatibility(struct super_block *sb,
>  
>  #ifdef CONFIG_EROFS_FS_ZIP
>  /* read variable-sized metadata, offset will be aligned by 4-byte */
> -static void *erofs_read_metadata(struct super_block *sb, struct page **pagep,
> +static void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf,
>  				 erofs_off_t *offset, int *lengthp)
>  {
> -	struct page *page = *pagep;
>  	u8 *buffer, *ptr;
>  	int len, i, cnt;
> -	erofs_blk_t blk;
>  
>  	*offset = round_up(*offset, 4);
> -	blk = erofs_blknr(*offset);
> +	ptr = erofs_read_metabuf(buf, sb, erofs_blknr(*offset), EROFS_KMAP);
> +	if (IS_ERR(ptr))
> +		return ptr;
>  
> -	if (!page || page->index != blk) {
> -		if (page) {
> -			unlock_page(page);
> -			put_page(page);
> -		}
> -		page = erofs_get_meta_page(sb, blk);
> -		if (IS_ERR(page))
> -			goto err_nullpage;
> -	}
> -
> -	ptr = kmap(page);
>  	len = le16_to_cpu(*(__le16 *)&ptr[erofs_blkoff(*offset)]);
>  	if (!len)
>  		len = U16_MAX + 1;
>  	buffer = kmalloc(len, GFP_KERNEL);
> -	if (!buffer) {
> -		buffer = ERR_PTR(-ENOMEM);
> -		goto out;
> -	}
> +	if (!buffer)
> +		return ERR_PTR(-ENOMEM);
>  	*offset += sizeof(__le16);
>  	*lengthp = len;
>  
>  	for (i = 0; i < len; i += cnt) {
>  		cnt = min(EROFS_BLKSIZ - (int)erofs_blkoff(*offset), len - i);
> -		blk = erofs_blknr(*offset);
> -
> -		if (!page || page->index != blk) {
> -			if (page) {
> -				kunmap(page);
> -				unlock_page(page);
> -				put_page(page);
> -			}
> -			page = erofs_get_meta_page(sb, blk);
> -			if (IS_ERR(page)) {
> -				kfree(buffer);
> -				goto err_nullpage;
> -			}
> -			ptr = kmap(page);
> +		ptr = erofs_read_metabuf(buf, sb, erofs_blknr(*offset),
> +					 EROFS_KMAP);
> +		if (IS_ERR(ptr)) {
> +			kfree(buffer);
> +			return ptr;
>  		}
>  		memcpy(buffer + i, ptr + erofs_blkoff(*offset), cnt);
>  		*offset += cnt;
>  	}
> -out:
> -	kunmap(page);
> -	*pagep = page;
>  	return buffer;
> -err_nullpage:
> -	*pagep = NULL;
> -	return page;
>  }
>  
>  static int erofs_load_compr_cfgs(struct super_block *sb,
>  				 struct erofs_super_block *dsb)
>  {
> -	struct erofs_sb_info *sbi;
> -	struct page *page;
> +	struct erofs_sb_info *sbi = EROFS_SB(sb);
> +	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
>  	unsigned int algs, alg;
>  	erofs_off_t offset;
> -	int size, ret;
> +	int size, ret = 0;
>  
> -	sbi = EROFS_SB(sb);
>  	sbi->available_compr_algs = le16_to_cpu(dsb->u1.available_compr_algs);
> -
>  	if (sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS) {
>  		erofs_err(sb, "try to load compressed fs with unsupported algorithms %x",
>  			  sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS);
> @@ -205,20 +176,17 @@ static int erofs_load_compr_cfgs(struct super_block *sb,
>  	}
>  
>  	offset = EROFS_SUPER_OFFSET + sbi->sb_size;
> -	page = NULL;
>  	alg = 0;
> -	ret = 0;
> -
>  	for (algs = sbi->available_compr_algs; algs; algs >>= 1, ++alg) {
>  		void *data;
>  
>  		if (!(algs & 1))
>  			continue;
>  
> -		data = erofs_read_metadata(sb, &page, &offset, &size);
> +		data = erofs_read_metadata(sb, &buf, &offset, &size);
>  		if (IS_ERR(data)) {
>  			ret = PTR_ERR(data);
> -			goto err;
> +			break;
>  		}
>  
>  		switch (alg) {
> @@ -234,13 +202,9 @@ static int erofs_load_compr_cfgs(struct super_block *sb,
>  		}
>  		kfree(data);
>  		if (ret)
> -			goto err;
> -	}
> -err:
> -	if (page) {
> -		unlock_page(page);
> -		put_page(page);
> +			break;
>  	}
> +	erofs_put_metabuf(&buf);
>  	return ret;
>  }
>  #else
> @@ -261,7 +225,7 @@ static int erofs_init_devices(struct super_block *sb,
>  	struct erofs_sb_info *sbi = EROFS_SB(sb);
>  	unsigned int ondisk_extradevs;
>  	erofs_off_t pos;
> -	struct page *page = NULL;
> +	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
>  	struct erofs_device_info *dif;
>  	struct erofs_deviceslot *dis;
>  	void *ptr;
> @@ -285,22 +249,13 @@ static int erofs_init_devices(struct super_block *sb,
>  	pos = le16_to_cpu(dsb->devt_slotoff) * EROFS_DEVT_SLOT_SIZE;
>  	down_read(&sbi->devs->rwsem);
>  	idr_for_each_entry(&sbi->devs->tree, dif, id) {
> -		erofs_blk_t blk = erofs_blknr(pos);
>  		struct block_device *bdev;
>  
> -		if (!page || page->index != blk) {
> -			if (page) {
> -				kunmap(page);
> -				unlock_page(page);
> -				put_page(page);
> -			}
> -
> -			page = erofs_get_meta_page(sb, blk);
> -			if (IS_ERR(page)) {
> -				up_read(&sbi->devs->rwsem);
> -				return PTR_ERR(page);
> -			}
> -			ptr = kmap(page);
> +		ptr = erofs_read_metabuf(&buf, sb, erofs_blknr(pos),
> +					 EROFS_KMAP);
> +		if (IS_ERR(ptr)) {
> +			err = PTR_ERR(ptr);
> +			break;
>  		}
>  		dis = ptr + erofs_blkoff(pos);
>  
> @@ -309,7 +264,7 @@ static int erofs_init_devices(struct super_block *sb,
>  					  sb->s_type);
>  		if (IS_ERR(bdev)) {
>  			err = PTR_ERR(bdev);
> -			goto err_out;
> +			break;
>  		}
>  		dif->bdev = bdev;
>  		dif->dax_dev = fs_dax_get_by_bdev(bdev);
> @@ -318,13 +273,8 @@ static int erofs_init_devices(struct super_block *sb,
>  		sbi->total_blocks += dif->blocks;
>  		pos += EROFS_DEVT_SLOT_SIZE;
>  	}
> -err_out:
>  	up_read(&sbi->devs->rwsem);
> -	if (page) {
> -		kunmap(page);
> -		unlock_page(page);
> -		put_page(page);
> -	}
> +	erofs_put_metabuf(&buf);
>  	return err;
>  }
>  
> -- 
> 2.24.4

Re: [PATCH v3 3/5] erofs: use meta buffers for super operations

[Chao Yu]

On 2022/1/2 16:13, Gao Xiang wrote:
> Get rid of old erofs_get_meta_page() within super operations by
> using on-stack meta buffers in order to prepare subpage and folio
> features.
> 
> Reviewed-by: Yue Hu <huyue2@yulong.com>
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

[PATCH v2 4/5] erofs: use meta buffers for xattr operations

[Gao Xiang]

Get rid of old erofs_get_meta_page() within xattr operations by
using on-stack meta buffers in order to prepare subpage and folio
features.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/xattr.c | 134 ++++++++++++++---------------------------------
 1 file changed, 40 insertions(+), 94 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 01c581e93c5f..437ff6e7293d 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -8,33 +8,13 @@
 
 struct xattr_iter {
 	struct super_block *sb;
-	struct page *page;
+	struct erofs_buf buf;
 	void *kaddr;
 
 	erofs_blk_t blkaddr;
 	unsigned int ofs;
 };
 
-static inline void xattr_iter_end(struct xattr_iter *it, bool atomic)
-{
-	/* the only user of kunmap() is 'init_inode_xattrs' */
-	if (!atomic)
-		kunmap(it->page);
-	else
-		kunmap_atomic(it->kaddr);
-
-	unlock_page(it->page);
-	put_page(it->page);
-}
-
-static inline void xattr_iter_end_final(struct xattr_iter *it)
-{
-	if (!it->page)
-		return;
-
-	xattr_iter_end(it, true);
-}
-
 static int init_inode_xattrs(struct inode *inode)
 {
 	struct erofs_inode *const vi = EROFS_I(inode);
@@ -43,7 +23,6 @@ static int init_inode_xattrs(struct inode *inode)
 	struct erofs_xattr_ibody_header *ih;
 	struct super_block *sb;
 	struct erofs_sb_info *sbi;
-	bool atomic_map;
 	int ret = 0;
 
 	/* the most case is that xattrs of this inode are initialized. */
@@ -91,26 +70,23 @@ static int init_inode_xattrs(struct inode *inode)
 
 	sb = inode->i_sb;
 	sbi = EROFS_SB(sb);
+	it.buf = __EROFS_BUF_INITIALIZER;
 	it.blkaddr = erofs_blknr(iloc(sbi, vi->nid) + vi->inode_isize);
 	it.ofs = erofs_blkoff(iloc(sbi, vi->nid) + vi->inode_isize);
 
-	it.page = erofs_get_meta_page(sb, it.blkaddr);
-	if (IS_ERR(it.page)) {
-		ret = PTR_ERR(it.page);
+	/* read in shared xattr array (non-atomic, see kmalloc below) */
+	it.kaddr = erofs_read_metabuf(&it.buf, sb, it.blkaddr, EROFS_KMAP);
+	if (IS_ERR(it.kaddr)) {
+		ret = PTR_ERR(it.kaddr);
 		goto out_unlock;
 	}
 
-	/* read in shared xattr array (non-atomic, see kmalloc below) */
-	it.kaddr = kmap(it.page);
-	atomic_map = false;
-
 	ih = (struct erofs_xattr_ibody_header *)(it.kaddr + it.ofs);
-
 	vi->xattr_shared_count = ih->h_shared_count;
 	vi->xattr_shared_xattrs = kmalloc_array(vi->xattr_shared_count,
 						sizeof(uint), GFP_KERNEL);
 	if (!vi->xattr_shared_xattrs) {
-		xattr_iter_end(&it, atomic_map);
+		erofs_put_metabuf(&it.buf);
 		ret = -ENOMEM;
 		goto out_unlock;
 	}
@@ -122,25 +98,22 @@ static int init_inode_xattrs(struct inode *inode)
 		if (it.ofs >= EROFS_BLKSIZ) {
 			/* cannot be unaligned */
 			DBG_BUGON(it.ofs != EROFS_BLKSIZ);
-			xattr_iter_end(&it, atomic_map);
 
-			it.page = erofs_get_meta_page(sb, ++it.blkaddr);
-			if (IS_ERR(it.page)) {
+			it.kaddr = erofs_read_metabuf(&it.buf, sb, ++it.blkaddr,
+						      EROFS_KMAP);
+			if (IS_ERR(it.kaddr)) {
 				kfree(vi->xattr_shared_xattrs);
 				vi->xattr_shared_xattrs = NULL;
-				ret = PTR_ERR(it.page);
+				ret = PTR_ERR(it.kaddr);
 				goto out_unlock;
 			}
-
-			it.kaddr = kmap_atomic(it.page);
-			atomic_map = true;
 			it.ofs = 0;
 		}
 		vi->xattr_shared_xattrs[i] =
 			le32_to_cpu(*(__le32 *)(it.kaddr + it.ofs));
 		it.ofs += sizeof(__le32);
 	}
-	xattr_iter_end(&it, atomic_map);
+	erofs_put_metabuf(&it.buf);
 
 	/* paired with smp_mb() at the beginning of the function. */
 	smp_mb();
@@ -172,19 +145,11 @@ static inline int xattr_iter_fixup(struct xattr_iter *it)
 	if (it->ofs < EROFS_BLKSIZ)
 		return 0;
 
-	xattr_iter_end(it, true);
-
 	it->blkaddr += erofs_blknr(it->ofs);
-
-	it->page = erofs_get_meta_page(it->sb, it->blkaddr);
-	if (IS_ERR(it->page)) {
-		int err = PTR_ERR(it->page);
-
-		it->page = NULL;
-		return err;
-	}
-
-	it->kaddr = kmap_atomic(it->page);
+	it->kaddr = erofs_read_metabuf(&it->buf, it->sb, it->blkaddr,
+				       EROFS_KMAP_ATOMIC);
+	if (IS_ERR(it->kaddr))
+		return PTR_ERR(it->kaddr);
 	it->ofs = erofs_blkoff(it->ofs);
 	return 0;
 }
@@ -207,11 +172,10 @@ static int inline_xattr_iter_begin(struct xattr_iter *it,
 	it->blkaddr = erofs_blknr(iloc(sbi, vi->nid) + inline_xattr_ofs);
 	it->ofs = erofs_blkoff(iloc(sbi, vi->nid) + inline_xattr_ofs);
 
-	it->page = erofs_get_meta_page(inode->i_sb, it->blkaddr);
-	if (IS_ERR(it->page))
-		return PTR_ERR(it->page);
-
-	it->kaddr = kmap_atomic(it->page);
+	it->kaddr = erofs_read_metabuf(&it->buf, inode->i_sb, it->blkaddr,
+				       EROFS_KMAP_ATOMIC);
+	if (IS_ERR(it->kaddr))
+		return PTR_ERR(it->kaddr);
 	return vi->xattr_isize - xattr_header_sz;
 }
 
@@ -272,7 +236,7 @@ static int xattr_foreach(struct xattr_iter *it,
 			it->ofs = 0;
 		}
 
-		slice = min_t(unsigned int, PAGE_SIZE - it->ofs,
+		slice = min_t(unsigned int, EROFS_BLKSIZ - it->ofs,
 			      entry.e_name_len - processed);
 
 		/* handle name */
@@ -307,7 +271,7 @@ static int xattr_foreach(struct xattr_iter *it,
 			it->ofs = 0;
 		}
 
-		slice = min_t(unsigned int, PAGE_SIZE - it->ofs,
+		slice = min_t(unsigned int, EROFS_BLKSIZ - it->ofs,
 			      value_sz - processed);
 		op->value(it, processed, it->kaddr + it->ofs, slice);
 		it->ofs += slice;
@@ -386,8 +350,6 @@ static int inline_getxattr(struct inode *inode, struct getxattr_iter *it)
 		if (ret != -ENOATTR)
 			break;
 	}
-	xattr_iter_end_final(&it->it);
-
 	return ret ? ret : it->buffer_size;
 }
 
@@ -404,26 +366,16 @@ static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
 			xattrblock_addr(sbi, vi->xattr_shared_xattrs[i]);
 
 		it->it.ofs = xattrblock_offset(sbi, vi->xattr_shared_xattrs[i]);
-
-		if (!i || blkaddr != it->it.blkaddr) {
-			if (i)
-				xattr_iter_end(&it->it, true);
-
-			it->it.page = erofs_get_meta_page(sb, blkaddr);
-			if (IS_ERR(it->it.page))
-				return PTR_ERR(it->it.page);
-
-			it->it.kaddr = kmap_atomic(it->it.page);
-			it->it.blkaddr = blkaddr;
-		}
+		it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb, blkaddr,
+						  EROFS_KMAP_ATOMIC);
+		if (IS_ERR(it->it.kaddr))
+			return PTR_ERR(it->it.kaddr);
+		it->it.blkaddr = blkaddr;
 
 		ret = xattr_foreach(&it->it, &find_xattr_handlers, NULL);
 		if (ret != -ENOATTR)
 			break;
 	}
-	if (vi->xattr_shared_count)
-		xattr_iter_end_final(&it->it);
-
 	return ret ? ret : it->buffer_size;
 }
 
@@ -452,10 +404,11 @@ int erofs_getxattr(struct inode *inode, int index,
 		return ret;
 
 	it.index = index;
-
 	it.name.len = strlen(name);
 	if (it.name.len > EROFS_NAME_LEN)
 		return -ERANGE;
+
+	it.it.buf = __EROFS_BUF_INITIALIZER;
 	it.name.name = name;
 
 	it.buffer = buffer;
@@ -465,6 +418,7 @@ int erofs_getxattr(struct inode *inode, int index,
 	ret = inline_getxattr(inode, &it);
 	if (ret == -ENOATTR)
 		ret = shared_getxattr(inode, &it);
+	erofs_put_metabuf(&it.it.buf);
 	return ret;
 }
 
@@ -607,7 +561,6 @@ static int inline_listxattr(struct listxattr_iter *it)
 		if (ret)
 			break;
 	}
-	xattr_iter_end_final(&it->it);
 	return ret ? ret : it->buffer_ofs;
 }
 
@@ -625,25 +578,16 @@ static int shared_listxattr(struct listxattr_iter *it)
 			xattrblock_addr(sbi, vi->xattr_shared_xattrs[i]);
 
 		it->it.ofs = xattrblock_offset(sbi, vi->xattr_shared_xattrs[i]);
-		if (!i || blkaddr != it->it.blkaddr) {
-			if (i)
-				xattr_iter_end(&it->it, true);
-
-			it->it.page = erofs_get_meta_page(sb, blkaddr);
-			if (IS_ERR(it->it.page))
-				return PTR_ERR(it->it.page);
-
-			it->it.kaddr = kmap_atomic(it->it.page);
-			it->it.blkaddr = blkaddr;
-		}
+		it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb, blkaddr,
+						  EROFS_KMAP_ATOMIC);
+		if (IS_ERR(it->it.kaddr))
+			return PTR_ERR(it->it.kaddr);
+		it->it.blkaddr = blkaddr;
 
 		ret = xattr_foreach(&it->it, &list_xattr_handlers, NULL);
 		if (ret)
 			break;
 	}
-	if (vi->xattr_shared_count)
-		xattr_iter_end_final(&it->it);
-
 	return ret ? ret : it->buffer_ofs;
 }
 
@@ -659,6 +603,7 @@ ssize_t erofs_listxattr(struct dentry *dentry,
 	if (ret)
 		return ret;
 
+	it.it.buf = __EROFS_BUF_INITIALIZER;
 	it.dentry = dentry;
 	it.buffer = buffer;
 	it.buffer_size = buffer_size;
@@ -667,9 +612,10 @@ ssize_t erofs_listxattr(struct dentry *dentry,
 	it.it.sb = dentry->d_sb;
 
 	ret = inline_listxattr(&it);
-	if (ret < 0 && ret != -ENOATTR)
-		return ret;
-	return shared_listxattr(&it);
+	if (ret >= 0 || ret == -ENOATTR)
+		ret = shared_listxattr(&it);
+	erofs_put_metabuf(&it.it.buf);
+	return ret;
 }
 
 #ifdef CONFIG_EROFS_FS_POSIX_ACL
-- 
2.24.4

Re: [PATCH v2 4/5] erofs: use meta buffers for xattr operations

[Liu Bo]

On Sun, Jan 02, 2022 at 12:00:16PM +0800, Gao Xiang wrote:
> Get rid of old erofs_get_meta_page() within xattr operations by
> using on-stack meta buffers in order to prepare subpage and folio
> features.
>

Reviewed-by: Liu Bo <bo.liu@linux.alibaba.com>

> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
>  fs/erofs/xattr.c | 134 ++++++++++++++---------------------------------
>  1 file changed, 40 insertions(+), 94 deletions(-)
> 
> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
> index 01c581e93c5f..437ff6e7293d 100644
> --- a/fs/erofs/xattr.c
> +++ b/fs/erofs/xattr.c
> @@ -8,33 +8,13 @@
>  
>  struct xattr_iter {
>  	struct super_block *sb;
> -	struct page *page;
> +	struct erofs_buf buf;
>  	void *kaddr;
>  
>  	erofs_blk_t blkaddr;
>  	unsigned int ofs;
>  };
>  
> -static inline void xattr_iter_end(struct xattr_iter *it, bool atomic)
> -{
> -	/* the only user of kunmap() is 'init_inode_xattrs' */
> -	if (!atomic)
> -		kunmap(it->page);
> -	else
> -		kunmap_atomic(it->kaddr);
> -
> -	unlock_page(it->page);
> -	put_page(it->page);
> -}
> -
> -static inline void xattr_iter_end_final(struct xattr_iter *it)
> -{
> -	if (!it->page)
> -		return;
> -
> -	xattr_iter_end(it, true);
> -}
> -
>  static int init_inode_xattrs(struct inode *inode)
>  {
>  	struct erofs_inode *const vi = EROFS_I(inode);
> @@ -43,7 +23,6 @@ static int init_inode_xattrs(struct inode *inode)
>  	struct erofs_xattr_ibody_header *ih;
>  	struct super_block *sb;
>  	struct erofs_sb_info *sbi;
> -	bool atomic_map;
>  	int ret = 0;
>  
>  	/* the most case is that xattrs of this inode are initialized. */
> @@ -91,26 +70,23 @@ static int init_inode_xattrs(struct inode *inode)
>  
>  	sb = inode->i_sb;
>  	sbi = EROFS_SB(sb);
> +	it.buf = __EROFS_BUF_INITIALIZER;
>  	it.blkaddr = erofs_blknr(iloc(sbi, vi->nid) + vi->inode_isize);
>  	it.ofs = erofs_blkoff(iloc(sbi, vi->nid) + vi->inode_isize);
>  
> -	it.page = erofs_get_meta_page(sb, it.blkaddr);
> -	if (IS_ERR(it.page)) {
> -		ret = PTR_ERR(it.page);
> +	/* read in shared xattr array (non-atomic, see kmalloc below) */
> +	it.kaddr = erofs_read_metabuf(&it.buf, sb, it.blkaddr, EROFS_KMAP);
> +	if (IS_ERR(it.kaddr)) {
> +		ret = PTR_ERR(it.kaddr);
>  		goto out_unlock;
>  	}
>  
> -	/* read in shared xattr array (non-atomic, see kmalloc below) */
> -	it.kaddr = kmap(it.page);
> -	atomic_map = false;
> -
>  	ih = (struct erofs_xattr_ibody_header *)(it.kaddr + it.ofs);
> -
>  	vi->xattr_shared_count = ih->h_shared_count;
>  	vi->xattr_shared_xattrs = kmalloc_array(vi->xattr_shared_count,
>  						sizeof(uint), GFP_KERNEL);
>  	if (!vi->xattr_shared_xattrs) {
> -		xattr_iter_end(&it, atomic_map);
> +		erofs_put_metabuf(&it.buf);
>  		ret = -ENOMEM;
>  		goto out_unlock;
>  	}
> @@ -122,25 +98,22 @@ static int init_inode_xattrs(struct inode *inode)
>  		if (it.ofs >= EROFS_BLKSIZ) {
>  			/* cannot be unaligned */
>  			DBG_BUGON(it.ofs != EROFS_BLKSIZ);
> -			xattr_iter_end(&it, atomic_map);
>  
> -			it.page = erofs_get_meta_page(sb, ++it.blkaddr);
> -			if (IS_ERR(it.page)) {
> +			it.kaddr = erofs_read_metabuf(&it.buf, sb, ++it.blkaddr,
> +						      EROFS_KMAP);
> +			if (IS_ERR(it.kaddr)) {
>  				kfree(vi->xattr_shared_xattrs);
>  				vi->xattr_shared_xattrs = NULL;
> -				ret = PTR_ERR(it.page);
> +				ret = PTR_ERR(it.kaddr);
>  				goto out_unlock;
>  			}
> -
> -			it.kaddr = kmap_atomic(it.page);
> -			atomic_map = true;
>  			it.ofs = 0;
>  		}
>  		vi->xattr_shared_xattrs[i] =
>  			le32_to_cpu(*(__le32 *)(it.kaddr + it.ofs));
>  		it.ofs += sizeof(__le32);
>  	}
> -	xattr_iter_end(&it, atomic_map);
> +	erofs_put_metabuf(&it.buf);
>  
>  	/* paired with smp_mb() at the beginning of the function. */
>  	smp_mb();
> @@ -172,19 +145,11 @@ static inline int xattr_iter_fixup(struct xattr_iter *it)
>  	if (it->ofs < EROFS_BLKSIZ)
>  		return 0;
>  
> -	xattr_iter_end(it, true);
> -
>  	it->blkaddr += erofs_blknr(it->ofs);
> -
> -	it->page = erofs_get_meta_page(it->sb, it->blkaddr);
> -	if (IS_ERR(it->page)) {
> -		int err = PTR_ERR(it->page);
> -
> -		it->page = NULL;
> -		return err;
> -	}
> -
> -	it->kaddr = kmap_atomic(it->page);
> +	it->kaddr = erofs_read_metabuf(&it->buf, it->sb, it->blkaddr,
> +				       EROFS_KMAP_ATOMIC);
> +	if (IS_ERR(it->kaddr))
> +		return PTR_ERR(it->kaddr);
>  	it->ofs = erofs_blkoff(it->ofs);
>  	return 0;
>  }
> @@ -207,11 +172,10 @@ static int inline_xattr_iter_begin(struct xattr_iter *it,
>  	it->blkaddr = erofs_blknr(iloc(sbi, vi->nid) + inline_xattr_ofs);
>  	it->ofs = erofs_blkoff(iloc(sbi, vi->nid) + inline_xattr_ofs);
>  
> -	it->page = erofs_get_meta_page(inode->i_sb, it->blkaddr);
> -	if (IS_ERR(it->page))
> -		return PTR_ERR(it->page);
> -
> -	it->kaddr = kmap_atomic(it->page);
> +	it->kaddr = erofs_read_metabuf(&it->buf, inode->i_sb, it->blkaddr,
> +				       EROFS_KMAP_ATOMIC);
> +	if (IS_ERR(it->kaddr))
> +		return PTR_ERR(it->kaddr);
>  	return vi->xattr_isize - xattr_header_sz;
>  }
>  
> @@ -272,7 +236,7 @@ static int xattr_foreach(struct xattr_iter *it,
>  			it->ofs = 0;
>  		}
>  
> -		slice = min_t(unsigned int, PAGE_SIZE - it->ofs,
> +		slice = min_t(unsigned int, EROFS_BLKSIZ - it->ofs,
>  			      entry.e_name_len - processed);
>  
>  		/* handle name */
> @@ -307,7 +271,7 @@ static int xattr_foreach(struct xattr_iter *it,
>  			it->ofs = 0;
>  		}
>  
> -		slice = min_t(unsigned int, PAGE_SIZE - it->ofs,
> +		slice = min_t(unsigned int, EROFS_BLKSIZ - it->ofs,
>  			      value_sz - processed);
>  		op->value(it, processed, it->kaddr + it->ofs, slice);
>  		it->ofs += slice;
> @@ -386,8 +350,6 @@ static int inline_getxattr(struct inode *inode, struct getxattr_iter *it)
>  		if (ret != -ENOATTR)
>  			break;
>  	}
> -	xattr_iter_end_final(&it->it);
> -
>  	return ret ? ret : it->buffer_size;
>  }
>  
> @@ -404,26 +366,16 @@ static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
>  			xattrblock_addr(sbi, vi->xattr_shared_xattrs[i]);
>  
>  		it->it.ofs = xattrblock_offset(sbi, vi->xattr_shared_xattrs[i]);
> -
> -		if (!i || blkaddr != it->it.blkaddr) {
> -			if (i)
> -				xattr_iter_end(&it->it, true);
> -
> -			it->it.page = erofs_get_meta_page(sb, blkaddr);
> -			if (IS_ERR(it->it.page))
> -				return PTR_ERR(it->it.page);
> -
> -			it->it.kaddr = kmap_atomic(it->it.page);
> -			it->it.blkaddr = blkaddr;
> -		}
> +		it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb, blkaddr,
> +						  EROFS_KMAP_ATOMIC);
> +		if (IS_ERR(it->it.kaddr))
> +			return PTR_ERR(it->it.kaddr);
> +		it->it.blkaddr = blkaddr;
>  
>  		ret = xattr_foreach(&it->it, &find_xattr_handlers, NULL);
>  		if (ret != -ENOATTR)
>  			break;
>  	}
> -	if (vi->xattr_shared_count)
> -		xattr_iter_end_final(&it->it);
> -
>  	return ret ? ret : it->buffer_size;
>  }
>  
> @@ -452,10 +404,11 @@ int erofs_getxattr(struct inode *inode, int index,
>  		return ret;
>  
>  	it.index = index;
> -
>  	it.name.len = strlen(name);
>  	if (it.name.len > EROFS_NAME_LEN)
>  		return -ERANGE;
> +
> +	it.it.buf = __EROFS_BUF_INITIALIZER;
>  	it.name.name = name;
>  
>  	it.buffer = buffer;
> @@ -465,6 +418,7 @@ int erofs_getxattr(struct inode *inode, int index,
>  	ret = inline_getxattr(inode, &it);
>  	if (ret == -ENOATTR)
>  		ret = shared_getxattr(inode, &it);
> +	erofs_put_metabuf(&it.it.buf);
>  	return ret;
>  }
>  
> @@ -607,7 +561,6 @@ static int inline_listxattr(struct listxattr_iter *it)
>  		if (ret)
>  			break;
>  	}
> -	xattr_iter_end_final(&it->it);
>  	return ret ? ret : it->buffer_ofs;
>  }
>  
> @@ -625,25 +578,16 @@ static int shared_listxattr(struct listxattr_iter *it)
>  			xattrblock_addr(sbi, vi->xattr_shared_xattrs[i]);
>  
>  		it->it.ofs = xattrblock_offset(sbi, vi->xattr_shared_xattrs[i]);
> -		if (!i || blkaddr != it->it.blkaddr) {
> -			if (i)
> -				xattr_iter_end(&it->it, true);
> -
> -			it->it.page = erofs_get_meta_page(sb, blkaddr);
> -			if (IS_ERR(it->it.page))
> -				return PTR_ERR(it->it.page);
> -
> -			it->it.kaddr = kmap_atomic(it->it.page);
> -			it->it.blkaddr = blkaddr;
> -		}
> +		it->it.kaddr = erofs_read_metabuf(&it->it.buf, sb, blkaddr,
> +						  EROFS_KMAP_ATOMIC);
> +		if (IS_ERR(it->it.kaddr))
> +			return PTR_ERR(it->it.kaddr);
> +		it->it.blkaddr = blkaddr;
>  
>  		ret = xattr_foreach(&it->it, &list_xattr_handlers, NULL);
>  		if (ret)
>  			break;
>  	}
> -	if (vi->xattr_shared_count)
> -		xattr_iter_end_final(&it->it);
> -
>  	return ret ? ret : it->buffer_ofs;
>  }
>  
> @@ -659,6 +603,7 @@ ssize_t erofs_listxattr(struct dentry *dentry,
>  	if (ret)
>  		return ret;
>  
> +	it.it.buf = __EROFS_BUF_INITIALIZER;
>  	it.dentry = dentry;
>  	it.buffer = buffer;
>  	it.buffer_size = buffer_size;
> @@ -667,9 +612,10 @@ ssize_t erofs_listxattr(struct dentry *dentry,
>  	it.it.sb = dentry->d_sb;
>  
>  	ret = inline_listxattr(&it);
> -	if (ret < 0 && ret != -ENOATTR)
> -		return ret;
> -	return shared_listxattr(&it);
> +	if (ret >= 0 || ret == -ENOATTR)
> +		ret = shared_listxattr(&it);
> +	erofs_put_metabuf(&it.it.buf);
> +	return ret;
>  }
>  
>  #ifdef CONFIG_EROFS_FS_POSIX_ACL
> -- 
> 2.24.4

Re: [PATCH v2 4/5] erofs: use meta buffers for xattr operations

[Chao Yu]

On 2022/1/2 12:00, Gao Xiang wrote:
> Get rid of old erofs_get_meta_page() within xattr operations by
> using on-stack meta buffers in order to prepare subpage and folio
> features.
> 
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

[PATCH v2 5/5] erofs: use meta buffers for zmap operations

[Gao Xiang]

Get rid of old erofs_get_meta_page() within zmap operations by
using on-stack meta buffers in order to prepare subpage and folio
features.

Finally, erofs_get_meta_page() is useless. Get rid of it!

Reviewed-by: Yue Hu <huyue2@yulong.com>
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/data.c     | 13 -----------
 fs/erofs/internal.h |  6 ++---
 fs/erofs/zdata.c    | 23 ++++++++-----------
 fs/erofs/zmap.c     | 56 +++++++++++++--------------------------------
 4 files changed, 28 insertions(+), 70 deletions(-)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index 6495e16a50a9..187f19f8a9a1 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -9,19 +9,6 @@
 #include <linux/dax.h>
 #include <trace/events/erofs.h>
 
-struct page *erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr)
-{
-	struct address_space *const mapping = sb->s_bdev->bd_inode->i_mapping;
-	struct page *page;
-
-	page = read_cache_page_gfp(mapping, blkaddr,
-				   mapping_gfp_constraint(mapping, ~__GFP_FS));
-	/* should already be PageUptodate */
-	if (!IS_ERR(page))
-		lock_page(page);
-	return page;
-}
-
 void erofs_unmap_metabuf(struct erofs_buf *buf)
 {
 	if (buf->kmap_type == EROFS_KMAP)
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index f1e4eb3025f6..3db494a398b2 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -419,14 +419,14 @@ enum {
 #define EROFS_MAP_FULL_MAPPED	(1 << BH_FullMapped)
 
 struct erofs_map_blocks {
+	struct erofs_buf buf;
+
 	erofs_off_t m_pa, m_la;
 	u64 m_plen, m_llen;
 
 	unsigned short m_deviceid;
 	char m_algorithmformat;
 	unsigned int m_flags;
-
-	struct page *mpage;
 };
 
 /* Flags used by erofs_map_blocks_flatmode() */
@@ -474,7 +474,7 @@ struct erofs_map_dev {
 
 /* data.c */
 extern const struct file_operations erofs_file_fops;
-struct page *erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr);
+void erofs_unmap_metabuf(struct erofs_buf *buf);
 void erofs_put_metabuf(struct erofs_buf *buf);
 void *erofs_read_metabuf(struct erofs_buf *buf, struct super_block *sb,
 			 erofs_blk_t blkaddr, enum erofs_kmap_type type);
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index 49da3931b2e3..498b7666efe8 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -698,20 +698,18 @@ static int z_erofs_do_read_page(struct z_erofs_decompress_frontend *fe,
 		goto err_out;
 
 	if (z_erofs_is_inline_pcluster(clt->pcl)) {
-		struct page *mpage;
+		void *mp;
 
-		mpage = erofs_get_meta_page(inode->i_sb,
-					    erofs_blknr(map->m_pa));
-		if (IS_ERR(mpage)) {
-			err = PTR_ERR(mpage);
+		mp = erofs_read_metabuf(&fe->map.buf, inode->i_sb,
+					erofs_blknr(map->m_pa), EROFS_NO_KMAP);
+		if (IS_ERR(mp)) {
+			err = PTR_ERR(mp);
 			erofs_err(inode->i_sb,
 				  "failed to get inline page, err %d", err);
 			goto err_out;
 		}
-		/* TODO: new subpage feature will get rid of it */
-		unlock_page(mpage);
-
-		WRITE_ONCE(clt->pcl->compressed_pages[0], mpage);
+		get_page(fe->map.buf.page);
+		WRITE_ONCE(clt->pcl->compressed_pages[0], fe->map.buf.page);
 		clt->mode = COLLECT_PRIMARY_FOLLOWED_NOINPLACE;
 	} else {
 		/* preload all compressed pages (can change mode if needed) */
@@ -1529,9 +1527,7 @@ static int z_erofs_readpage(struct file *file, struct page *page)
 	if (err)
 		erofs_err(inode->i_sb, "failed to read, err [%d]", err);
 
-	if (f.map.mpage)
-		put_page(f.map.mpage);
-
+	erofs_put_metabuf(&f.map.buf);
 	erofs_release_pages(&pagepool);
 	return err;
 }
@@ -1576,8 +1572,7 @@ static void z_erofs_readahead(struct readahead_control *rac)
 
 	z_erofs_runqueue(inode->i_sb, &f, &pagepool,
 			 z_erofs_get_sync_decompress_policy(sbi, nr_pages));
-	if (f.map.mpage)
-		put_page(f.map.mpage);
+	erofs_put_metabuf(&f.map.buf);
 	erofs_release_pages(&pagepool);
 }
 
diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
index 1037ac17b7a6..18d7fd1a5064 100644
--- a/fs/erofs/zmap.c
+++ b/fs/erofs/zmap.c
@@ -35,7 +35,7 @@ static int z_erofs_fill_inode_lazy(struct inode *inode)
 	struct super_block *const sb = inode->i_sb;
 	int err, headnr;
 	erofs_off_t pos;
-	struct page *page;
+	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
 	void *kaddr;
 	struct z_erofs_map_header *h;
 
@@ -61,14 +61,13 @@ static int z_erofs_fill_inode_lazy(struct inode *inode)
 
 	pos = ALIGN(iloc(EROFS_SB(sb), vi->nid) + vi->inode_isize +
 		    vi->xattr_isize, 8);
-	page = erofs_get_meta_page(sb, erofs_blknr(pos));
-	if (IS_ERR(page)) {
-		err = PTR_ERR(page);
+	kaddr = erofs_read_metabuf(&buf, sb, erofs_blknr(pos),
+				   EROFS_KMAP_ATOMIC);
+	if (IS_ERR(kaddr)) {
+		err = PTR_ERR(kaddr);
 		goto out_unlock;
 	}
 
-	kaddr = kmap_atomic(page);
-
 	h = kaddr + erofs_blkoff(pos);
 	vi->z_advise = le16_to_cpu(h->h_advise);
 	vi->z_algorithmtype[0] = h->h_algorithmtype & 15;
@@ -101,20 +100,19 @@ static int z_erofs_fill_inode_lazy(struct inode *inode)
 		goto unmap_done;
 	}
 unmap_done:
-	kunmap_atomic(kaddr);
-	unlock_page(page);
-	put_page(page);
+	erofs_put_metabuf(&buf);
 	if (err)
 		goto out_unlock;
 
 	if (vi->z_advise & Z_EROFS_ADVISE_INLINE_PCLUSTER) {
-		struct erofs_map_blocks map = { .mpage = NULL };
+		struct erofs_map_blocks map = {
+			.buf = __EROFS_BUF_INITIALIZER
+		};
 
 		vi->z_idata_size = le16_to_cpu(h->h_idata_size);
 		err = z_erofs_do_map_blocks(inode, &map,
 					    EROFS_GET_BLOCKS_FINDTAIL);
-		if (map.mpage)
-			put_page(map.mpage);
+		erofs_put_metabuf(&map.buf);
 
 		if (!map.m_plen ||
 		    erofs_blkoff(map.m_pa) + map.m_plen > EROFS_BLKSIZ) {
@@ -151,31 +149,11 @@ static int z_erofs_reload_indexes(struct z_erofs_maprecorder *m,
 				  erofs_blk_t eblk)
 {
 	struct super_block *const sb = m->inode->i_sb;
-	struct erofs_map_blocks *const map = m->map;
-	struct page *mpage = map->mpage;
-
-	if (mpage) {
-		if (mpage->index == eblk) {
-			if (!m->kaddr)
-				m->kaddr = kmap_atomic(mpage);
-			return 0;
-		}
 
-		if (m->kaddr) {
-			kunmap_atomic(m->kaddr);
-			m->kaddr = NULL;
-		}
-		put_page(mpage);
-	}
-
-	mpage = erofs_get_meta_page(sb, eblk);
-	if (IS_ERR(mpage)) {
-		map->mpage = NULL;
-		return PTR_ERR(mpage);
-	}
-	m->kaddr = kmap_atomic(mpage);
-	unlock_page(mpage);
-	map->mpage = mpage;
+	m->kaddr = erofs_read_metabuf(&m->map->buf, sb, eblk,
+				      EROFS_KMAP_ATOMIC);
+	if (IS_ERR(m->kaddr))
+		return PTR_ERR(m->kaddr);
 	return 0;
 }
 
@@ -711,8 +689,7 @@ static int z_erofs_do_map_blocks(struct inode *inode,
 			map->m_flags |= EROFS_MAP_FULL_MAPPED;
 	}
 unmap_out:
-	if (m.kaddr)
-		kunmap_atomic(m.kaddr);
+	erofs_unmap_metabuf(&m.map->buf);
 
 out:
 	erofs_dbg("%s, m_la %llu m_pa %llu m_llen %llu m_plen %llu m_flags 0%o",
@@ -759,8 +736,7 @@ static int z_erofs_iomap_begin_report(struct inode *inode, loff_t offset,
 	struct erofs_map_blocks map = { .m_la = offset };
 
 	ret = z_erofs_map_blocks_iter(inode, &map, EROFS_GET_BLOCKS_FIEMAP);
-	if (map.mpage)
-		put_page(map.mpage);
+	erofs_put_metabuf(&map.buf);
 	if (ret < 0)
 		return ret;
 
-- 
2.24.4

Re: [PATCH v2 5/5] erofs: use meta buffers for zmap operations

[Liu Bo]

On Sun, Jan 02, 2022 at 12:00:17PM +0800, Gao Xiang wrote:
> Get rid of old erofs_get_meta_page() within zmap operations by
> using on-stack meta buffers in order to prepare subpage and folio
> features.
> 
> Finally, erofs_get_meta_page() is useless. Get rid of it!

Reviewed-by: Liu Bo <bo.liu@linux.alibaba.com>

> 
> Reviewed-by: Yue Hu <huyue2@yulong.com>
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
>  fs/erofs/data.c     | 13 -----------
>  fs/erofs/internal.h |  6 ++---
>  fs/erofs/zdata.c    | 23 ++++++++-----------
>  fs/erofs/zmap.c     | 56 +++++++++++++--------------------------------
>  4 files changed, 28 insertions(+), 70 deletions(-)
> 
> diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> index 6495e16a50a9..187f19f8a9a1 100644
> --- a/fs/erofs/data.c
> +++ b/fs/erofs/data.c
> @@ -9,19 +9,6 @@
>  #include <linux/dax.h>
>  #include <trace/events/erofs.h>
>  
> -struct page *erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr)
> -{
> -	struct address_space *const mapping = sb->s_bdev->bd_inode->i_mapping;
> -	struct page *page;
> -
> -	page = read_cache_page_gfp(mapping, blkaddr,
> -				   mapping_gfp_constraint(mapping, ~__GFP_FS));
> -	/* should already be PageUptodate */
> -	if (!IS_ERR(page))
> -		lock_page(page);
> -	return page;
> -}
> -
>  void erofs_unmap_metabuf(struct erofs_buf *buf)
>  {
>  	if (buf->kmap_type == EROFS_KMAP)
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index f1e4eb3025f6..3db494a398b2 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -419,14 +419,14 @@ enum {
>  #define EROFS_MAP_FULL_MAPPED	(1 << BH_FullMapped)
>  
>  struct erofs_map_blocks {
> +	struct erofs_buf buf;
> +
>  	erofs_off_t m_pa, m_la;
>  	u64 m_plen, m_llen;
>  
>  	unsigned short m_deviceid;
>  	char m_algorithmformat;
>  	unsigned int m_flags;
> -
> -	struct page *mpage;
>  };
>  
>  /* Flags used by erofs_map_blocks_flatmode() */
> @@ -474,7 +474,7 @@ struct erofs_map_dev {
>  
>  /* data.c */
>  extern const struct file_operations erofs_file_fops;
> -struct page *erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr);
> +void erofs_unmap_metabuf(struct erofs_buf *buf);
>  void erofs_put_metabuf(struct erofs_buf *buf);
>  void *erofs_read_metabuf(struct erofs_buf *buf, struct super_block *sb,
>  			 erofs_blk_t blkaddr, enum erofs_kmap_type type);
> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
> index 49da3931b2e3..498b7666efe8 100644
> --- a/fs/erofs/zdata.c
> +++ b/fs/erofs/zdata.c
> @@ -698,20 +698,18 @@ static int z_erofs_do_read_page(struct z_erofs_decompress_frontend *fe,
>  		goto err_out;
>  
>  	if (z_erofs_is_inline_pcluster(clt->pcl)) {
> -		struct page *mpage;
> +		void *mp;
>  
> -		mpage = erofs_get_meta_page(inode->i_sb,
> -					    erofs_blknr(map->m_pa));
> -		if (IS_ERR(mpage)) {
> -			err = PTR_ERR(mpage);
> +		mp = erofs_read_metabuf(&fe->map.buf, inode->i_sb,
> +					erofs_blknr(map->m_pa), EROFS_NO_KMAP);
> +		if (IS_ERR(mp)) {
> +			err = PTR_ERR(mp);
>  			erofs_err(inode->i_sb,
>  				  "failed to get inline page, err %d", err);
>  			goto err_out;
>  		}
> -		/* TODO: new subpage feature will get rid of it */
> -		unlock_page(mpage);
> -
> -		WRITE_ONCE(clt->pcl->compressed_pages[0], mpage);
> +		get_page(fe->map.buf.page);
> +		WRITE_ONCE(clt->pcl->compressed_pages[0], fe->map.buf.page);
>  		clt->mode = COLLECT_PRIMARY_FOLLOWED_NOINPLACE;
>  	} else {
>  		/* preload all compressed pages (can change mode if needed) */
> @@ -1529,9 +1527,7 @@ static int z_erofs_readpage(struct file *file, struct page *page)
>  	if (err)
>  		erofs_err(inode->i_sb, "failed to read, err [%d]", err);
>  
> -	if (f.map.mpage)
> -		put_page(f.map.mpage);
> -
> +	erofs_put_metabuf(&f.map.buf);
>  	erofs_release_pages(&pagepool);
>  	return err;
>  }
> @@ -1576,8 +1572,7 @@ static void z_erofs_readahead(struct readahead_control *rac)
>  
>  	z_erofs_runqueue(inode->i_sb, &f, &pagepool,
>  			 z_erofs_get_sync_decompress_policy(sbi, nr_pages));
> -	if (f.map.mpage)
> -		put_page(f.map.mpage);
> +	erofs_put_metabuf(&f.map.buf);
>  	erofs_release_pages(&pagepool);
>  }
>  
> diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
> index 1037ac17b7a6..18d7fd1a5064 100644
> --- a/fs/erofs/zmap.c
> +++ b/fs/erofs/zmap.c
> @@ -35,7 +35,7 @@ static int z_erofs_fill_inode_lazy(struct inode *inode)
>  	struct super_block *const sb = inode->i_sb;
>  	int err, headnr;
>  	erofs_off_t pos;
> -	struct page *page;
> +	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
>  	void *kaddr;
>  	struct z_erofs_map_header *h;
>  
> @@ -61,14 +61,13 @@ static int z_erofs_fill_inode_lazy(struct inode *inode)
>  
>  	pos = ALIGN(iloc(EROFS_SB(sb), vi->nid) + vi->inode_isize +
>  		    vi->xattr_isize, 8);
> -	page = erofs_get_meta_page(sb, erofs_blknr(pos));
> -	if (IS_ERR(page)) {
> -		err = PTR_ERR(page);
> +	kaddr = erofs_read_metabuf(&buf, sb, erofs_blknr(pos),
> +				   EROFS_KMAP_ATOMIC);
> +	if (IS_ERR(kaddr)) {
> +		err = PTR_ERR(kaddr);
>  		goto out_unlock;
>  	}
>  
> -	kaddr = kmap_atomic(page);
> -
>  	h = kaddr + erofs_blkoff(pos);
>  	vi->z_advise = le16_to_cpu(h->h_advise);
>  	vi->z_algorithmtype[0] = h->h_algorithmtype & 15;
> @@ -101,20 +100,19 @@ static int z_erofs_fill_inode_lazy(struct inode *inode)
>  		goto unmap_done;
>  	}
>  unmap_done:
> -	kunmap_atomic(kaddr);
> -	unlock_page(page);
> -	put_page(page);
> +	erofs_put_metabuf(&buf);
>  	if (err)
>  		goto out_unlock;
>  
>  	if (vi->z_advise & Z_EROFS_ADVISE_INLINE_PCLUSTER) {
> -		struct erofs_map_blocks map = { .mpage = NULL };
> +		struct erofs_map_blocks map = {
> +			.buf = __EROFS_BUF_INITIALIZER
> +		};
>  
>  		vi->z_idata_size = le16_to_cpu(h->h_idata_size);
>  		err = z_erofs_do_map_blocks(inode, &map,
>  					    EROFS_GET_BLOCKS_FINDTAIL);
> -		if (map.mpage)
> -			put_page(map.mpage);
> +		erofs_put_metabuf(&map.buf);
>  
>  		if (!map.m_plen ||
>  		    erofs_blkoff(map.m_pa) + map.m_plen > EROFS_BLKSIZ) {
> @@ -151,31 +149,11 @@ static int z_erofs_reload_indexes(struct z_erofs_maprecorder *m,
>  				  erofs_blk_t eblk)
>  {
>  	struct super_block *const sb = m->inode->i_sb;
> -	struct erofs_map_blocks *const map = m->map;
> -	struct page *mpage = map->mpage;
> -
> -	if (mpage) {
> -		if (mpage->index == eblk) {
> -			if (!m->kaddr)
> -				m->kaddr = kmap_atomic(mpage);
> -			return 0;
> -		}
>  
> -		if (m->kaddr) {
> -			kunmap_atomic(m->kaddr);
> -			m->kaddr = NULL;
> -		}
> -		put_page(mpage);
> -	}
> -
> -	mpage = erofs_get_meta_page(sb, eblk);
> -	if (IS_ERR(mpage)) {
> -		map->mpage = NULL;
> -		return PTR_ERR(mpage);
> -	}
> -	m->kaddr = kmap_atomic(mpage);
> -	unlock_page(mpage);
> -	map->mpage = mpage;
> +	m->kaddr = erofs_read_metabuf(&m->map->buf, sb, eblk,
> +				      EROFS_KMAP_ATOMIC);
> +	if (IS_ERR(m->kaddr))
> +		return PTR_ERR(m->kaddr);
>  	return 0;
>  }
>  
> @@ -711,8 +689,7 @@ static int z_erofs_do_map_blocks(struct inode *inode,
>  			map->m_flags |= EROFS_MAP_FULL_MAPPED;
>  	}
>  unmap_out:
> -	if (m.kaddr)
> -		kunmap_atomic(m.kaddr);
> +	erofs_unmap_metabuf(&m.map->buf);
>  
>  out:
>  	erofs_dbg("%s, m_la %llu m_pa %llu m_llen %llu m_plen %llu m_flags 0%o",
> @@ -759,8 +736,7 @@ static int z_erofs_iomap_begin_report(struct inode *inode, loff_t offset,
>  	struct erofs_map_blocks map = { .m_la = offset };
>  
>  	ret = z_erofs_map_blocks_iter(inode, &map, EROFS_GET_BLOCKS_FIEMAP);
> -	if (map.mpage)
> -		put_page(map.mpage);
> +	erofs_put_metabuf(&map.buf);
>  	if (ret < 0)
>  		return ret;
>  
> -- 
> 2.24.4

Re: [PATCH v2 5/5] erofs: use meta buffers for zmap operations

[Chao Yu]

On 2022/1/2 12:00, Gao Xiang wrote:
> Get rid of old erofs_get_meta_page() within zmap operations by
> using on-stack meta buffers in order to prepare subpage and folio
> features.
> 
> Finally, erofs_get_meta_page() is useless. Get rid of it!
> 
> Reviewed-by: Yue Hu <huyue2@yulong.com>
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
>   fs/erofs/data.c     | 13 -----------
>   fs/erofs/internal.h |  6 ++---
>   fs/erofs/zdata.c    | 23 ++++++++-----------
>   fs/erofs/zmap.c     | 56 +++++++++++++--------------------------------
>   4 files changed, 28 insertions(+), 70 deletions(-)
> 
> diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> index 6495e16a50a9..187f19f8a9a1 100644
> --- a/fs/erofs/data.c
> +++ b/fs/erofs/data.c
> @@ -9,19 +9,6 @@
>   #include <linux/dax.h>
>   #include <trace/events/erofs.h>
>   
> -struct page *erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr)
> -{
> -	struct address_space *const mapping = sb->s_bdev->bd_inode->i_mapping;
> -	struct page *page;
> -
> -	page = read_cache_page_gfp(mapping, blkaddr,
> -				   mapping_gfp_constraint(mapping, ~__GFP_FS));
> -	/* should already be PageUptodate */
> -	if (!IS_ERR(page))
> -		lock_page(page);
> -	return page;
> -}
> -
>   void erofs_unmap_metabuf(struct erofs_buf *buf)
>   {
>   	if (buf->kmap_type == EROFS_KMAP)
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index f1e4eb3025f6..3db494a398b2 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -419,14 +419,14 @@ enum {
>   #define EROFS_MAP_FULL_MAPPED	(1 << BH_FullMapped)
>   
>   struct erofs_map_blocks {
> +	struct erofs_buf buf;
> +
>   	erofs_off_t m_pa, m_la;
>   	u64 m_plen, m_llen;
>   
>   	unsigned short m_deviceid;
>   	char m_algorithmformat;
>   	unsigned int m_flags;
> -
> -	struct page *mpage;
>   };
>   
>   /* Flags used by erofs_map_blocks_flatmode() */
> @@ -474,7 +474,7 @@ struct erofs_map_dev {
>   
>   /* data.c */
>   extern const struct file_operations erofs_file_fops;
> -struct page *erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr);
> +void erofs_unmap_metabuf(struct erofs_buf *buf);
>   void erofs_put_metabuf(struct erofs_buf *buf);
>   void *erofs_read_metabuf(struct erofs_buf *buf, struct super_block *sb,
>   			 erofs_blk_t blkaddr, enum erofs_kmap_type type);
> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
> index 49da3931b2e3..498b7666efe8 100644
> --- a/fs/erofs/zdata.c
> +++ b/fs/erofs/zdata.c
> @@ -698,20 +698,18 @@ static int z_erofs_do_read_page(struct z_erofs_decompress_frontend *fe,
>   		goto err_out;
>   
>   	if (z_erofs_is_inline_pcluster(clt->pcl)) {
> -		struct page *mpage;
> +		void *mp;
>   
> -		mpage = erofs_get_meta_page(inode->i_sb,
> -					    erofs_blknr(map->m_pa));
> -		if (IS_ERR(mpage)) {
> -			err = PTR_ERR(mpage);
> +		mp = erofs_read_metabuf(&fe->map.buf, inode->i_sb,
> +					erofs_blknr(map->m_pa), EROFS_NO_KMAP);
> +		if (IS_ERR(mp)) {
> +			err = PTR_ERR(mp);
>   			erofs_err(inode->i_sb,
>   				  "failed to get inline page, err %d", err);
>   			goto err_out;
>   		}
> -		/* TODO: new subpage feature will get rid of it */
> -		unlock_page(mpage);
> -
> -		WRITE_ONCE(clt->pcl->compressed_pages[0], mpage);
> +		get_page(fe->map.buf.page);

Comparing to previous implementation, it adds an extra reference on the page, why?

Thanks,

> +		WRITE_ONCE(clt->pcl->compressed_pages[0], fe->map.buf.page);
>   		clt->mode = COLLECT_PRIMARY_FOLLOWED_NOINPLACE;
>   	} else {
>   		/* preload all compressed pages (can change mode if needed) */
> @@ -1529,9 +1527,7 @@ static int z_erofs_readpage(struct file *file, struct page *page)
>   	if (err)
>   		erofs_err(inode->i_sb, "failed to read, err [%d]", err);
>   
> -	if (f.map.mpage)
> -		put_page(f.map.mpage);
> -
> +	erofs_put_metabuf(&f.map.buf);
>   	erofs_release_pages(&pagepool);
>   	return err;
>   }
> @@ -1576,8 +1572,7 @@ static void z_erofs_readahead(struct readahead_control *rac)
>   
>   	z_erofs_runqueue(inode->i_sb, &f, &pagepool,
>   			 z_erofs_get_sync_decompress_policy(sbi, nr_pages));
> -	if (f.map.mpage)
> -		put_page(f.map.mpage);
> +	erofs_put_metabuf(&f.map.buf);
>   	erofs_release_pages(&pagepool);
>   }
>   
> diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
> index 1037ac17b7a6..18d7fd1a5064 100644
> --- a/fs/erofs/zmap.c
> +++ b/fs/erofs/zmap.c
> @@ -35,7 +35,7 @@ static int z_erofs_fill_inode_lazy(struct inode *inode)
>   	struct super_block *const sb = inode->i_sb;
>   	int err, headnr;
>   	erofs_off_t pos;
> -	struct page *page;
> +	struct erofs_buf buf = __EROFS_BUF_INITIALIZER;
>   	void *kaddr;
>   	struct z_erofs_map_header *h;
>   
> @@ -61,14 +61,13 @@ static int z_erofs_fill_inode_lazy(struct inode *inode)
>   
>   	pos = ALIGN(iloc(EROFS_SB(sb), vi->nid) + vi->inode_isize +
>   		    vi->xattr_isize, 8);
> -	page = erofs_get_meta_page(sb, erofs_blknr(pos));
> -	if (IS_ERR(page)) {
> -		err = PTR_ERR(page);
> +	kaddr = erofs_read_metabuf(&buf, sb, erofs_blknr(pos),
> +				   EROFS_KMAP_ATOMIC);
> +	if (IS_ERR(kaddr)) {
> +		err = PTR_ERR(kaddr);
>   		goto out_unlock;
>   	}
>   
> -	kaddr = kmap_atomic(page);
> -
>   	h = kaddr + erofs_blkoff(pos);
>   	vi->z_advise = le16_to_cpu(h->h_advise);
>   	vi->z_algorithmtype[0] = h->h_algorithmtype & 15;
> @@ -101,20 +100,19 @@ static int z_erofs_fill_inode_lazy(struct inode *inode)
>   		goto unmap_done;
>   	}
>   unmap_done:
> -	kunmap_atomic(kaddr);
> -	unlock_page(page);
> -	put_page(page);
> +	erofs_put_metabuf(&buf);
>   	if (err)
>   		goto out_unlock;
>   
>   	if (vi->z_advise & Z_EROFS_ADVISE_INLINE_PCLUSTER) {
> -		struct erofs_map_blocks map = { .mpage = NULL };
> +		struct erofs_map_blocks map = {
> +			.buf = __EROFS_BUF_INITIALIZER
> +		};
>   
>   		vi->z_idata_size = le16_to_cpu(h->h_idata_size);
>   		err = z_erofs_do_map_blocks(inode, &map,
>   					    EROFS_GET_BLOCKS_FINDTAIL);
> -		if (map.mpage)
> -			put_page(map.mpage);
> +		erofs_put_metabuf(&map.buf);
>   
>   		if (!map.m_plen ||
>   		    erofs_blkoff(map.m_pa) + map.m_plen > EROFS_BLKSIZ) {
> @@ -151,31 +149,11 @@ static int z_erofs_reload_indexes(struct z_erofs_maprecorder *m,
>   				  erofs_blk_t eblk)
>   {
>   	struct super_block *const sb = m->inode->i_sb;
> -	struct erofs_map_blocks *const map = m->map;
> -	struct page *mpage = map->mpage;
> -
> -	if (mpage) {
> -		if (mpage->index == eblk) {
> -			if (!m->kaddr)
> -				m->kaddr = kmap_atomic(mpage);
> -			return 0;
> -		}
>   
> -		if (m->kaddr) {
> -			kunmap_atomic(m->kaddr);
> -			m->kaddr = NULL;
> -		}
> -		put_page(mpage);
> -	}
> -
> -	mpage = erofs_get_meta_page(sb, eblk);
> -	if (IS_ERR(mpage)) {
> -		map->mpage = NULL;
> -		return PTR_ERR(mpage);
> -	}
> -	m->kaddr = kmap_atomic(mpage);
> -	unlock_page(mpage);
> -	map->mpage = mpage;
> +	m->kaddr = erofs_read_metabuf(&m->map->buf, sb, eblk,
> +				      EROFS_KMAP_ATOMIC);
> +	if (IS_ERR(m->kaddr))
> +		return PTR_ERR(m->kaddr);
>   	return 0;
>   }
>   
> @@ -711,8 +689,7 @@ static int z_erofs_do_map_blocks(struct inode *inode,
>   			map->m_flags |= EROFS_MAP_FULL_MAPPED;
>   	}
>   unmap_out:
> -	if (m.kaddr)
> -		kunmap_atomic(m.kaddr);
> +	erofs_unmap_metabuf(&m.map->buf);
>   
>   out:
>   	erofs_dbg("%s, m_la %llu m_pa %llu m_llen %llu m_plen %llu m_flags 0%o",
> @@ -759,8 +736,7 @@ static int z_erofs_iomap_begin_report(struct inode *inode, loff_t offset,
>   	struct erofs_map_blocks map = { .m_la = offset };
>   
>   	ret = z_erofs_map_blocks_iter(inode, &map, EROFS_GET_BLOCKS_FIEMAP);
> -	if (map.mpage)
> -		put_page(map.mpage);
> +	erofs_put_metabuf(&map.buf);
>   	if (ret < 0)
>   		return ret;
>   

Re: [PATCH v2 5/5] erofs: use meta buffers for zmap operations

[Gao Xiang]

Hi Chao,

On Tue, Jan 04, 2022 at 04:15:05PM +0800, Chao Yu wrote:
> On 2022/1/2 12:00, Gao Xiang wrote:

...

> > --- a/fs/erofs/zdata.c
> > +++ b/fs/erofs/zdata.c
> > @@ -698,20 +698,18 @@ static int z_erofs_do_read_page(struct z_erofs_decompress_frontend *fe,
> >   		goto err_out;
> >   	if (z_erofs_is_inline_pcluster(clt->pcl)) {
> > -		struct page *mpage;
> > +		void *mp;
> > -		mpage = erofs_get_meta_page(inode->i_sb,
> > -					    erofs_blknr(map->m_pa));
> > -		if (IS_ERR(mpage)) {
> > -			err = PTR_ERR(mpage);
> > +		mp = erofs_read_metabuf(&fe->map.buf, inode->i_sb,
> > +					erofs_blknr(map->m_pa), EROFS_NO_KMAP);
> > +		if (IS_ERR(mp)) {
> > +			err = PTR_ERR(mp);
> >   			erofs_err(inode->i_sb,
> >   				  "failed to get inline page, err %d", err);
> >   			goto err_out;
> >   		}
> > -		/* TODO: new subpage feature will get rid of it */
> > -		unlock_page(mpage);
> > -
> > -		WRITE_ONCE(clt->pcl->compressed_pages[0], mpage);
> > +		get_page(fe->map.buf.page);
> 
> Comparing to previous implementation, it adds an extra reference on the page, why?

Thanks for the question. Previously, erofs_get_meta_page was called
independently without reusing zmap mpage, so the page refcount had no
relationship with zmap mpage.

However, now we reuse zmap metabuf instead(fe->map.buf), so an extra
page refcount is needed since zmap metabuf will be released at the end
of readpage or readahead...

Thanks,
Gao Xiang

Re: [PATCH v2 5/5] erofs: use meta buffers for zmap operations

[Chao Yu]

On 2022/1/4 17:02, Gao Xiang wrote:
> Thanks for the question. Previously, erofs_get_meta_page was called
> independently without reusing zmap mpage, so the page refcount had no
> relationship with zmap mpage.
> 
> However, now we reuse zmap metabuf instead(fe->map.buf), so an extra
> page refcount is needed since zmap metabuf will be released at the end
> of readpage or readahead...

Thanks for the explanation.

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,