linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] staging: erofs: sunset erofs_workstation_cleanup_all
@ 2019-01-16  8:59 Gao Xiang
  2019-01-16  8:59 ` [PATCH 2/5] staging: erofs: localize erofs_workgroup_get Gao Xiang
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Gao Xiang @ 2019-01-16  8:59 UTC (permalink / raw)
  To: Chao Yu, Greg Kroah-Hartman, devel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei, Gao Xiang

There is only one user calling erofs_workstation_cleanup_all,
and it is no likely that more users will use in that way
in the future.

Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
Hi,

 These 5 patches are all cleanup patches, no logic change.

Thanks,
Gao Xiang

 drivers/staging/erofs/internal.h | 5 -----
 drivers/staging/erofs/super.c    | 3 ++-
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index c3de24e7fb67..80cc09ea787d 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -283,11 +283,6 @@ extern int erofs_register_workgroup(struct super_block *sb,
 extern unsigned long erofs_shrink_workstation(struct erofs_sb_info *sbi,
 	unsigned long nr_shrink, bool cleanup);
 
-static inline void erofs_workstation_cleanup_all(struct super_block *sb)
-{
-	erofs_shrink_workstation(EROFS_SB(sb), ~0UL, true);
-}
-
 extern void erofs_workgroup_free_rcu(struct erofs_workgroup *grp);
 
 #ifdef EROFS_FS_HAS_MANAGED_CACHE
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 45c7f6ddb9f5..69e1840d3842 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -494,7 +494,8 @@ static void erofs_put_super(struct super_block *sb)
 	mutex_lock(&sbi->umount_mutex);
 
 #ifdef CONFIG_EROFS_FS_ZIP
-	erofs_workstation_cleanup_all(sb);
+	/* clean up the compression space of this sb */
+	erofs_shrink_workstation(EROFS_SB(sb), ~0UL, true);
 #endif
 
 	erofs_unregister_super(sb);
-- 
2.14.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/5] staging: erofs: localize erofs_workgroup_get
  2019-01-16  8:59 [PATCH 1/5] staging: erofs: sunset erofs_workstation_cleanup_all Gao Xiang
@ 2019-01-16  8:59 ` Gao Xiang
  2019-01-16  9:20   ` Chao Yu
  2019-01-16  8:59 ` [PATCH 3/5] staging: erofs: decrease the shrink count in erofs_workgroup_get Gao Xiang
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Gao Xiang @ 2019-01-16  8:59 UTC (permalink / raw)
  To: Chao Yu, Greg Kroah-Hartman, devel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei, Gao Xiang

Staticize erofs_workgroup_get since no external user
out of utils.c directly calls erofs_workgroup_get.

Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/internal.h | 21 ---------------------
 drivers/staging/erofs/utils.c    | 19 +++++++++++++++++++
 2 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 80cc09ea787d..967ec1a56726 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -252,28 +252,7 @@ static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp)
 }
 #endif
 
-static inline int erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
-{
-	int o;
-
-repeat:
-	o = erofs_wait_on_workgroup_freezed(grp);
-
-	if (unlikely(o <= 0))
-		return -1;
-
-	if (unlikely(atomic_cmpxchg(&grp->refcount, o, o + 1) != o))
-		goto repeat;
-
-	*ocnt = o;
-	return 0;
-}
-
-#define __erofs_workgroup_get(grp)	atomic_inc(&(grp)->refcount)
-#define __erofs_workgroup_put(grp)	atomic_dec(&(grp)->refcount)
-
 extern int erofs_workgroup_put(struct erofs_workgroup *grp);
-
 extern struct erofs_workgroup *erofs_find_workgroup(
 	struct super_block *sb, pgoff_t index, bool *tag);
 
diff --git a/drivers/staging/erofs/utils.c b/drivers/staging/erofs/utils.c
index aed211cd5875..d24945aab133 100644
--- a/drivers/staging/erofs/utils.c
+++ b/drivers/staging/erofs/utils.c
@@ -31,6 +31,25 @@ struct page *erofs_allocpage(struct list_head *pool, gfp_t gfp)
 static atomic_long_t erofs_global_shrink_cnt;
 
 #ifdef CONFIG_EROFS_FS_ZIP
