* [PATCH 1/6] staging: erofs: remove redundant CONFIG_EROFS_FS_XATTRs
@ 2018-09-19 5:49 Gao Xiang
2018-09-19 5:49 ` [PATCH 2/6] staging: erofs: fold in `__update_workgrp_llen' Gao Xiang
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Gao Xiang @ 2018-09-19 5:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Chao Yu, devel
Cc: LKML, linux-erofs, Miao Xie, Du Wei, Gao Xiang
some CONFIG_EROFS_FS_XATTR conditions were added because of
the historial Linux kernel compatibility, which are unneeded now.
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
Hi all,
These are cleanup patches in Chao's erofs-dev test tree for a while.
Many of them are trivial.
In order for all preview patches to keep up with the staging tree,
I resend them now.
Thanks,
Gao Xiang
drivers/staging/erofs/inode.c | 6 ------
drivers/staging/erofs/internal.h | 6 ++----
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
index c46a8d4..da8693a 100644
--- a/drivers/staging/erofs/inode.c
+++ b/drivers/staging/erofs/inode.c
@@ -260,22 +260,16 @@ struct inode *erofs_iget(struct super_block *sb,
const struct inode_operations erofs_generic_xattr_iops = {
.listxattr = erofs_listxattr,
};
-#endif
-#ifdef CONFIG_EROFS_FS_XATTR
const struct inode_operations erofs_symlink_xattr_iops = {
.get_link = page_get_link,
.listxattr = erofs_listxattr,
};
-#endif
const struct inode_operations erofs_special_inode_operations = {
-#ifdef CONFIG_EROFS_FS_XATTR
.listxattr = erofs_listxattr,
-#endif
};
-#ifdef CONFIG_EROFS_FS_XATTR
const struct inode_operations erofs_fast_symlink_xattr_iops = {
.get_link = simple_get_link,
.listxattr = erofs_listxattr,
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 0011b9d..cfcc6db 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -507,13 +507,11 @@ extern struct inode *erofs_iget(struct super_block *sb,
int erofs_namei(struct inode *dir, struct qstr *name,
erofs_nid_t *nid, unsigned *d_type);
-/* xattr.c */
#ifdef CONFIG_EROFS_FS_XATTR
+/* xattr.c */
extern const struct xattr_handler *erofs_xattr_handlers[];
-#endif
-/* symlink */
-#ifdef CONFIG_EROFS_FS_XATTR
+/* symlink and special inode */
extern const struct inode_operations erofs_symlink_xattr_iops;
extern const struct inode_operations erofs_fast_symlink_xattr_iops;
extern const struct inode_operations erofs_special_inode_operations;
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/6] staging: erofs: fold in `__update_workgrp_llen'
2018-09-19 5:49 [PATCH 1/6] staging: erofs: remove redundant CONFIG_EROFS_FS_XATTRs Gao Xiang
@ 2018-09-19 5:49 ` Gao Xiang
2018-09-19 15:14 ` Chao Yu
2018-09-19 5:49 ` [PATCH 3/6] staging: erofs: drop multiref support temporarily Gao Xiang
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Gao Xiang @ 2018-09-19 5:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Chao Yu, devel
Cc: LKML, linux-erofs, Miao Xie, Du Wei, Gao Xiang
There is the only one user to use `__update_workgrp_llen'.
Fold it in `z_erofs_vle_work_iter_begin' and cleanup related code.
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
drivers/staging/erofs/unzip_vle.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
index 2b9b313..e820490 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -422,18 +422,6 @@ struct z_erofs_vle_work_finder {
return work;
}
-static inline void __update_workgrp_llen(struct z_erofs_vle_workgroup *grp,
- unsigned int llen)
-{
- while (1) {
- unsigned int orig_llen = grp->llen;
-
- if (orig_llen >= llen || orig_llen ==
- cmpxchg(&grp->llen, orig_llen, llen))
- break;
- }
-}
-
#define builder_is_followed(builder) \
((builder)->role >= Z_EROFS_VLE_WORK_PRIMARY_FOLLOWED)
@@ -466,7 +454,13 @@ static int z_erofs_vle_work_iter_begin(struct z_erofs_vle_work_builder *builder,
repeat:
work = z_erofs_vle_work_lookup(&finder);
if (work != NULL) {
- __update_workgrp_llen(grp, map->m_llen);
+ unsigned int orig_llen;
+
+ /* increase workgroup `llen' if needed */
+ while ((orig_llen = READ_ONCE(grp->llen)) < map->m_llen &&
+ orig_llen != cmpxchg_relaxed(&grp->llen,
+ orig_llen, map->m_llen))
+ cpu_relax();
goto got_it;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/6] staging: erofs: drop multiref support temporarily
2018-09-19 5:49 [PATCH 1/6] staging: erofs: remove redundant CONFIG_EROFS_FS_XATTRs Gao Xiang
2018-09-19 5:49 ` [PATCH 2/6] staging: erofs: fold in `__update_workgrp_llen' Gao Xiang
@ 2018-09-19 5:49 ` Gao Xiang
2018-09-19 15:20 ` Chao Yu
2018-09-19 5:49 ` [PATCH 4/6] staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages' Gao Xiang
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Gao Xiang @ 2018-09-19 5:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Chao Yu, devel
Cc: LKML, linux-erofs, Miao Xie, Du Wei, Gao Xiang
Multiref support means that a compressed page could have
more than one reference, which is designed for on-disk data
deduplication. However, mkfs doesn't support this mode
at this moment, and the kernel implementation is also broken.
Let's drop multiref support. If it is fully implemented
in the future, it can be reverted later.
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
drivers/staging/erofs/unzip_vle.c | 36 +++++-------------------------------
drivers/staging/erofs/unzip_vle.h | 12 +-----------
2 files changed, 6 insertions(+), 42 deletions(-)
diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
index e820490..9fe18cd3 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -301,12 +301,9 @@ struct z_erofs_vle_work_finder {
grp = container_of(egrp, struct z_erofs_vle_workgroup, obj);
*f->grp_ret = grp;
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
work = z_erofs_vle_grab_work(grp, f->pageofs);
+ /* if multiref is disabled, `primary' is always true */
primary = true;
-#else
- BUG();
-#endif
DBG_BUGON(work->pageofs != f->pageofs);
@@ -368,12 +365,9 @@ struct z_erofs_vle_work_finder {
struct z_erofs_vle_workgroup *grp = *f->grp_ret;
struct z_erofs_vle_work *work;
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
+ /* if multiref is disabled, grp should never be nullptr */
BUG_ON(grp != NULL);
-#else
- if (grp != NULL)
- goto skip;
-#endif
+
/* no available workgroup, let's allocate one */
grp = kmem_cache_zalloc(z_erofs_workgroup_cachep, GFP_NOFS);
if (unlikely(grp == NULL))
@@ -396,13 +390,7 @@ struct z_erofs_vle_work_finder {
*f->hosted = true;
gnew = true;
-#ifdef CONFIG_EROFS_FS_ZIP_MULTIREF
-skip:
- /* currently unimplemented */
- BUG();
-#else
work = z_erofs_vle_grab_primary_work(grp);
-#endif
work->pageofs = f->pageofs;
mutex_init(&work->lock);
@@ -797,9 +785,7 @@ static int z_erofs_vle_unzip(struct super_block *sb,
struct z_erofs_pagevec_ctor ctor;
unsigned int nr_pages;
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
unsigned int sparsemem_pages = 0;
-#endif
struct page *pages_onstack[Z_EROFS_VLE_VMAP_ONSTACK_PAGES];
struct page **pages, **compressed_pages, *page;
unsigned int i, llen;
@@ -811,11 +797,7 @@ static int z_erofs_vle_unzip(struct super_block *sb,
int err;
might_sleep();
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
work = z_erofs_vle_grab_primary_work(grp);
-#else
- BUG();
-#endif
BUG_ON(!READ_ONCE(work->nr_pages));
mutex_lock(&work->lock);
@@ -866,13 +848,11 @@ static int z_erofs_vle_unzip(struct super_block *sb,
pagenr = z_erofs_onlinepage_index(page);
BUG_ON(pagenr >= nr_pages);
-
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
BUG_ON(pages[pagenr] != NULL);
- ++sparsemem_pages;
-#endif
+
pages[pagenr] = page;
}
+ sparsemem_pages = i;
z_erofs_pagevec_ctor_exit(&ctor, true);
@@ -902,10 +882,8 @@ static int z_erofs_vle_unzip(struct super_block *sb,
pagenr = z_erofs_onlinepage_index(page);
BUG_ON(pagenr >= nr_pages);
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
BUG_ON(pages[pagenr] != NULL);
++sparsemem_pages;
-#endif
pages[pagenr] = page;
overlapped = true;
@@ -931,12 +909,10 @@ static int z_erofs_vle_unzip(struct super_block *sb,
if (err != -ENOTSUPP)
goto out_percpu;
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
if (sparsemem_pages >= nr_pages) {
BUG_ON(sparsemem_pages > nr_pages);
goto skip_allocpage;
}
-#endif
for (i = 0; i < nr_pages; ++i) {
if (pages[i] != NULL)
@@ -945,9 +921,7 @@ static int z_erofs_vle_unzip(struct super_block *sb,
pages[i] = __stagingpage_alloc(page_pool, GFP_NOFS);
}
-#ifndef CONFIG_EROFS_FS_ZIP_MULTIREF
skip_allocpage:
-#endif
vout = erofs_vmap(pages, nr_pages);
err = z_erofs_vle_unzip_vmap(compressed_pages,
diff --git a/drivers/staging/erofs/unzip_vle.h b/drivers/staging/erofs/unzip_vle.h
index 3939985..3316bc3 100644
--- a/drivers/staging/erofs/unzip_vle.h
+++ b/drivers/staging/erofs/unzip_vle.h
@@ -47,13 +47,6 @@ static inline bool z_erofs_gather_if_stagingpage(struct list_head *page_pool,
#define Z_EROFS_VLE_INLINE_PAGEVECS 3
struct z_erofs_vle_work {
- /* struct z_erofs_vle_work *left, *right; */
-
-#ifdef CONFIG_EROFS_FS_ZIP_MULTIREF
- struct list_head list;
-
- atomic_t refcount;
-#endif
struct mutex lock;
/* I: decompression offset in page */
@@ -107,10 +100,8 @@ static inline void z_erofs_vle_set_workgrp_fmt(
grp->flags = fmt | (grp->flags & ~Z_EROFS_VLE_WORKGRP_FMT_MASK);
}
-#ifdef CONFIG_EROFS_FS_ZIP_MULTIREF
-#error multiref decompression is unimplemented yet
-#else
+/* definitions if multiref is disabled */
#define z_erofs_vle_grab_primary_work(grp) (&(grp)->work)
#define z_erofs_vle_grab_work(grp, pageofs) (&(grp)->work)
#define z_erofs_vle_work_workgroup(wrk, primary) \
@@ -118,7 +109,6 @@ static inline void z_erofs_vle_set_workgrp_fmt(
struct z_erofs_vle_workgroup, work) : \
({ BUG(); (void *)NULL; }))
-#endif
#define Z_EROFS_WORKGROUP_SIZE sizeof(struct z_erofs_vle_workgroup)
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/6] staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages'
2018-09-19 5:49 [PATCH 1/6] staging: erofs: remove redundant CONFIG_EROFS_FS_XATTRs Gao Xiang
2018-09-19 5:49 ` [PATCH 2/6] staging: erofs: fold in `__update_workgrp_llen' Gao Xiang
2018-09-19 5:49 ` [PATCH 3/6] staging: erofs: drop multiref support temporarily Gao Xiang
@ 2018-09-19 5:49 ` Gao Xiang
2018-09-19 15:26 ` Chao Yu
2018-09-19 16:06 ` [PATCH v2 " Gao Xiang
2018-09-19 5:49 ` [PATCH 5/6] staging: erofs: add some comments for xattr subsystem Gao Xiang
` (2 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Gao Xiang @ 2018-09-19 5:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Chao Yu, devel
Cc: LKML, linux-erofs, Miao Xie, Du Wei, Gao Xiang
This patch introduces `__should_decompress_synchronously' to
cleanup `z_erofs_vle_normalaccess_readpages'.
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
drivers/staging/erofs/internal.h | 11 +++++++++++
drivers/staging/erofs/super.c | 5 +++++
drivers/staging/erofs/unzip_vle.c | 20 ++++++--------------
3 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index cfcc6db..c84eb97 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -95,6 +95,9 @@ struct erofs_sb_info {
/* the dedicated workstation for compression */
struct radix_tree_root workstn_tree;
+ /* threshold for decompression synchronously */
+ unsigned int max_sync_decompress_pages;
+
#ifdef EROFS_FS_HAS_MANAGED_CACHE
struct inode *managed_cache;
#endif
@@ -273,6 +276,14 @@ extern int erofs_try_to_free_cached_page(struct address_space *mapping,
struct page *page);
#endif
+#define DEFAULT_MAX_SYNC_DECOMPRESS_PAGES 4
+
+static inline bool __should_decompress_synchronously(struct erofs_sb_info *sbi,
+ unsigned int nr)
+{
+ return nr <= sbi->max_sync_decompress_pages;
+}
+
#endif
/* we strictly follow PAGE_SIZE and no buffer head yet */
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 802202c..5b87f59 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -162,6 +162,11 @@ static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
static void default_options(struct erofs_sb_info *sbi)
{
+ /* set up some FS parameters */
+#ifdef CONFIG_EROFS_FS_ZIP
+ sbi->max_sync_decompress_pages = DEFAULT_MAX_SYNC_DECOMPRESS_PAGES;
+#endif
+
#ifdef CONFIG_EROFS_FS_XATTR
set_opt(sbi, XATTR_USER);
#endif
diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
index 9fe18cd3..337a82d 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -1308,12 +1308,14 @@ static int z_erofs_vle_normalaccess_readpage(struct file *file,
return 0;
}
-static inline int __z_erofs_vle_normalaccess_readpages(
- struct file *filp,
- struct address_space *mapping,
- struct list_head *pages, unsigned int nr_pages, bool sync)
+static int z_erofs_vle_normalaccess_readpages(struct file *filp,
+ struct address_space *mapping,
+ struct list_head *pages,
+ unsigned int nr_pages)
{
struct inode *const inode = mapping->host;
+ struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
+ const bool sync = __should_decompress_synchronously(sbi, nr_pages);
struct z_erofs_vle_frontend f = VLE_FRONTEND_INIT(inode);
gfp_t gfp = mapping_gfp_constraint(mapping, GFP_KERNEL);
@@ -1372,16 +1374,6 @@ static inline int __z_erofs_vle_normalaccess_readpages(
return 0;
}
-static int z_erofs_vle_normalaccess_readpages(
- struct file *filp,
- struct address_space *mapping,
- struct list_head *pages, unsigned int nr_pages)
-{
- return __z_erofs_vle_normalaccess_readpages(filp,
- mapping, pages, nr_pages,
- nr_pages < 4 /* sync */);
-}
-
const struct address_space_operations z_erofs_vle_normalaccess_aops = {
.readpage = z_erofs_vle_normalaccess_readpage,
.readpages = z_erofs_vle_normalaccess_readpages,
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/6] staging: erofs: add some comments for xattr subsystem
2018-09-19 5:49 [PATCH 1/6] staging: erofs: remove redundant CONFIG_EROFS_FS_XATTRs Gao Xiang
` (2 preceding siblings ...)
2018-09-19 5:49 ` [PATCH 4/6] staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages' Gao Xiang
@ 2018-09-19 5:49 ` Gao Xiang
2018-09-19 15:27 ` Chao Yu
2018-09-19 5:49 ` [PATCH 6/6] staging: erofs: simplify return value of `xattr_foreach' Gao Xiang
2018-09-19 15:07 ` [PATCH 1/6] staging: erofs: remove redundant CONFIG_EROFS_FS_XATTRs Chao Yu
5 siblings, 1 reply; 18+ messages in thread
From: Gao Xiang @ 2018-09-19 5:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Chao Yu, devel
Cc: LKML, linux-erofs, Miao Xie, Du Wei, Gao Xiang
As Dan Carpenter pointed out, it is better to document what
return values of these callbacks in `struct xattr_iter_handlers'
mean and why it->ofs is increased regardless of success or
failure in `xattr_foreach'.
Suggested-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
drivers/staging/erofs/xattr.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
index 4942ca1..7b1367e 100644
--- a/drivers/staging/erofs/xattr.c
+++ b/drivers/staging/erofs/xattr.c
@@ -109,6 +109,13 @@ static int init_inode_xattrs(struct inode *inode)
return 0;
}
+/*
+ * the general idea for these return values is
+ * if 0 is returned, go on processing the current xattr;
+ * 1 (> 0) is returned, skip this round to process the next xattr;
+ * -err (< 0) is returned, an error (maybe ENOXATTR) occurred
+ * and need to be handled
+ */
struct xattr_iter_handlers {
int (*entry)(struct xattr_iter *, struct erofs_xattr_entry *);
int (*name)(struct xattr_iter *, unsigned int, char *, unsigned int);
@@ -164,6 +171,10 @@ static int inline_xattr_iter_begin(struct xattr_iter *it,
return vi->xattr_isize - xattr_header_sz;
}
+/*
+ * Regardless of success or failure, `xattr_foreach' will end up with
+ * `ofs' pointing to the next xattr item rather than an arbitrary position.
+ */
static int xattr_foreach(struct xattr_iter *it,
const struct xattr_iter_handlers *op, unsigned int *tlimit)
{
@@ -255,7 +266,7 @@ static int xattr_foreach(struct xattr_iter *it,
}
out:
- /* we assume that ofs is aligned with 4 bytes */
+ /* xattrs should be 4-byte aligned (on-disk constraint) */
it->ofs = EROFS_XATTR_ALIGN(it->ofs);
return err;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 6/6] staging: erofs: simplify return value of `xattr_foreach'
2018-09-19 5:49 [PATCH 1/6] staging: erofs: remove redundant CONFIG_EROFS_FS_XATTRs Gao Xiang
` (3 preceding siblings ...)
2018-09-19 5:49 ` [PATCH 5/6] staging: erofs: add some comments for xattr subsystem Gao Xiang
@ 2018-09-19 5:49 ` Gao Xiang
2018-09-19 15:28 ` Chao Yu
2018-09-19 15:07 ` [PATCH 1/6] staging: erofs: remove redundant CONFIG_EROFS_FS_XATTRs Chao Yu
5 siblings, 1 reply; 18+ messages in thread
From: Gao Xiang @ 2018-09-19 5:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Chao Yu, devel
Cc: LKML, linux-erofs, Miao Xie, Du Wei, Gao Xiang
As Dan Carpenter pointed out, there is no need to propagate positive
return values back to its callers.
Suggested-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
drivers/staging/erofs/xattr.c | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
index 7b1367e..80dca6a 100644
--- a/drivers/staging/erofs/xattr.c
+++ b/drivers/staging/erofs/xattr.c
@@ -268,7 +268,7 @@ static int xattr_foreach(struct xattr_iter *it,
out:
/* xattrs should be 4-byte aligned (on-disk constraint) */
it->ofs = EROFS_XATTR_ALIGN(it->ofs);
- return err;
+ return err < 0 ? err : 0;
}
struct getxattr_iter {
@@ -333,15 +333,12 @@ static int inline_getxattr(struct inode *inode, struct getxattr_iter *it)
remaining = ret;
while (remaining) {
ret = xattr_foreach(&it->it, &find_xattr_handlers, &remaining);
- if (ret >= 0)
- break;
-
- if (ret != -ENOATTR) /* -ENOMEM, -EIO, etc. */
+ if (ret != -ENOATTR)
break;
}
xattr_iter_end_final(&it->it);
- return ret < 0 ? ret : it->buffer_size;
+ return ret ? ret : it->buffer_size;
}
static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
@@ -371,16 +368,13 @@ static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
}
ret = xattr_foreach(&it->it, &find_xattr_handlers, NULL);
- if (ret >= 0)
- break;
-
- if (ret != -ENOATTR) /* -ENOMEM, -EIO, etc. */
+ if (ret != -ENOATTR)
break;
}
if (vi->xattr_shared_count)
xattr_iter_end_final(&it->it);
- return ret < 0 ? ret : it->buffer_size;
+ return ret ? ret : it->buffer_size;
}
static bool erofs_xattr_user_list(struct dentry *dentry)
@@ -567,11 +561,11 @@ static int inline_listxattr(struct listxattr_iter *it)
remaining = ret;
while (remaining) {
ret = xattr_foreach(&it->it, &list_xattr_handlers, &remaining);
- if (ret < 0)
+ if (ret)
break;
}
xattr_iter_end_final(&it->it);
- return ret < 0 ? ret : it->buffer_ofs;
+ return ret ? ret : it->buffer_ofs;
}
static int shared_listxattr(struct listxattr_iter *it)
@@ -601,13 +595,13 @@ static int shared_listxattr(struct listxattr_iter *it)
}
ret = xattr_foreach(&it->it, &list_xattr_handlers, NULL);
- if (ret < 0)
+ if (ret)
break;
}
if (vi->xattr_shared_count)
xattr_iter_end_final(&it->it);
- return ret < 0 ? ret : it->buffer_ofs;
+ return ret ? ret : it->buffer_ofs;
}
ssize_t erofs_listxattr(struct dentry *dentry,
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/6] staging: erofs: remove redundant CONFIG_EROFS_FS_XATTRs
2018-09-19 5:49 [PATCH 1/6] staging: erofs: remove redundant CONFIG_EROFS_FS_XATTRs Gao Xiang
` (4 preceding siblings ...)
2018-09-19 5:49 ` [PATCH 6/6] staging: erofs: simplify return value of `xattr_foreach' Gao Xiang
@ 2018-09-19 15:07 ` Chao Yu
5 siblings, 0 replies; 18+ messages in thread
From: Chao Yu @ 2018-09-19 15:07 UTC (permalink / raw)
To: Gao Xiang, Greg Kroah-Hartman, Chao Yu, devel
Cc: Miao Xie, linux-erofs, LKML, Du Wei
On 2018/9/19 13:49, Gao Xiang wrote:
> some CONFIG_EROFS_FS_XATTR conditions were added because of
> the historial Linux kernel compatibility, which are unneeded now.
>
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Thanks,
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/6] staging: erofs: fold in `__update_workgrp_llen'
2018-09-19 5:49 ` [PATCH 2/6] staging: erofs: fold in `__update_workgrp_llen' Gao Xiang
@ 2018-09-19 15:14 ` Chao Yu
0 siblings, 0 replies; 18+ messages in thread
From: Chao Yu @ 2018-09-19 15:14 UTC (permalink / raw)
To: Gao Xiang, Greg Kroah-Hartman, Chao Yu, devel
Cc: Miao Xie, linux-erofs, LKML, Du Wei
On 2018/9/19 13:49, Gao Xiang wrote:
> There is the only one user to use `__update_workgrp_llen'.
> Fold it in `z_erofs_vle_work_iter_begin' and cleanup related code.
>
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Thanks,
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/6] staging: erofs: drop multiref support temporarily
2018-09-19 5:49 ` [PATCH 3/6] staging: erofs: drop multiref support temporarily Gao Xiang
@ 2018-09-19 15:20 ` Chao Yu
0 siblings, 0 replies; 18+ messages in thread
From: Chao Yu @ 2018-09-19 15:20 UTC (permalink / raw)
To: Gao Xiang, Greg Kroah-Hartman, Chao Yu, devel
Cc: Miao Xie, linux-erofs, LKML, Du Wei
On 2018/9/19 13:49, Gao Xiang wrote:
> Multiref support means that a compressed page could have
> more than one reference, which is designed for on-disk data
> deduplication. However, mkfs doesn't support this mode
> at this moment, and the kernel implementation is also broken.
>
> Let's drop multiref support. If it is fully implemented
> in the future, it can be reverted later.
>
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Thanks,
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/6] staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages'
2018-09-19 5:49 ` [PATCH 4/6] staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages' Gao Xiang
@ 2018-09-19 15:26 ` Chao Yu
2018-09-19 15:32 ` Gao Xiang
2018-09-19 16:06 ` [PATCH v2 " Gao Xiang
1 sibling, 1 reply; 18+ messages in thread
From: Chao Yu @ 2018-09-19 15:26 UTC (permalink / raw)
To: Gao Xiang, Greg Kroah-Hartman, Chao Yu, devel
Cc: Miao Xie, linux-erofs, LKML, Du Wei
Hi Xiang,
On 2018/9/19 13:49, Gao Xiang wrote:
> This patch introduces `__should_decompress_synchronously' to
> cleanup `z_erofs_vle_normalaccess_readpages'.
>
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
> ---
> drivers/staging/erofs/internal.h | 11 +++++++++++
> drivers/staging/erofs/super.c | 5 +++++
> drivers/staging/erofs/unzip_vle.c | 20 ++++++--------------
> 3 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
> index cfcc6db..c84eb97 100644
> --- a/drivers/staging/erofs/internal.h
> +++ b/drivers/staging/erofs/internal.h
> @@ -95,6 +95,9 @@ struct erofs_sb_info {
> /* the dedicated workstation for compression */
> struct radix_tree_root workstn_tree;
>
> + /* threshold for decompression synchronously */
> + unsigned int max_sync_decompress_pages;
> +
> #ifdef EROFS_FS_HAS_MANAGED_CACHE
> struct inode *managed_cache;
> #endif
> @@ -273,6 +276,14 @@ extern int erofs_try_to_free_cached_page(struct address_space *mapping,
> struct page *page);
> #endif
>
> +#define DEFAULT_MAX_SYNC_DECOMPRESS_PAGES 4
> +
> +static inline bool __should_decompress_synchronously(struct erofs_sb_info *sbi,
> + unsigned int nr)
> +{
> + return nr <= sbi->max_sync_decompress_pages;
- nr_pages < 4 /* sync */);
There is a little bit changed except cleanup, IIUC, would it be any difference
of performance around boundary of four pages?
Thanks,
> +}
> +
> #endif
>
> /* we strictly follow PAGE_SIZE and no buffer head yet */
> diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
> index 802202c..5b87f59 100644
> --- a/drivers/staging/erofs/super.c
> +++ b/drivers/staging/erofs/super.c
> @@ -162,6 +162,11 @@ static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
>
> static void default_options(struct erofs_sb_info *sbi)
> {
> + /* set up some FS parameters */
> +#ifdef CONFIG_EROFS_FS_ZIP
> + sbi->max_sync_decompress_pages = DEFAULT_MAX_SYNC_DECOMPRESS_PAGES;
> +#endif
> +
> #ifdef CONFIG_EROFS_FS_XATTR
> set_opt(sbi, XATTR_USER);
> #endif
> diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
> index 9fe18cd3..337a82d 100644
> --- a/drivers/staging/erofs/unzip_vle.c
> +++ b/drivers/staging/erofs/unzip_vle.c
> @@ -1308,12 +1308,14 @@ static int z_erofs_vle_normalaccess_readpage(struct file *file,
> return 0;
> }
>
> -static inline int __z_erofs_vle_normalaccess_readpages(
> - struct file *filp,
> - struct address_space *mapping,
> - struct list_head *pages, unsigned int nr_pages, bool sync)
> +static int z_erofs_vle_normalaccess_readpages(struct file *filp,
> + struct address_space *mapping,
> + struct list_head *pages,
> + unsigned int nr_pages)
> {
> struct inode *const inode = mapping->host;
> + struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
> + const bool sync = __should_decompress_synchronously(sbi, nr_pages);
>
> struct z_erofs_vle_frontend f = VLE_FRONTEND_INIT(inode);
> gfp_t gfp = mapping_gfp_constraint(mapping, GFP_KERNEL);
> @@ -1372,16 +1374,6 @@ static inline int __z_erofs_vle_normalaccess_readpages(
> return 0;
> }
>
> -static int z_erofs_vle_normalaccess_readpages(
> - struct file *filp,
> - struct address_space *mapping,
> - struct list_head *pages, unsigned int nr_pages)
> -{
> - return __z_erofs_vle_normalaccess_readpages(filp,
> - mapping, pages, nr_pages,
> - nr_pages < 4 /* sync */);
> -}
> -
> const struct address_space_operations z_erofs_vle_normalaccess_aops = {
> .readpage = z_erofs_vle_normalaccess_readpage,
> .readpages = z_erofs_vle_normalaccess_readpages,
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] staging: erofs: add some comments for xattr subsystem
2018-09-19 5:49 ` [PATCH 5/6] staging: erofs: add some comments for xattr subsystem Gao Xiang
@ 2018-09-19 15:27 ` Chao Yu
0 siblings, 0 replies; 18+ messages in thread
From: Chao Yu @ 2018-09-19 15:27 UTC (permalink / raw)
To: Gao Xiang, Greg Kroah-Hartman, Chao Yu, devel
Cc: Miao Xie, linux-erofs, LKML, Du Wei
On 2018/9/19 13:49, Gao Xiang wrote:
> As Dan Carpenter pointed out, it is better to document what
> return values of these callbacks in `struct xattr_iter_handlers'
> mean and why it->ofs is increased regardless of success or
> failure in `xattr_foreach'.
>
> Suggested-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
> ---
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Thanks,
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] staging: erofs: simplify return value of `xattr_foreach'
2018-09-19 5:49 ` [PATCH 6/6] staging: erofs: simplify return value of `xattr_foreach' Gao Xiang
@ 2018-09-19 15:28 ` Chao Yu
0 siblings, 0 replies; 18+ messages in thread
From: Chao Yu @ 2018-09-19 15:28 UTC (permalink / raw)
To: Gao Xiang, Greg Kroah-Hartman, Chao Yu, devel
Cc: Miao Xie, linux-erofs, LKML, Du Wei
On 2018/9/19 13:49, Gao Xiang wrote:
> As Dan Carpenter pointed out, there is no need to propagate positive
> return values back to its callers.
>
> Suggested-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Thanks,
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/6] staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages'
2018-09-19 15:26 ` Chao Yu
@ 2018-09-19 15:32 ` Gao Xiang
2018-09-19 15:40 ` Gao Xiang
2018-09-19 15:45 ` Chao Yu
0 siblings, 2 replies; 18+ messages in thread
From: Gao Xiang @ 2018-09-19 15:32 UTC (permalink / raw)
To: Chao Yu, Chao Yu, devel
Cc: Gao Xiang, Greg Kroah-Hartman, linux-erofs, Miao Xie, LKML, Du Wei
Hi Chao,
On 2018/9/19 23:26, Chao Yu wrote:
> Hi Xiang,
>
> On 2018/9/19 13:49, Gao Xiang wrote:
>> This patch introduces `__should_decompress_synchronously' to
>> cleanup `z_erofs_vle_normalaccess_readpages'.
>>
>> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
>> ---
>> drivers/staging/erofs/internal.h | 11 +++++++++++
>> drivers/staging/erofs/super.c | 5 +++++
>> drivers/staging/erofs/unzip_vle.c | 20 ++++++--------------
>> 3 files changed, 22 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
>> index cfcc6db..c84eb97 100644
>> --- a/drivers/staging/erofs/internal.h
>> +++ b/drivers/staging/erofs/internal.h
>> @@ -95,6 +95,9 @@ struct erofs_sb_info {
>> /* the dedicated workstation for compression */
>> struct radix_tree_root workstn_tree;
>>
>> + /* threshold for decompression synchronously */
>> + unsigned int max_sync_decompress_pages;
>> +
>> #ifdef EROFS_FS_HAS_MANAGED_CACHE
>> struct inode *managed_cache;
>> #endif
>> @@ -273,6 +276,14 @@ extern int erofs_try_to_free_cached_page(struct address_space *mapping,
>> struct page *page);
>> #endif
>>
>> +#define DEFAULT_MAX_SYNC_DECOMPRESS_PAGES 4
>> +
>> +static inline bool __should_decompress_synchronously(struct erofs_sb_info *sbi,
>> + unsigned int nr)
>> +{
>> + return nr <= sbi->max_sync_decompress_pages;
> - nr_pages < 4 /* sync */);
>
> There is a little bit changed except cleanup, IIUC, would it be any difference
> of performance around boundary of four pages?
No.. Once synchronous decompression is applied for 1,2,3 pages for no special reasons.
But I think it could be better to adjust it to the power of two --- 1,2,3,4 is preferred.
Since I have no idea to measure which is better or what value is best for all platform or use cases...
Therefore I tune it in this patch since I don't like the number
DEFAULT_MAX_SYNC_DECOMPRESS_PAGES == 3 ...
Thanks,
Gao Xiang
>
> Thanks,
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/6] staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages'
2018-09-19 15:32 ` Gao Xiang
@ 2018-09-19 15:40 ` Gao Xiang
2018-09-19 15:45 ` Chao Yu
1 sibling, 0 replies; 18+ messages in thread
From: Gao Xiang @ 2018-09-19 15:40 UTC (permalink / raw)
To: Chao Yu, Chao Yu
Cc: devel, Greg Kroah-Hartman, Miao Xie, LKML, Du Wei, linux-erofs
Hi Chao,
On 2018/9/19 23:32, Gao Xiang via Linux-erofs wrote:
> Hi Chao,
>
> On 2018/9/19 23:26, Chao Yu wrote:
>> Hi Xiang,
>>
>> On 2018/9/19 13:49, Gao Xiang wrote:
>>> This patch introduces `__should_decompress_synchronously' to
>>> cleanup `z_erofs_vle_normalaccess_readpages'.
>>>
>>> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
>>> ---
>>> drivers/staging/erofs/internal.h | 11 +++++++++++
>>> drivers/staging/erofs/super.c | 5 +++++
>>> drivers/staging/erofs/unzip_vle.c | 20 ++++++--------------
>>> 3 files changed, 22 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
>>> index cfcc6db..c84eb97 100644
>>> --- a/drivers/staging/erofs/internal.h
>>> +++ b/drivers/staging/erofs/internal.h
>>> @@ -95,6 +95,9 @@ struct erofs_sb_info {
>>> /* the dedicated workstation for compression */
>>> struct radix_tree_root workstn_tree;
>>>
>>> + /* threshold for decompression synchronously */
>>> + unsigned int max_sync_decompress_pages;
>>> +
>>> #ifdef EROFS_FS_HAS_MANAGED_CACHE
>>> struct inode *managed_cache;
>>> #endif
>>> @@ -273,6 +276,14 @@ extern int erofs_try_to_free_cached_page(struct address_space *mapping,
>>> struct page *page);
>>> #endif
>>>
>>> +#define DEFAULT_MAX_SYNC_DECOMPRESS_PAGES 4
>>> +
>>> +static inline bool __should_decompress_synchronously(struct erofs_sb_info *sbi,
>>> + unsigned int nr)
>>> +{
>>> + return nr <= sbi->max_sync_decompress_pages;
>> - nr_pages < 4 /* sync */);
>>
>> There is a little bit changed except cleanup, IIUC, would it be any difference
>> of performance around boundary of four pages?
>
> No.. Once synchronous decompression is applied for 1,2,3 pages for no special reasons.
> But I think it could be better to adjust it to the power of two --- 1,2,3,4 is preferred.
> Since I have no idea to measure which is better or what value is best for all platform or use cases...
>
> Therefore I tune it in this patch since I don't like the number
> DEFAULT_MAX_SYNC_DECOMPRESS_PAGES == 3 ...
p.s. If you think that is not the cleanup meaning, I will resend this patch with
the original 3. Both are ok for me. it is a minor tuning and I don't have a way
to measure which is better and the best boundary.
Thanks,
Gao Xiang
>
> Thanks,
> Gao Xiang
>
>>
>> Thanks,
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/6] staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages'
2018-09-19 15:32 ` Gao Xiang
2018-09-19 15:40 ` Gao Xiang
@ 2018-09-19 15:45 ` Chao Yu
2018-09-19 15:51 ` Gao Xiang
1 sibling, 1 reply; 18+ messages in thread
From: Chao Yu @ 2018-09-19 15:45 UTC (permalink / raw)
To: Gao Xiang, Chao Yu, devel
Cc: Gao Xiang, Greg Kroah-Hartman, linux-erofs, Miao Xie, LKML, Du Wei
Hi Xiang,
On 2018/9/19 23:32, Gao Xiang wrote:
> Hi Chao,
>
> On 2018/9/19 23:26, Chao Yu wrote:
>> Hi Xiang,
>>
>> On 2018/9/19 13:49, Gao Xiang wrote:
>>> This patch introduces `__should_decompress_synchronously' to
>>> cleanup `z_erofs_vle_normalaccess_readpages'.
>>>
>>> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
>>> ---
>>> drivers/staging/erofs/internal.h | 11 +++++++++++
>>> drivers/staging/erofs/super.c | 5 +++++
>>> drivers/staging/erofs/unzip_vle.c | 20 ++++++--------------
>>> 3 files changed, 22 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
>>> index cfcc6db..c84eb97 100644
>>> --- a/drivers/staging/erofs/internal.h
>>> +++ b/drivers/staging/erofs/internal.h
>>> @@ -95,6 +95,9 @@ struct erofs_sb_info {
>>> /* the dedicated workstation for compression */
>>> struct radix_tree_root workstn_tree;
>>>
>>> + /* threshold for decompression synchronously */
>>> + unsigned int max_sync_decompress_pages;
>>> +
>>> #ifdef EROFS_FS_HAS_MANAGED_CACHE
>>> struct inode *managed_cache;
>>> #endif
>>> @@ -273,6 +276,14 @@ extern int erofs_try_to_free_cached_page(struct address_space *mapping,
>>> struct page *page);
>>> #endif
>>>
>>> +#define DEFAULT_MAX_SYNC_DECOMPRESS_PAGES 4
>>> +
>>> +static inline bool __should_decompress_synchronously(struct erofs_sb_info *sbi,
>>> + unsigned int nr)
>>> +{
>>> + return nr <= sbi->max_sync_decompress_pages;
>> - nr_pages < 4 /* sync */);
>>
>> There is a little bit changed except cleanup, IIUC, would it be any difference
>> of performance around boundary of four pages?
>
> No.. Once synchronous decompression is applied for 1,2,3 pages for no special reasons.
> But I think it could be better to adjust it to the power of two --- 1,2,3,4 is preferred.
> Since I have no idea to measure which is better or what value is best for all platform or use cases...
>
> Therefore I tune it in this patch since I don't like the number
> DEFAULT_MAX_SYNC_DECOMPRESS_PAGES == 3 ...
Yeah, how about adding one more patch to tune this magic number first? as that
change would not be part of cleanup thing. I guess later maybe we could export a
sysfs node to provider tuning ability on that threshold.
Thanks,
>
> Thanks,
> Gao Xiang
>
>>
>> Thanks,
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/6] staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages'
2018-09-19 15:45 ` Chao Yu
@ 2018-09-19 15:51 ` Gao Xiang
0 siblings, 0 replies; 18+ messages in thread
From: Gao Xiang @ 2018-09-19 15:51 UTC (permalink / raw)
To: Chao Yu, Chao Yu
Cc: devel, Gao Xiang, Greg Kroah-Hartman, linux-erofs, Miao Xie,
LKML, Du Wei
Hi Chao,
On 2018/9/19 23:45, Chao Yu wrote:
> Hi Xiang,
>
> On 2018/9/19 23:32, Gao Xiang wrote:
>> Hi Chao,
>>
>> On 2018/9/19 23:26, Chao Yu wrote:
>>> Hi Xiang,
>>>
>>> On 2018/9/19 13:49, Gao Xiang wrote:
>>>> This patch introduces `__should_decompress_synchronously' to
>>>> cleanup `z_erofs_vle_normalaccess_readpages'.
>>>>
>>>> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
>>>> ---
>>>> drivers/staging/erofs/internal.h | 11 +++++++++++
>>>> drivers/staging/erofs/super.c | 5 +++++
>>>> drivers/staging/erofs/unzip_vle.c | 20 ++++++--------------
>>>> 3 files changed, 22 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
>>>> index cfcc6db..c84eb97 100644
>>>> --- a/drivers/staging/erofs/internal.h
>>>> +++ b/drivers/staging/erofs/internal.h
>>>> @@ -95,6 +95,9 @@ struct erofs_sb_info {
>>>> /* the dedicated workstation for compression */
>>>> struct radix_tree_root workstn_tree;
>>>>
>>>> + /* threshold for decompression synchronously */
>>>> + unsigned int max_sync_decompress_pages;
>>>> +
>>>> #ifdef EROFS_FS_HAS_MANAGED_CACHE
>>>> struct inode *managed_cache;
>>>> #endif
>>>> @@ -273,6 +276,14 @@ extern int erofs_try_to_free_cached_page(struct address_space *mapping,
>>>> struct page *page);
>>>> #endif
>>>>
>>>> +#define DEFAULT_MAX_SYNC_DECOMPRESS_PAGES 4
>>>> +
>>>> +static inline bool __should_decompress_synchronously(struct erofs_sb_info *sbi,
>>>> + unsigned int nr)
>>>> +{
>>>> + return nr <= sbi->max_sync_decompress_pages;
>>> - nr_pages < 4 /* sync */);
>>>
>>> There is a little bit changed except cleanup, IIUC, would it be any difference
>>> of performance around boundary of four pages?
>>
>> No.. Once synchronous decompression is applied for 1,2,3 pages for no special reasons.
>> But I think it could be better to adjust it to the power of two --- 1,2,3,4 is preferred.
>> Since I have no idea to measure which is better or what value is best for all platform or use cases...
>>
>> Therefore I tune it in this patch since I don't like the number
>> DEFAULT_MAX_SYNC_DECOMPRESS_PAGES == 3 ...
>
> Yeah, how about adding one more patch to tune this magic number first? as that
> change would not be part of cleanup thing. I guess later maybe we could export a
> sysfs node to provider tuning ability on that threshold.
OK, I think we can leave the original "3" for now. I will fix that value and resend this patch soon.
The sysfs interface could be necessary later since it seems some arguments
involved in decompression could impact the decompression performance.
Thanks,
Gao Xiang
>
> Thanks,
>
>>
>> Thanks,
>> Gao Xiang
>>
>>>
>>> Thanks,
>>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 4/6] staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages'
2018-09-19 5:49 ` [PATCH 4/6] staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages' Gao Xiang
2018-09-19 15:26 ` Chao Yu
@ 2018-09-19 16:06 ` Gao Xiang
2018-09-20 1:29 ` Chao Yu
1 sibling, 1 reply; 18+ messages in thread
From: Gao Xiang @ 2018-09-19 16:06 UTC (permalink / raw)
To: Greg Kroah-Hartman, Chao Yu, devel; +Cc: LKML, linux-erofs, Miao Xie, Gao Xiang
From: Gao Xiang <gaoxiang25@huawei.com>
This patch introduces `__should_decompress_synchronously' to
cleanup `z_erofs_vle_normalaccess_readpages'.
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
change log v2:
- Leave the original threshold "3" for DEFAULT_MAX_SYNC_DECOMPRESS_PAGES
and just do the cleanup work.
drivers/staging/erofs/internal.h | 11 +++++++++++
drivers/staging/erofs/super.c | 5 +++++
drivers/staging/erofs/unzip_vle.c | 20 ++++++--------------
3 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index cfcc6db..c2cc4a016 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -95,6 +95,9 @@ struct erofs_sb_info {
/* the dedicated workstation for compression */
struct radix_tree_root workstn_tree;
+ /* threshold for decompression synchronously */
+ unsigned int max_sync_decompress_pages;
+
#ifdef EROFS_FS_HAS_MANAGED_CACHE
struct inode *managed_cache;
#endif
@@ -273,6 +276,14 @@ extern int erofs_try_to_free_cached_page(struct address_space *mapping,
struct page *page);
#endif
+#define DEFAULT_MAX_SYNC_DECOMPRESS_PAGES 3
+
+static inline bool __should_decompress_synchronously(struct erofs_sb_info *sbi,
+ unsigned int nr)
+{
+ return nr <= sbi->max_sync_decompress_pages;
+}
+
#endif
/* we strictly follow PAGE_SIZE and no buffer head yet */
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 802202c..5b87f59 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -162,6 +162,11 @@ static void erofs_build_fault_attr(struct erofs_sb_info *sbi,
static void default_options(struct erofs_sb_info *sbi)
{
+ /* set up some FS parameters */
+#ifdef CONFIG_EROFS_FS_ZIP
+ sbi->max_sync_decompress_pages = DEFAULT_MAX_SYNC_DECOMPRESS_PAGES;
+#endif
+
#ifdef CONFIG_EROFS_FS_XATTR
set_opt(sbi, XATTR_USER);
#endif
diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
index 9fe18cd3..337a82d 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -1308,12 +1308,14 @@ static int z_erofs_vle_normalaccess_readpage(struct file *file,
return 0;
}
-static inline int __z_erofs_vle_normalaccess_readpages(
- struct file *filp,
- struct address_space *mapping,
- struct list_head *pages, unsigned int nr_pages, bool sync)
+static int z_erofs_vle_normalaccess_readpages(struct file *filp,
+ struct address_space *mapping,
+ struct list_head *pages,
+ unsigned int nr_pages)
{
struct inode *const inode = mapping->host;
+ struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
+ const bool sync = __should_decompress_synchronously(sbi, nr_pages);
struct z_erofs_vle_frontend f = VLE_FRONTEND_INIT(inode);
gfp_t gfp = mapping_gfp_constraint(mapping, GFP_KERNEL);
@@ -1372,16 +1374,6 @@ static inline int __z_erofs_vle_normalaccess_readpages(
return 0;
}
-static int z_erofs_vle_normalaccess_readpages(
- struct file *filp,
- struct address_space *mapping,
- struct list_head *pages, unsigned int nr_pages)
-{
- return __z_erofs_vle_normalaccess_readpages(filp,
- mapping, pages, nr_pages,
- nr_pages < 4 /* sync */);
-}
-
const struct address_space_operations z_erofs_vle_normalaccess_aops = {
.readpage = z_erofs_vle_normalaccess_readpage,
.readpages = z_erofs_vle_normalaccess_readpages,
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 4/6] staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages'
2018-09-19 16:06 ` [PATCH v2 " Gao Xiang
@ 2018-09-20 1:29 ` Chao Yu
0 siblings, 0 replies; 18+ messages in thread
From: Chao Yu @ 2018-09-20 1:29 UTC (permalink / raw)
To: Gao Xiang, Greg Kroah-Hartman, devel
Cc: LKML, linux-erofs, Miao Xie, Gao Xiang
On 2018/9/20 0:06, Gao Xiang wrote:
> From: Gao Xiang <gaoxiang25@huawei.com>
>
> This patch introduces `__should_decompress_synchronously' to
> cleanup `z_erofs_vle_normalaccess_readpages'.
>
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Thanks,
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-09-20 1:29 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-19 5:49 [PATCH 1/6] staging: erofs: remove redundant CONFIG_EROFS_FS_XATTRs Gao Xiang
2018-09-19 5:49 ` [PATCH 2/6] staging: erofs: fold in `__update_workgrp_llen' Gao Xiang
2018-09-19 15:14 ` Chao Yu
2018-09-19 5:49 ` [PATCH 3/6] staging: erofs: drop multiref support temporarily Gao Xiang
2018-09-19 15:20 ` Chao Yu
2018-09-19 5:49 ` [PATCH 4/6] staging: erofs: cleanup `z_erofs_vle_normalaccess_readpages' Gao Xiang
2018-09-19 15:26 ` Chao Yu
2018-09-19 15:32 ` Gao Xiang
2018-09-19 15:40 ` Gao Xiang
2018-09-19 15:45 ` Chao Yu
2018-09-19 15:51 ` Gao Xiang
2018-09-19 16:06 ` [PATCH v2 " Gao Xiang
2018-09-20 1:29 ` Chao Yu
2018-09-19 5:49 ` [PATCH 5/6] staging: erofs: add some comments for xattr subsystem Gao Xiang
2018-09-19 15:27 ` Chao Yu
2018-09-19 5:49 ` [PATCH 6/6] staging: erofs: simplify return value of `xattr_foreach' Gao Xiang
2018-09-19 15:28 ` Chao Yu
2018-09-19 15:07 ` [PATCH 1/6] staging: erofs: remove redundant CONFIG_EROFS_FS_XATTRs Chao Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).