* [PATCH 1/5] erofs: introduce meta buffer operations
2021-12-29 4:14 [PATCH 0/5] erofs: get rid of erofs_get_meta_page() Gao Xiang
@ 2021-12-29 4:14 ` Gao Xiang
2021-12-29 4:14 ` [PATCH 2/5] erofs: use meta buffers for inode operations Gao Xiang
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Gao Xiang @ 2021-12-29 4:14 UTC (permalink / raw)
To: linux-erofs, Chao Yu, Liu Bo; +Cc: LKML, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/5] erofs: use meta buffers for inode operations
2021-12-29 4:14 [PATCH 0/5] erofs: get rid of erofs_get_meta_page() Gao Xiang
2021-12-29 4:14 ` [PATCH 1/5] erofs: introduce meta buffer operations Gao Xiang
@ 2021-12-29 4:14 ` Gao Xiang
2021-12-30 4:05 ` Yue Hu
2021-12-29 4:14 ` [PATCH 3/5] erofs: use meta buffers for super operations Gao Xiang
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Gao Xiang @ 2021-12-29 4:14 UTC (permalink / raw)
To: linux-erofs, Chao Yu, Liu Bo; +Cc: LKML, 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.
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/5] erofs: use meta buffers for inode operations
2021-12-29 4:14 ` [PATCH 2/5] erofs: use meta buffers for inode operations Gao Xiang
@ 2021-12-30 4:05 ` Yue Hu
0 siblings, 0 replies; 13+ messages in thread
From: Yue Hu @ 2021-12-30 4:05 UTC (permalink / raw)
To: Gao Xiang
Cc: linux-erofs, Chao Yu, Liu Bo, LKML, geshifei, zhangwen,
shaojunjun, huyue
On Wed, 29 Dec 2021 12:14:02 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> 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.
>
> 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);
Reviewed-by: Yue Hu <huyue2@yulong.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/5] erofs: use meta buffers for super operations
2021-12-29 4:14 [PATCH 0/5] erofs: get rid of erofs_get_meta_page() Gao Xiang
2021-12-29 4:14 ` [PATCH 1/5] erofs: introduce meta buffer operations Gao Xiang
2021-12-29 4:14 ` [PATCH 2/5] erofs: use meta buffers for inode operations Gao Xiang
@ 2021-12-29 4:14 ` Gao Xiang
2021-12-30 4:11 ` Yue Hu
2022-01-06 15:08 ` Dan Carpenter
2021-12-29 4:14 ` [PATCH 4/5] erofs: use meta buffers for xattr operations Gao Xiang
2021-12-29 4:14 ` [PATCH 5/5] erofs: use meta buffers for zmap operations Gao Xiang
4 siblings, 2 replies; 13+ messages in thread
From: Gao Xiang @ 2021-12-29 4:14 UTC (permalink / raw)
To: linux-erofs, Chao Yu, Liu Bo; +Cc: LKML, 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.
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/5] erofs: use meta buffers for super operations
2021-12-29 4:14 ` [PATCH 3/5] erofs: use meta buffers for super operations Gao Xiang
@ 2021-12-30 4:11 ` Yue Hu
2022-01-06 15:08 ` Dan Carpenter
1 sibling, 0 replies; 13+ messages in thread
From: Yue Hu @ 2021-12-30 4:11 UTC (permalink / raw)
To: Gao Xiang
Cc: linux-erofs, Chao Yu, Liu Bo, LKML, huyue, geshifei, zhangwen,
shaojunjun
On Wed, 29 Dec 2021 12:14:03 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> 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.
>
> 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;
> }
>
Reviewed-by: Yue Hu <huyue2@yulong.com>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/5] erofs: use meta buffers for super operations
2021-12-29 4:14 ` [PATCH 3/5] erofs: use meta buffers for super operations Gao Xiang
2021-12-30 4:11 ` Yue Hu
@ 2022-01-06 15:08 ` Dan Carpenter
2022-01-06 16:40 ` Gao Xiang
1 sibling, 1 reply; 13+ messages in thread
From: Dan Carpenter @ 2022-01-06 15:08 UTC (permalink / raw)
To: kbuild, Gao Xiang, linux-erofs, Chao Yu, Chao Yu, Liu Bo
Cc: lkp, kbuild-all, LKML, Gao Xiang
Hi Gao,
url: https://github.com/0day-ci/linux/commits/Gao-Xiang/erofs-get-rid-of-erofs_get_meta_page/20211229-121538
base: https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs.git dev-test
config: x86_64-randconfig-m001-20211230 (https://download.01.org/0day-ci/archive/20211231/202112310650.TDDinvVT-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
fs/erofs/super.c:153 erofs_read_metadata() warn: possible memory leak of 'buffer'
vim +/buffer +153 fs/erofs/super.c
6477e408d41152 fs/erofs/super.c Gao Xiang 2021-12-29 128 static void *erofs_read_metadata(struct super_block *sb, struct erofs_buf *buf,
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 129 erofs_off_t *offset, int *lengthp)
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 130 {
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 131 u8 *buffer, *ptr;
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 132 int len, i, cnt;
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 133
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 134 *offset = round_up(*offset, 4);
6477e408d41152 fs/erofs/super.c Gao Xiang 2021-12-29 135 ptr = erofs_read_metabuf(buf, sb, erofs_blknr(*offset), EROFS_KMAP);
6477e408d41152 fs/erofs/super.c Gao Xiang 2021-12-29 136 if (IS_ERR(ptr))
6477e408d41152 fs/erofs/super.c Gao Xiang 2021-12-29 137 return ptr;
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 138
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 139 len = le16_to_cpu(*(__le16 *)&ptr[erofs_blkoff(*offset)]);
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 140 if (!len)
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 141 len = U16_MAX + 1;
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 142 buffer = kmalloc(len, GFP_KERNEL);
6477e408d41152 fs/erofs/super.c Gao Xiang 2021-12-29 143 if (!buffer)
6477e408d41152 fs/erofs/super.c Gao Xiang 2021-12-29 144 return ERR_PTR(-ENOMEM);
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 145 *offset += sizeof(__le16);
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 146 *lengthp = len;
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 147
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 148 for (i = 0; i < len; i += cnt) {
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 149 cnt = min(EROFS_BLKSIZ - (int)erofs_blkoff(*offset), len - i);
6477e408d41152 fs/erofs/super.c Gao Xiang 2021-12-29 150 ptr = erofs_read_metabuf(buf, sb, erofs_blknr(*offset),
6477e408d41152 fs/erofs/super.c Gao Xiang 2021-12-29 151 EROFS_KMAP);
6477e408d41152 fs/erofs/super.c Gao Xiang 2021-12-29 152 if (IS_ERR(ptr))
6477e408d41152 fs/erofs/super.c Gao Xiang 2021-12-29 @153 return ptr;
"buffer" not freed on error path.
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 154 memcpy(buffer + i, ptr + erofs_blkoff(*offset), cnt);
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 155 *offset += cnt;
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 156 }
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 157 return buffer;
14373711dd54be fs/erofs/super.c Gao Xiang 2021-03-29 158 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 3/5] erofs: use meta buffers for super operations
2022-01-06 15:08 ` Dan Carpenter
@ 2022-01-06 16:40 ` Gao Xiang
0 siblings, 0 replies; 13+ messages in thread
From: Gao Xiang @ 2022-01-06 16:40 UTC (permalink / raw)
To: Dan Carpenter
Cc: kbuild, linux-erofs, Chao Yu, Chao Yu, Liu Bo, lkp, kbuild-all,
LKML
Hi Dan,
On Thu, Jan 06, 2022 at 06:08:48PM +0300, Dan Carpenter wrote:
> Hi Gao,
>
> url: https://github.com/0day-ci/linux/commits/Gao-Xiang/erofs-get-rid-of-erofs_get_meta_page/20211229-121538
> base: https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs.git dev-test
> config: x86_64-randconfig-m001-20211230 (https://download.01.org/0day-ci/archive/20211231/202112310650.TDDinvVT-lkp@intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> smatch warnings:
> fs/erofs/super.c:153 erofs_read_metadata() warn: possible memory leak of 'buffer'
Many thanks for the report.
It has already been fixed in the latest version:
https://lore.kernel.org/all/20220102081317.109797-1-hsiangkao@linux.alibaba.com/
(I've pushed out to the linux-next...)
Thanks,
Gao Xiang
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/5] erofs: use meta buffers for xattr operations
2021-12-29 4:14 [PATCH 0/5] erofs: get rid of erofs_get_meta_page() Gao Xiang
` (2 preceding siblings ...)
2021-12-29 4:14 ` [PATCH 3/5] erofs: use meta buffers for super operations Gao Xiang
@ 2021-12-29 4:14 ` Gao Xiang
2021-12-30 4:14 ` Gao Xiang
2021-12-29 4:14 ` [PATCH 5/5] erofs: use meta buffers for zmap operations Gao Xiang
4 siblings, 1 reply; 13+ messages in thread
From: Gao Xiang @ 2021-12-29 4:14 UTC (permalink / raw)
To: linux-erofs, Chao Yu, Liu Bo; +Cc: LKML, 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 | 123 ++++++++++++++---------------------------------
1 file changed, 37 insertions(+), 86 deletions(-)
diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 01c581e93c5f..539c2aec20d3 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;
}
@@ -405,15 +367,11 @@ static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
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);
+ if (blkaddr != it->it.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;
}
@@ -421,9 +379,6 @@ static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
if (ret != -ENOATTR)
break;
}
- if (vi->xattr_shared_count)
- xattr_iter_end_final(&it->it);
-
return ret ? ret : it->buffer_size;
}
@@ -452,10 +407,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 +421,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 +564,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,15 +581,11 @@ 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);
+ if (blkaddr != it->it.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;
}
@@ -641,9 +593,6 @@ static int shared_listxattr(struct listxattr_iter *it)
if (ret)
break;
}
- if (vi->xattr_shared_count)
- xattr_iter_end_final(&it->it);
-
return ret ? ret : it->buffer_ofs;
}
@@ -659,6 +608,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;
@@ -669,6 +619,7 @@ ssize_t erofs_listxattr(struct dentry *dentry,
ret = inline_listxattr(&it);
if (ret < 0 && ret != -ENOATTR)
return ret;
+ erofs_put_metabuf(&it.it.buf);
return shared_listxattr(&it);
}
--
2.24.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/5] erofs: use meta buffers for xattr operations
2021-12-29 4:14 ` [PATCH 4/5] erofs: use meta buffers for xattr operations Gao Xiang
@ 2021-12-30 4:14 ` Gao Xiang
0 siblings, 0 replies; 13+ messages in thread
From: Gao Xiang @ 2021-12-30 4:14 UTC (permalink / raw)
To: linux-erofs, Chao Yu, Liu Bo; +Cc: LKML
On Wed, Dec 29, 2021 at 12:14:04PM +0800, Gao Xiang wrote:
...
>
> @@ -659,6 +608,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;
> @@ -669,6 +619,7 @@ ssize_t erofs_listxattr(struct dentry *dentry,
> ret = inline_listxattr(&it);
> if (ret < 0 && ret != -ENOATTR)
> return ret;
> + erofs_put_metabuf(&it.it.buf);
> return shared_listxattr(&it);
There is a bug here, I will fix it in the next version.
Thanks,
Gao Xiang
> }
>
> --
> 2.24.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/5] erofs: use meta buffers for zmap operations
2021-12-29 4:14 [PATCH 0/5] erofs: get rid of erofs_get_meta_page() Gao Xiang
` (3 preceding siblings ...)
2021-12-29 4:14 ` [PATCH 4/5] erofs: use meta buffers for xattr operations Gao Xiang
@ 2021-12-29 4:14 ` Gao Xiang
2021-12-29 5:19 ` Yue Hu
4 siblings, 1 reply; 13+ messages in thread
From: Gao Xiang @ 2021-12-29 4:14 UTC (permalink / raw)
To: linux-erofs, Chao Yu, Liu Bo; +Cc: LKML, 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!
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 5/5] erofs: use meta buffers for zmap operations
2021-12-29 4:14 ` [PATCH 5/5] erofs: use meta buffers for zmap operations Gao Xiang
@ 2021-12-29 5:19 ` Yue Hu
2021-12-29 5:28 ` Gao Xiang
0 siblings, 1 reply; 13+ messages in thread
From: Yue Hu @ 2021-12-29 5:19 UTC (permalink / raw)
To: Gao Xiang
Cc: linux-erofs, Chao Yu, Liu Bo, LKML, geshifei, zhangwen,
shaojunjun
On Wed, 29 Dec 2021 12:14:05 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> 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!
>
> 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;
> }
remove z_erofs_reload_indexes() and use erofs_read_metabuf() directly in caller?
>
> @@ -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;
>
Reviewed-by: Yue Hu <huyue2@yulong.com>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 5/5] erofs: use meta buffers for zmap operations
2021-12-29 5:19 ` Yue Hu
@ 2021-12-29 5:28 ` Gao Xiang
0 siblings, 0 replies; 13+ messages in thread
From: Gao Xiang @ 2021-12-29 5:28 UTC (permalink / raw)
To: Yue Hu; +Cc: linux-erofs, Chao Yu, Liu Bo, LKML, geshifei, zhangwen,
shaojunjun
Hi Yue,
On Wed, Dec 29, 2021 at 01:19:46PM +0800, Yue Hu wrote:
> On Wed, 29 Dec 2021 12:14:05 +0800
> Gao Xiang <hsiangkao@linux.alibaba.com> 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!
> >
> > 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;
> > }
>
> remove z_erofs_reload_indexes() and use erofs_read_metabuf() directly in caller?
I'd suggest we do later since liberofs in erofs-utils doesn't
have erofs_buf for now.
We could get rid of it together with erofs-utils separately.
and if you have time ;)
>
> >
> > @@ -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;
> >
>
> Reviewed-by: Yue Hu <huyue2@yulong.com>
Thanks, if you have time, please also check out the rest patches...
Thanks,
Gao Xiang
^ permalink raw reply [flat|nested] 13+ messages in thread