+#define __erofs_workgroup_get(grp)	atomic_inc(&(grp)->refcount)
+#define __erofs_workgroup_put(grp)	atomic_dec(&(grp)->refcount)
+
+static int erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
+{
+	int o;
+
+repeat:
+	o = erofs_wait_on_workgroup_freezed(grp);
+
+	if (unlikely(o <= 0))
+		return -1;
+
+	if (unlikely(atomic_cmpxchg(&grp->refcount, o, o + 1) != o))
+		goto repeat;
+
+	*ocnt = o;
+	return 0;
+}
 
 struct erofs_workgroup *erofs_find_workgroup(
 	struct super_block *sb, pgoff_t index, bool *tag)
-- 
2.14.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/5] staging: erofs: decrease the shrink count in erofs_workgroup_get
  2019-01-16  8:59 [PATCH 1/5] staging: erofs: sunset erofs_workstation_cleanup_all Gao Xiang
  2019-01-16  8:59 ` [PATCH 2/5] staging: erofs: localize erofs_workgroup_get Gao Xiang
@ 2019-01-16  8:59 ` Gao Xiang
  2019-01-16  9:27   ` Chao Yu
  2019-01-16 10:45   ` Dan Carpenter
  2019-01-16  8:59 ` [PATCH 4/5] staging: erofs: staticize erofs_shrink_count, erofs_shrink_scan Gao Xiang
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 13+ messages in thread
From: Gao Xiang @ 2019-01-16  8:59 UTC (permalink / raw)
  To: Chao Yu, Greg Kroah-Hartman, devel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei, Gao Xiang

It is more suitable to update in erofs_workgroup_get since
it's actually the one matched with erofs_workgroup_put.

Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/utils.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/erofs/utils.c b/drivers/staging/erofs/utils.c
index d24945aab133..5695efaeb43a 100644
--- a/drivers/staging/erofs/utils.c
+++ b/drivers/staging/erofs/utils.c
@@ -34,29 +34,29 @@ static atomic_long_t erofs_global_shrink_cnt;
 #define __erofs_workgroup_get(grp)	atomic_inc(&(grp)->refcount)
 #define __erofs_workgroup_put(grp)	atomic_dec(&(grp)->refcount)
 
-static int erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
+static int erofs_workgroup_get(struct erofs_workgroup *grp)
 {
 	int o;
 
 repeat:
 	o = erofs_wait_on_workgroup_freezed(grp);
-
 	if (unlikely(o <= 0))
 		return -1;
 
 	if (unlikely(atomic_cmpxchg(&grp->refcount, o, o + 1) != o))
 		goto repeat;
 
-	*ocnt = o;
+	/* decrease refcount paired by erofs_workgroup_put */
+	if (unlikely(o == 1))
+		atomic_long_dec(&erofs_global_shrink_cnt);
 	return 0;
 }
 
-struct erofs_workgroup *erofs_find_workgroup(
-	struct super_block *sb, pgoff_t index, bool *tag)
+struct erofs_workgroup *erofs_find_workgroup(struct super_block *sb,
+					     pgoff_t index, bool *tag)
 {
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
 	struct erofs_workgroup *grp;
-	int oldcount;
 
 repeat:
 	rcu_read_lock();
@@ -65,15 +65,12 @@ struct erofs_workgroup *erofs_find_workgroup(
 		*tag = xa_pointer_tag(grp);
 		grp = xa_untag_pointer(grp);
 
-		if (erofs_workgroup_get(grp, &oldcount)) {
+		if (erofs_workgroup_get(grp)) {
 			/* prefer to relax rcu read side */
 			rcu_read_unlock();
 			goto repeat;
 		}
 
-		/* decrease refcount added by erofs_workgroup_put */
-		if (unlikely(oldcount == 1))
-			atomic_long_dec(&erofs_global_shrink_cnt);
 		DBG_BUGON(index != grp->index);
 	}
 	rcu_read_unlock();
-- 
2.14.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/5] staging: erofs: staticize erofs_shrink_count, erofs_shrink_scan
  2019-01-16  8:59 [PATCH 1/5] staging: erofs: sunset erofs_workstation_cleanup_all Gao Xiang
  2019-01-16  8:59 ` [PATCH 2/5] staging: erofs: localize erofs_workgroup_get Gao Xiang
  2019-01-16  8:59 ` [PATCH 3/5] staging: erofs: decrease the shrink count in erofs_workgroup_get Gao Xiang
@ 2019-01-16  8:59 ` Gao Xiang
  2019-01-16  9:28   ` Chao Yu
  2019-01-16  8:59 ` [PATCH 5/5] staging: erofs: drop the extern prefix for function definitions Gao Xiang
  2019-01-16  9:19 ` [PATCH 1/5] staging: erofs: sunset erofs_workstation_cleanup_all Chao Yu
  4 siblings, 1 reply; 13+ messages in thread
From: Gao Xiang @ 2019-01-16  8:59 UTC (permalink / raw)
  To: Chao Yu, Greg Kroah-Hartman, devel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei, Gao Xiang

Move erofs_shrinker_info to utils.c and therefore
no need to globalize erofs_shrink_count and erofs_shrink_scan.

Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/internal.h |  5 +----
 drivers/staging/erofs/super.c    |  6 ------
 drivers/staging/erofs/utils.c    | 14 ++++++++++----
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 967ec1a56726..304c79d253ae 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -589,10 +589,7 @@ extern struct page *erofs_allocpage(struct list_head *pool, gfp_t gfp);
 extern void erofs_register_super(struct super_block *sb);
 extern void erofs_unregister_super(struct super_block *sb);
 
-extern unsigned long erofs_shrink_count(struct shrinker *shrink,
-	struct shrink_control *sc);
-extern unsigned long erofs_shrink_scan(struct shrinker *shrink,
-	struct shrink_control *sc);
+extern struct shrinker erofs_shrinker_info;
 
 #ifndef lru_to_page
 #define lru_to_page(head) (list_entry((head)->prev, struct page, lru))
diff --git a/drivers/staging/erofs/super.c b/drivers/staging/erofs/super.c
index 69e1840d3842..af5140eede18 100644
--- a/drivers/staging/erofs/super.c
+++ b/drivers/staging/erofs/super.c
@@ -539,12 +539,6 @@ static void erofs_kill_sb(struct super_block *sb)
 	kill_block_super(sb);
 }
 
-static struct shrinker erofs_shrinker_info = {
-	.scan_objects = erofs_shrink_scan,
-	.count_objects = erofs_shrink_count,
-	.seeks = DEFAULT_SEEKS,
-};
-
 static struct file_system_type erofs_fs_type = {
 	.owner          = THIS_MODULE,
 	.name           = "erofs",
diff --git a/drivers/staging/erofs/utils.c b/drivers/staging/erofs/utils.c
index 5695efaeb43a..5f61f99f4c10 100644
--- a/drivers/staging/erofs/utils.c
+++ b/drivers/staging/erofs/utils.c
@@ -270,14 +270,14 @@ void erofs_unregister_super(struct super_block *sb)
 	spin_unlock(&erofs_sb_list_lock);
 }
 
-unsigned long erofs_shrink_count(struct shrinker *shrink,
-				 struct shrink_control *sc)
+static unsigned long erofs_shrink_count(struct shrinker *shrink,
+					struct shrink_control *sc)
 {
 	return atomic_long_read(&erofs_global_shrink_cnt);
 }
 
-unsigned long erofs_shrink_scan(struct shrinker *shrink,
-				struct shrink_control *sc)
+static unsigned long erofs_shrink_scan(struct shrinker *shrink,
+				       struct shrink_control *sc)
 {
 	struct erofs_sb_info *sbi;
 	struct list_head *p;
@@ -333,3 +333,9 @@ unsigned long erofs_shrink_scan(struct shrinker *shrink,
 	return freed;
 }
 
+struct shrinker erofs_shrinker_info = {
+	.scan_objects = erofs_shrink_scan,
+	.count_objects = erofs_shrink_count,
+	.seeks = DEFAULT_SEEKS,
+};
+
-- 
2.14.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/5] staging: erofs: drop the extern prefix for function definitions
  2019-01-16  8:59 [PATCH 1/5] staging: erofs: sunset erofs_workstation_cleanup_all Gao Xiang
                   ` (2 preceding siblings ...)
  2019-01-16  8:59 ` [PATCH 4/5] staging: erofs: staticize erofs_shrink_count, erofs_shrink_scan Gao Xiang
@ 2019-01-16  8:59 ` Gao Xiang
  2019-01-16  9:29   ` Chao Yu
  2019-01-16  9:19 ` [PATCH 1/5] staging: erofs: sunset erofs_workstation_cleanup_all Chao Yu
  4 siblings, 1 reply; 13+ messages in thread
From: Gao Xiang @ 2019-01-16  8:59 UTC (permalink / raw)
  To: Chao Yu, Greg Kroah-Hartman, devel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei, Gao Xiang

Fix all `CHECK: extern prototypes should be avoided in .h files'
reported by checkpatch.pl.

Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
 drivers/staging/erofs/internal.h  | 47 +++++++++++++++++----------------------
 drivers/staging/erofs/unzip_vle.h | 24 ++++++++++----------
 2 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 304c79d253ae..3a27c255950b 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -252,23 +252,20 @@ static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp)
 }
 #endif
 
-extern int erofs_workgroup_put(struct erofs_workgroup *grp);
-extern struct erofs_workgroup *erofs_find_workgroup(
-	struct super_block *sb, pgoff_t index, bool *tag);
-
-extern int erofs_register_workgroup(struct super_block *sb,
-	struct erofs_workgroup *grp, bool tag);
-
-extern unsigned long erofs_shrink_workstation(struct erofs_sb_info *sbi,
-	unsigned long nr_shrink, bool cleanup);
-
-extern void erofs_workgroup_free_rcu(struct erofs_workgroup *grp);
+int erofs_workgroup_put(struct erofs_workgroup *grp);
+struct erofs_workgroup *erofs_find_workgroup(struct super_block *sb,
+					     pgoff_t index, bool *tag);
+int erofs_register_workgroup(struct super_block *sb,
+			     struct erofs_workgroup *grp, bool tag);
+unsigned long erofs_shrink_workstation(struct erofs_sb_info *sbi,
+				       unsigned long nr_shrink, bool cleanup);
+void erofs_workgroup_free_rcu(struct erofs_workgroup *grp);
 
 #ifdef EROFS_FS_HAS_MANAGED_CACHE
-extern int erofs_try_to_free_all_cached_pages(struct erofs_sb_info *sbi,
-	struct erofs_workgroup *egrp);
-extern int erofs_try_to_free_cached_page(struct address_space *mapping,
-	struct page *page);
+int erofs_try_to_free_all_cached_pages(struct erofs_sb_info *sbi,
+				       struct erofs_workgroup *egrp);
+int erofs_try_to_free_cached_page(struct address_space *mapping,
+				  struct page *page);
 
 #define MNGD_MAPPING(sbi)	((sbi)->managed_cache->i_mapping)
 #else
@@ -495,8 +492,8 @@ static inline void __submit_bio(struct bio *bio, unsigned op, unsigned op_flags)
 #define EROFS_IO_MAX_RETRIES_NOFAIL	CONFIG_EROFS_FS_IO_MAX_RETRIES
 #endif
 
-extern struct page *__erofs_get_meta_page(struct super_block *sb,
-	erofs_blk_t blkaddr, bool prio, bool nofail);
+struct page *__erofs_get_meta_page(struct super_block *sb, erofs_blk_t blkaddr,
+				   bool prio, bool nofail);
 
 static inline struct page *erofs_get_meta_page(struct super_block *sb,
 	erofs_blk_t blkaddr, bool prio)
@@ -510,7 +507,7 @@ static inline struct page *erofs_get_meta_page_nofail(struct super_block *sb,
 	return __erofs_get_meta_page(sb, blkaddr, prio, true);
 }
 
-extern int erofs_map_blocks(struct inode *, struct erofs_map_blocks *, int);
+int erofs_map_blocks(struct inode *, struct erofs_map_blocks *, int);
 
 static inline struct page *
 erofs_get_inline_page(struct inode *inode,
@@ -530,9 +527,6 @@ static inline unsigned long erofs_inode_hash(erofs_nid_t nid)
 #endif
 }
 
-extern struct inode *erofs_iget(struct super_block *sb,
-	erofs_nid_t nid, bool dir);
-
 extern const struct inode_operations erofs_generic_iops;
 extern const struct inode_operations erofs_symlink_iops;
 extern const struct inode_operations erofs_fast_symlink_iops;
@@ -547,6 +541,8 @@ static inline bool is_inode_fast_symlink(struct inode *inode)
 	return inode->i_op == &erofs_fast_symlink_iops;
 }
 
+struct inode *erofs_iget(struct super_block *sb, erofs_nid_t nid, bool dir);
+
 /* namei.c */
 extern const struct inode_operations erofs_dir_iops;
 
@@ -584,13 +580,12 @@ static inline void erofs_vunmap(const void *mem, unsigned int count)
 }
 
 /* utils.c */
-extern struct page *erofs_allocpage(struct list_head *pool, gfp_t gfp);
-
-extern void erofs_register_super(struct super_block *sb);
-extern void erofs_unregister_super(struct super_block *sb);
-
 extern struct shrinker erofs_shrinker_info;
 
+struct page *erofs_allocpage(struct list_head *pool, gfp_t gfp);
+void erofs_register_super(struct super_block *sb);
+void erofs_unregister_super(struct super_block *sb);
+
 #ifndef lru_to_page
 #define lru_to_page(head) (list_entry((head)->prev, struct page, lru))
 #endif
diff --git a/drivers/staging/erofs/unzip_vle.h b/drivers/staging/erofs/unzip_vle.h
index 5a4e1b62c0d1..9dafa8dc712c 100644
--- a/drivers/staging/erofs/unzip_vle.h
+++ b/drivers/staging/erofs/unzip_vle.h
@@ -212,18 +212,18 @@ static inline void z_erofs_onlinepage_endio(struct page *page)
 #define Z_EROFS_VLE_VMAP_GLOBAL_PAGES	2048
 
 /* unzip_vle_lz4.c */
-extern int z_erofs_vle_plain_copy(struct page **compressed_pages,
-	unsigned clusterpages, struct page **pages,
-	unsigned nr_pages, unsigned short pageofs);
-
-extern int z_erofs_vle_unzip_fast_percpu(struct page **compressed_pages,
-	unsigned clusterpages, struct page **pages,
-	unsigned outlen, unsigned short pageofs,
-	void (*endio)(struct page *));
-
-extern int z_erofs_vle_unzip_vmap(struct page **compressed_pages,
-	unsigned clusterpages, void *vaddr, unsigned llen,
-	unsigned short pageofs, bool overlapped);
+int z_erofs_vle_plain_copy(struct page **compressed_pages,
+			   unsigned int clusterpages, struct page **pages,
+			   unsigned int nr_pages, unsigned short pageofs);
+int z_erofs_vle_unzip_fast_percpu(struct page **compressed_pages,
+				  unsigned int clusterpages,
+				  struct page **pages, unsigned int outlen,
+				  unsigned short pageofs,
+				  void (*endio)(struct page *));
+int z_erofs_vle_unzip_vmap(struct page **compressed_pages,
+			   unsigned int clusterpages,
+			   void *vaddr, unsigned int llen,
+			   unsigned short pageofs, bool overlapped);
 
 #endif
 
-- 
2.14.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/5] staging: erofs: sunset erofs_workstation_cleanup_all
  2019-01-16  8:59 [PATCH 1/5] staging: erofs: sunset erofs_workstation_cleanup_all Gao Xiang
                   ` (3 preceding siblings ...)
  2019-01-16  8:59 ` [PATCH 5/5] staging: erofs: drop the extern prefix for function definitions Gao Xiang
@ 2019-01-16  9:19 ` Chao Yu
  4 siblings, 0 replies; 13+ messages in thread
From: Chao Yu @ 2019-01-16  9:19 UTC (permalink / raw)
  To: Gao Xiang, Greg Kroah-Hartman, devel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei

On 2019/1/16 16:59, Gao Xiang wrote:
> There is only one user calling erofs_workstation_cleanup_all,
> and it is no likely that more users will use in that way
> in the future.
> 
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/5] staging: erofs: localize erofs_workgroup_get
  2019-01-16  8:59 ` [PATCH 2/5] staging: erofs: localize erofs_workgroup_get Gao Xiang
@ 2019-01-16  9:20   ` Chao Yu
  0 siblings, 0 replies; 13+ messages in thread
From: Chao Yu @ 2019-01-16  9:20 UTC (permalink / raw)
  To: Gao Xiang, Greg Kroah-Hartman, devel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei

On 2019/1/16 16:59, Gao Xiang wrote:
> Staticize erofs_workgroup_get since no external user
> out of utils.c directly calls erofs_workgroup_get.
> 
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] staging: erofs: decrease the shrink count in erofs_workgroup_get
  2019-01-16  8:59 ` [PATCH 3/5] staging: erofs: decrease the shrink count in erofs_workgroup_get Gao Xiang
@ 2019-01-16  9:27   ` Chao Yu
  2019-01-16 10:45   ` Dan Carpenter
  1 sibling, 0 replies; 13+ messages in thread
From: Chao Yu @ 2019-01-16  9:27 UTC (permalink / raw)
  To: Gao Xiang, Greg Kroah-Hartman, devel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei

On 2019/1/16 16:59, Gao Xiang wrote:
> It is more suitable to update in erofs_workgroup_get since
> it's actually the one matched with erofs_workgroup_put.
> 
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/5] staging: erofs: staticize erofs_shrink_count, erofs_shrink_scan
  2019-01-16  8:59 ` [PATCH 4/5] staging: erofs: staticize erofs_shrink_count, erofs_shrink_scan Gao Xiang
@ 2019-01-16  9:28   ` Chao Yu
  0 siblings, 0 replies; 13+ messages in thread
From: Chao Yu @ 2019-01-16  9:28 UTC (permalink / raw)
  To: Gao Xiang, Greg Kroah-Hartman, devel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei

On 2019/1/16 16:59, Gao Xiang wrote:
> Move erofs_shrinker_info to utils.c and therefore
> no need to globalize erofs_shrink_count and erofs_shrink_scan.
> 
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 5/5] staging: erofs: drop the extern prefix for function definitions
  2019-01-16  8:59 ` [PATCH 5/5] staging: erofs: drop the extern prefix for function definitions Gao Xiang
@ 2019-01-16  9:29   ` Chao Yu
  0 siblings, 0 replies; 13+ messages in thread
From: Chao Yu @ 2019-01-16  9:29 UTC (permalink / raw)
  To: Gao Xiang, Greg Kroah-Hartman, devel
  Cc: LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du, Fang Wei

On 2019/1/16 16:59, Gao Xiang wrote:
> Fix all `CHECK: extern prototypes should be avoided in .h files'
> reported by checkpatch.pl.
> 
> Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] staging: erofs: decrease the shrink count in erofs_workgroup_get
  2019-01-16  8:59 ` [PATCH 3/5] staging: erofs: decrease the shrink count in erofs_workgroup_get Gao Xiang
  2019-01-16  9:27   ` Chao Yu
@ 2019-01-16 10:45   ` Dan Carpenter
  2019-01-16 13:01     ` Gao Xiang
  1 sibling, 1 reply; 13+ messages in thread
From: Dan Carpenter @ 2019-01-16 10:45 UTC (permalink / raw)
  To: Gao Xiang
  Cc: Chao Yu, Greg Kroah-Hartman, devel, linux-erofs, Chao Yu, LKML,
	weidu.du, Fang Wei, Miao Xie

On Wed, Jan 16, 2019 at 04:59:54PM +0800, Gao Xiang wrote:
> It is more suitable to update in erofs_workgroup_get since
> it's actually the one matched with erofs_workgroup_put.
> 

This patch is fine.  No need to resend.

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

But for future reference, I found the commit message a bit confusing.
Ideally, I would understand basically what the commit does and why
without needing to read the diff.

I mostly just read the subject or the commit message, either or, instead
of reading both.  (This is not really true.)  My email client looks like
this.  (Also not true).

https://marc.info/?l=linux-driver-devel&m=154762932712086&w=2

While none of that was strictly true, it is a little bit true-ish...
So please, assume that some people will start reading the commit message
without reading the subject.

I sort of thought from reading the commit message that it was a bugfix
or a behavior change.  A better commit message would be:

staging: erofs: move shrink accounting inside the function

This patch moves the &erofs_global_shrink_cnt accounting from the caller
to erofs_workgroup_get().  It's cleaner and it matches erofs_workgroup_put()
better.  No behavior change.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] staging: erofs: decrease the shrink count in erofs_workgroup_get
  2019-01-16 10:45   ` Dan Carpenter
@ 2019-01-16 13:01     ` Gao Xiang
  2019-01-16 13:10       ` [PATCH v2 3/5] staging: erofs: move shrink accounting inside the function Gao Xiang
  0 siblings, 1 reply; 13+ messages in thread
From: Gao Xiang @ 2019-01-16 13:01 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Chao Yu, Greg Kroah-Hartman, devel, linux-erofs, Chao Yu, LKML,
	weidu.du, Fang Wei, Miao Xie

Hi Dan,

On 2019/1/16 18:45, Dan Carpenter wrote:
> On Wed, Jan 16, 2019 at 04:59:54PM +0800, Gao Xiang wrote:
>> It is more suitable to update in erofs_workgroup_get since
>> it's actually the one matched with erofs_workgroup_put.
>>
> 
> This patch is fine.  No need to resend.
> 
> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> But for future reference, I found the commit message a bit confusing.
> Ideally, I would understand basically what the commit does and why
> without needing to read the diff.
> 
> I mostly just read the subject or the commit message, either or, instead
> of reading both.  (This is not really true.)  My email client looks like
> this.  (Also not true).
> 
> https://marc.info/?l=linux-driver-devel&m=154762932712086&w=2
> 
> While none of that was strictly true, it is a little bit true-ish...
> So please, assume that some people will start reading the commit message
> without reading the subject.
> 
> I sort of thought from reading the commit message that it was a bugfix
> or a behavior change.  A better commit message would be:
> 
> staging: erofs: move shrink accounting inside the function
> 
> This patch moves the &erofs_global_shrink_cnt accounting from the caller
> to erofs_workgroup_get().  It's cleaner and it matches erofs_workgroup_put()
> better.  No behavior change.

Thanks for your kindly suggestion.
Actually I have to think more about how to express in English
as a foreign speaker...

But you are absolutely right ;) this commit is actually confusing and I will
do my best to write commit subject and message clearly...

If something wrong like this, please kindly point out and I will update immediately :)

Anyway, I will send patch v2 of this commit for Greg to merge...

Thanks,
Gao Xiang

> 
> regards,
> dan carpenter
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 3/5] staging: erofs: move shrink accounting inside the function
  2019-01-16 13:01     ` Gao Xiang
@ 2019-01-16 13:10       ` Gao Xiang
  0 siblings, 0 replies; 13+ messages in thread
From: Gao Xiang @ 2019-01-16 13:10 UTC (permalink / raw)
  To: Greg Kroah-Hartman, devel
  Cc: Chao Yu, LKML, linux-erofs, Chao Yu, Miao Xie, weidu.du,
	Fang Wei, Dan Carpenter, Gao Xiang

This patch moves the &erofs_global_shrink_cnt accounting
from the caller to erofs_workgroup_get().  It's cleaner and
it matches erofs_workgroup_put() better. No behavior change.

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Gao Xiang <gaoxiang25@huawei.com>
---
change log v2:
 - fix commit message as Dan Carpenter pointed out to make more clear.

Thanks,
Gao Xiang

 drivers/staging/erofs/utils.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/erofs/utils.c b/drivers/staging/erofs/utils.c
index d24945aab133..5695efaeb43a 100644
--- a/drivers/staging/erofs/utils.c
+++ b/drivers/staging/erofs/utils.c
@@ -34,29 +34,29 @@ static atomic_long_t erofs_global_shrink_cnt;
 #define __erofs_workgroup_get(grp)	atomic_inc(&(grp)->refcount)
 #define __erofs_workgroup_put(grp)	atomic_dec(&(grp)->refcount)
 
-static int erofs_workgroup_get(struct erofs_workgroup *grp, int *ocnt)
+static int erofs_workgroup_get(struct erofs_workgroup *grp)
 {
 	int o;
 
 repeat:
 	o = erofs_wait_on_workgroup_freezed(grp);
-
 	if (unlikely(o <= 0))
 		return -1;
 
 	if (unlikely(atomic_cmpxchg(&grp->refcount, o, o + 1) != o))
 		goto repeat;
 
-	*ocnt = o;
+	/* decrease refcount paired by erofs_workgroup_put */
+	if (unlikely(o == 1))
+		atomic_long_dec(&erofs_global_shrink_cnt);
 	return 0;
 }
 
-struct erofs_workgroup *erofs_find_workgroup(
-	struct super_block *sb, pgoff_t index, bool *tag)
+struct erofs_workgroup *erofs_find_workgroup(struct super_block *sb,
+					     pgoff_t index, bool *tag)
 {
 	struct erofs_sb_info *sbi = EROFS_SB(sb);
 	struct erofs_workgroup *grp;
-	int oldcount;
 
 repeat:
 	rcu_read_lock();
@@ -65,15 +65,12 @@ struct erofs_workgroup *erofs_find_workgroup(
 		*tag = xa_pointer_tag(grp);
 		grp = xa_untag_pointer(grp);
 
-		if (erofs_workgroup_get(grp, &oldcount)) {
+		if (erofs_workgroup_get(grp)) {
 			/* prefer to relax rcu read side */
 			rcu_read_unlock();
 			goto repeat;
 		}
 
-		/* decrease refcount added by erofs_workgroup_put */
-		if (unlikely(oldcount == 1))
-			atomic_long_dec(&erofs_global_shrink_cnt);
 		DBG_BUGON(index != grp->index);
 	}
 	rcu_read_unlock();
-- 
2.14.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-01-16 13:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-16  8:59 [PATCH 1/5] staging: erofs: sunset erofs_workstation_cleanup_all Gao Xiang
2019-01-16  8:59 ` [PATCH 2/5] staging: erofs: localize erofs_workgroup_get Gao Xiang
2019-01-16  9:20   ` Chao Yu
2019-01-16  8:59 ` [PATCH 3/5] staging: erofs: decrease the shrink count in erofs_workgroup_get Gao Xiang
2019-01-16  9:27   ` Chao Yu
2019-01-16 10:45   ` Dan Carpenter
2019-01-16 13:01     ` Gao Xiang
2019-01-16 13:10       ` [PATCH v2 3/5] staging: erofs: move shrink accounting inside the function Gao Xiang
2019-01-16  8:59 ` [PATCH 4/5] staging: erofs: staticize erofs_shrink_count, erofs_shrink_scan Gao Xiang
2019-01-16  9:28   ` Chao Yu
2019-01-16  8:59 ` [PATCH 5/5] staging: erofs: drop the extern prefix for function definitions Gao Xiang
2019-01-16  9:29   ` Chao Yu
2019-01-16  9:19 ` [PATCH 1/5] staging: erofs: sunset erofs_workstation_cleanup_all 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).