linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/15] Remove callback indirections in compression code
@ 2019-10-14 12:22 David Sterba
  2019-10-14 12:22 ` [PATCH 01/15] btrfs: export compression and decompression callbacks David Sterba
                   ` (16 more replies)
  0 siblings, 17 replies; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

The series removes all per-compression algorithm callbacks and replaces
them with a switches. Lots of indirections are simplified and while it
looks nice from design POV, we don't need to over-engineer it as we have
only fixed known number of the users and not public API.

The cost of indirect function calls is also higher with enabled
mitigations of the spectre vulnerability, so they've been incrementally
removed from the whole kernel and from btrfs as well.

David Sterba (15):
  btrfs: export compression and decompression callbacks
  btrfs: switch compression callbacks to direct calls
  btrfs: compression: attach workspace manager to the ops
  btrfs: compression: let workspace manager init take only the type
  btrfs: compression: inline init_workspace_manager
  btrfs: compression: let workspace manager cleanup take only the type
  btrfs: compression: inline cleanup_workspace_manager
  btrfs: compression: export alloc/free/get/put callbacks of all algos
  btrfs: compression: inline get_workspace
  btrfs: compression: inline put_workspace
  btrfs: compression: pass type to btrfs_get_workspace
  btrfs: compression: inline alloc_workspace
  btrfs: compression: pass type to btrfs_put_workspace
  btrfs: compression: inline free_workspace
  btrfs: compression: remove ops pointer from workspace_manager

 fs/btrfs/compression.c | 241 +++++++++++++++++++++++++++++++----------
 fs/btrfs/compression.h |  39 +------
 fs/btrfs/lzo.c         |  53 ++-------
 fs/btrfs/zlib.c        |  52 ++-------
 fs/btrfs/zstd.c        |  47 +++-----
 5 files changed, 227 insertions(+), 205 deletions(-)

-- 
2.23.0


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

* [PATCH 01/15] btrfs: export compression and decompression callbacks
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17  9:39   ` Johannes Thumshirn
  2019-10-14 12:22 ` [PATCH 02/15] btrfs: switch compression callbacks to direct calls David Sterba
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

Export compress_pages, decompress_bio and decompress callbacks for all
compression algos. The indirect calls will be replaced by a switch.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 24 ++++++++++++++++++++++++
 fs/btrfs/lzo.c         | 19 +++++++------------
 fs/btrfs/zlib.c        | 19 +++++++------------
 fs/btrfs/zstd.c        | 19 +++++++------------
 4 files changed, 45 insertions(+), 36 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 93deaf0cc2b8..8611a8b3321a 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -29,6 +29,30 @@
 #include "extent_io.h"
 #include "extent_map.h"
 
+int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
+		u64 start, struct page **pages, unsigned long *out_pages,
+		unsigned long *total_in, unsigned long *total_out);
+int zlib_decompress_bio(struct list_head *ws, struct compressed_bio *cb);
+int zlib_decompress(struct list_head *ws, unsigned char *data_in,
+		struct page *dest_page, unsigned long start_byte, size_t srclen,
+		size_t destlen);
+
+int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
+		u64 start, struct page **pages, unsigned long *out_pages,
+		unsigned long *total_in, unsigned long *total_out);
+int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb);
+int lzo_decompress(struct list_head *ws, unsigned char *data_in,
+		struct page *dest_page, unsigned long start_byte, size_t srclen,
+		size_t destlen);
+
+int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
+		u64 start, struct page **pages, unsigned long *out_pages,
+		unsigned long *total_in, unsigned long *total_out);
+int zstd_decompress_bio(struct list_head *ws, struct compressed_bio *cb);
+int zstd_decompress(struct list_head *ws, unsigned char *data_in,
+		struct page *dest_page, unsigned long start_byte, size_t srclen,
+		size_t destlen);
+
 static const char* const btrfs_compress_types[] = { "", "zlib", "lzo", "zstd" };
 
 const char* btrfs_compress_type2str(enum btrfs_compression_type type)
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index acad4174f68d..04a6815ea9cb 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -131,13 +131,9 @@ static inline size_t read_compress_length(const char *buf)
 	return le32_to_cpu(dlen);
 }
 
-static int lzo_compress_pages(struct list_head *ws,
-			      struct address_space *mapping,
-			      u64 start,
-			      struct page **pages,
-			      unsigned long *out_pages,
-			      unsigned long *total_in,
-			      unsigned long *total_out)
+int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
+		u64 start, struct page **pages, unsigned long *out_pages,
+		unsigned long *total_in, unsigned long *total_out)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
 	int ret = 0;
@@ -303,7 +299,7 @@ static int lzo_compress_pages(struct list_head *ws,
 	return ret;
 }
 
-static int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
+int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
 	int ret = 0, ret2;
@@ -444,10 +440,9 @@ static int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 	return ret;
 }
 
-static int lzo_decompress(struct list_head *ws, unsigned char *data_in,
-			  struct page *dest_page,
-			  unsigned long start_byte,
-			  size_t srclen, size_t destlen)
+int lzo_decompress(struct list_head *ws, unsigned char *data_in,
+		struct page *dest_page, unsigned long start_byte, size_t srclen,
+		size_t destlen)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
 	size_t in_len;
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index df1aace5df50..4091f94ba378 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -88,13 +88,9 @@ static struct list_head *zlib_alloc_workspace(unsigned int level)
 	return ERR_PTR(-ENOMEM);
 }
 
-static int zlib_compress_pages(struct list_head *ws,
-			       struct address_space *mapping,
-			       u64 start,
-			       struct page **pages,
-			       unsigned long *out_pages,
-			       unsigned long *total_in,
-			       unsigned long *total_out)
+int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
+		u64 start, struct page **pages, unsigned long *out_pages,
+		unsigned long *total_in, unsigned long *total_out)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
 	int ret;
@@ -228,7 +224,7 @@ static int zlib_compress_pages(struct list_head *ws,
 	return ret;
 }
 
-static int zlib_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
+int zlib_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
 	int ret = 0, ret2;
@@ -319,10 +315,9 @@ static int zlib_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 	return ret;
 }
 
-static int zlib_decompress(struct list_head *ws, unsigned char *data_in,
-			   struct page *dest_page,
-			   unsigned long start_byte,
-			   size_t srclen, size_t destlen)
+int zlib_decompress(struct list_head *ws, unsigned char *data_in,
+		struct page *dest_page, unsigned long start_byte, size_t srclen,
+		size_t destlen)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
 	int ret = 0;
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 764d47b107e5..b3728220c329 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -367,13 +367,9 @@ static struct list_head *zstd_alloc_workspace(unsigned int level)
 	return ERR_PTR(-ENOMEM);
 }
 
-static int zstd_compress_pages(struct list_head *ws,
-		struct address_space *mapping,
-		u64 start,
-		struct page **pages,
-		unsigned long *out_pages,
-		unsigned long *total_in,
-		unsigned long *total_out)
+int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
+		u64 start, struct page **pages, unsigned long *out_pages,
+		unsigned long *total_in, unsigned long *total_out)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
 	ZSTD_CStream *stream;
@@ -548,7 +544,7 @@ static int zstd_compress_pages(struct list_head *ws,
 	return ret;
 }
 
-static int zstd_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
+int zstd_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
 	struct page **pages_in = cb->compressed_pages;
@@ -626,10 +622,9 @@ static int zstd_decompress_bio(struct list_head *ws, struct compressed_bio *cb)
 	return ret;
 }
 
-static int zstd_decompress(struct list_head *ws, unsigned char *data_in,
-		struct page *dest_page,
-		unsigned long start_byte,
-		size_t srclen, size_t destlen)
+int zstd_decompress(struct list_head *ws, unsigned char *data_in,
+		struct page *dest_page, unsigned long start_byte, size_t srclen,
+		size_t destlen)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
 	ZSTD_DStream *stream;
-- 
2.23.0


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

* [PATCH 02/15] btrfs: switch compression callbacks to direct calls
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
  2019-10-14 12:22 ` [PATCH 01/15] btrfs: export compression and decompression callbacks David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17  9:42   ` Johannes Thumshirn
  2019-10-14 12:22 ` [PATCH 03/15] btrfs: compression: attach workspace manager to the ops David Sterba
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

The indirect calls bring some overhead due to spectre vulnerability
mitigations. The number of cases is small and below the threshold
(10-20) where indirect call would be better.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 77 +++++++++++++++++++++++++++++++++++++-----
 fs/btrfs/compression.h | 17 ----------
 fs/btrfs/lzo.c         |  3 --
 fs/btrfs/zlib.c        |  3 --
 fs/btrfs/zstd.c        |  3 --
 5 files changed, 69 insertions(+), 34 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 8611a8b3321a..87bac8f73a99 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -86,6 +86,70 @@ bool btrfs_compress_is_valid_type(const char *str, size_t len)
 	return false;
 }
 
+static int compression_compress_pages(int type, struct list_head *ws,
+               struct address_space *mapping, u64 start, struct page **pages,
+               unsigned long *out_pages, unsigned long *total_in,
+               unsigned long *total_out)
+{
+	switch (type) {
+	case BTRFS_COMPRESS_ZLIB:
+		return zlib_compress_pages(ws, mapping, start, pages,
+				out_pages, total_in, total_out);
+	case BTRFS_COMPRESS_LZO:
+		return lzo_compress_pages(ws, mapping, start, pages,
+				out_pages, total_in, total_out);
+	case BTRFS_COMPRESS_ZSTD:
+		return zstd_compress_pages(ws, mapping, start, pages,
+				out_pages, total_in, total_out);
+	case BTRFS_COMPRESS_NONE:
+	default:
+		/*
+		 * This can't happen, the type is validated several times
+		 * before we get here. As a sane fallback, return what the
+		 * callers will understand as 'no compression happened'.
+		 */
+		return -E2BIG;
+	}
+}
+
+static int compression_decompress_bio(int type, struct list_head *ws,
+		struct compressed_bio *cb)
+{
+	switch (type) {
+	case BTRFS_COMPRESS_ZLIB: return zlib_decompress_bio(ws, cb);
+	case BTRFS_COMPRESS_LZO:  return lzo_decompress_bio(ws, cb);
+	case BTRFS_COMPRESS_ZSTD: return zstd_decompress_bio(ws, cb);
+	case BTRFS_COMPRESS_NONE:
+	default:
+		/*
+		 * This can't happen, the type is validated several times
+		 * before we get here.
+		 */
+		BUG();
+	}
+}
+
+static int compression_decompress(int type, struct list_head *ws,
+               unsigned char *data_in, struct page *dest_page,
+               unsigned long start_byte, size_t srclen, size_t destlen)
+{
+	switch (type) {
+	case BTRFS_COMPRESS_ZLIB: return zlib_decompress(ws, data_in, dest_page,
+						start_byte, srclen, destlen);
+	case BTRFS_COMPRESS_LZO:  return lzo_decompress(ws, data_in, dest_page,
+						start_byte, srclen, destlen);
+	case BTRFS_COMPRESS_ZSTD: return zstd_decompress(ws, data_in, dest_page,
+						start_byte, srclen, destlen);
+	case BTRFS_COMPRESS_NONE:
+	default:
+		/*
+		 * This can't happen, the type is validated several times
+		 * before we get here.
+		 */
+		BUG();
+	}
+}
+
 static int btrfs_decompress_bio(struct compressed_bio *cb);
 
 static inline int compressed_bio_size(struct btrfs_fs_info *fs_info,
@@ -1074,10 +1138,8 @@ int btrfs_compress_pages(unsigned int type_level, struct address_space *mapping,
 
 	level = btrfs_compress_set_level(type, level);
 	workspace = get_workspace(type, level);
-	ret = btrfs_compress_op[type]->compress_pages(workspace, mapping,
-						      start, pages,
-						      out_pages,
-						      total_in, total_out);
+	ret = compression_compress_pages(type, workspace, mapping, start, pages,
+					 out_pages, total_in, total_out);
 	put_workspace(type, workspace);
 	return ret;
 }
@@ -1103,7 +1165,7 @@ static int btrfs_decompress_bio(struct compressed_bio *cb)
 	int type = cb->compress_type;
 
 	workspace = get_workspace(type, 0);
-	ret = btrfs_compress_op[type]->decompress_bio(workspace, cb);
+	ret = compression_decompress_bio(type, workspace, cb);
 	put_workspace(type, workspace);
 
 	return ret;
@@ -1121,9 +1183,8 @@ int btrfs_decompress(int type, unsigned char *data_in, struct page *dest_page,
 	int ret;
 
 	workspace = get_workspace(type, 0);
-	ret = btrfs_compress_op[type]->decompress(workspace, data_in,
-						  dest_page, start_byte,
-						  srclen, destlen);
+	ret = compression_decompress(type, workspace, data_in, dest_page,
+				     start_byte, srclen, destlen);
 	put_workspace(type, workspace);
 
 	return ret;
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 52dce1176f89..7db14d3166b5 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -140,23 +140,6 @@ struct btrfs_compress_op {
 
 	void (*free_workspace)(struct list_head *workspace);
 
-	int (*compress_pages)(struct list_head *workspace,
-			      struct address_space *mapping,
-			      u64 start,
-			      struct page **pages,
-			      unsigned long *out_pages,
-			      unsigned long *total_in,
-			      unsigned long *total_out);
-
-	int (*decompress_bio)(struct list_head *workspace,
-				struct compressed_bio *cb);
-
-	int (*decompress)(struct list_head *workspace,
-			  unsigned char *data_in,
-			  struct page *dest_page,
-			  unsigned long start_byte,
-			  size_t srclen, size_t destlen);
-
 	/* Maximum level supported by the compression algorithm */
 	unsigned int max_level;
 	unsigned int default_level;
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index 04a6815ea9cb..9417944ec829 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -509,9 +509,6 @@ const struct btrfs_compress_op btrfs_lzo_compress = {
 	.put_workspace		= lzo_put_workspace,
 	.alloc_workspace	= lzo_alloc_workspace,
 	.free_workspace		= lzo_free_workspace,
-	.compress_pages		= lzo_compress_pages,
-	.decompress_bio		= lzo_decompress_bio,
-	.decompress		= lzo_decompress,
 	.max_level		= 1,
 	.default_level		= 1,
 };
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 4091f94ba378..8bb6f19ab369 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -420,9 +420,6 @@ const struct btrfs_compress_op btrfs_zlib_compress = {
 	.put_workspace		= zlib_put_workspace,
 	.alloc_workspace	= zlib_alloc_workspace,
 	.free_workspace		= zlib_free_workspace,
-	.compress_pages		= zlib_compress_pages,
-	.decompress_bio		= zlib_decompress_bio,
-	.decompress		= zlib_decompress,
 	.max_level		= 9,
 	.default_level		= BTRFS_ZLIB_DEFAULT_LEVEL,
 };
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index b3728220c329..5f17c741d167 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -713,9 +713,6 @@ const struct btrfs_compress_op btrfs_zstd_compress = {
 	.put_workspace = zstd_put_workspace,
 	.alloc_workspace = zstd_alloc_workspace,
 	.free_workspace = zstd_free_workspace,
-	.compress_pages = zstd_compress_pages,
-	.decompress_bio = zstd_decompress_bio,
-	.decompress = zstd_decompress,
 	.max_level	= ZSTD_BTRFS_MAX_LEVEL,
 	.default_level	= ZSTD_BTRFS_DEFAULT_LEVEL,
 };
-- 
2.23.0


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

* [PATCH 03/15] btrfs: compression: attach workspace manager to the ops
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
  2019-10-14 12:22 ` [PATCH 01/15] btrfs: export compression and decompression callbacks David Sterba
  2019-10-14 12:22 ` [PATCH 02/15] btrfs: switch compression callbacks to direct calls David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17 11:08   ` Johannes Thumshirn
  2019-10-14 12:22 ` [PATCH 04/15] btrfs: compression: let workspace manager init take only the type David Sterba
                   ` (13 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

There's a lot of indirection when the generic code calls into
algo-specific callbacks to reach the private workspace manager structure
and back to the generic code.

To simplify that, export the workspace manager for heuristic, LZO and
ZLIB, while ZSTD is going to use it's own manager.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 1 +
 fs/btrfs/compression.h | 1 +
 fs/btrfs/lzo.c         | 1 +
 fs/btrfs/zlib.c        | 1 +
 fs/btrfs/zstd.c        | 2 ++
 5 files changed, 6 insertions(+)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 87bac8f73a99..e650125b1d2b 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -920,6 +920,7 @@ static struct list_head *alloc_heuristic_ws(unsigned int level)
 }
 
 const struct btrfs_compress_op btrfs_heuristic_compress = {
+	.workspace_manager = &heuristic_wsm,
 	.init_workspace_manager = heuristic_init_workspace_manager,
 	.cleanup_workspace_manager = heuristic_cleanup_workspace_manager,
 	.get_workspace = heuristic_get_workspace,
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 7db14d3166b5..7091eae063e5 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -140,6 +140,7 @@ struct btrfs_compress_op {
 
 	void (*free_workspace)(struct list_head *workspace);
 
+	struct workspace_manager *workspace_manager;
 	/* Maximum level supported by the compression algorithm */
 	unsigned int max_level;
 	unsigned int default_level;
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index 9417944ec829..aff105cc80e7 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -503,6 +503,7 @@ int lzo_decompress(struct list_head *ws, unsigned char *data_in,
 }
 
 const struct btrfs_compress_op btrfs_lzo_compress = {
+	.workspace_manager	= &wsm,
 	.init_workspace_manager	= lzo_init_workspace_manager,
 	.cleanup_workspace_manager = lzo_cleanup_workspace_manager,
 	.get_workspace		= lzo_get_workspace,
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 8bb6f19ab369..a5e8f0207473 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -414,6 +414,7 @@ int zlib_decompress(struct list_head *ws, unsigned char *data_in,
 }
 
 const struct btrfs_compress_op btrfs_zlib_compress = {
+	.workspace_manager	= &wsm,
 	.init_workspace_manager	= zlib_init_workspace_manager,
 	.cleanup_workspace_manager = zlib_cleanup_workspace_manager,
 	.get_workspace		= zlib_get_workspace,
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 5f17c741d167..4791e89e43e3 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -707,6 +707,8 @@ int zstd_decompress(struct list_head *ws, unsigned char *data_in,
 }
 
 const struct btrfs_compress_op btrfs_zstd_compress = {
+	/* ZSTD uses own workspace manager */
+	.workspace_manager = NULL,
 	.init_workspace_manager = zstd_init_workspace_manager,
 	.cleanup_workspace_manager = zstd_cleanup_workspace_manager,
 	.get_workspace = zstd_get_workspace,
-- 
2.23.0


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

* [PATCH 04/15] btrfs: compression: let workspace manager init take only the type
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
                   ` (2 preceding siblings ...)
  2019-10-14 12:22 ` [PATCH 03/15] btrfs: compression: attach workspace manager to the ops David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17 11:03   ` Johannes Thumshirn
  2019-10-14 12:22 ` [PATCH 05/15] btrfs: compression: inline init_workspace_manager David Sterba
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

With the access to the workspace structures, we can look it up together
with the compression ops inside the workspace manager init helper.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 7 ++++---
 fs/btrfs/compression.h | 3 +--
 fs/btrfs/lzo.c         | 2 +-
 fs/btrfs/zlib.c        | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index e650125b1d2b..6adc7f6857d7 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -862,7 +862,7 @@ static struct workspace_manager heuristic_wsm;
 
 static void heuristic_init_workspace_manager(void)
 {
-	btrfs_init_workspace_manager(&heuristic_wsm, &btrfs_heuristic_compress);
+	btrfs_init_workspace_manager(BTRFS_COMPRESS_NONE);
 }
 
 static void heuristic_cleanup_workspace_manager(void)
@@ -937,9 +937,10 @@ static const struct btrfs_compress_op * const btrfs_compress_op[] = {
 	&btrfs_zstd_compress,
 };
 
-void btrfs_init_workspace_manager(struct workspace_manager *wsm,
-				  const struct btrfs_compress_op *ops)
+void btrfs_init_workspace_manager(int type)
 {
+	const struct btrfs_compress_op *ops = btrfs_compress_op[type];
+	struct workspace_manager *wsm = ops->workspace_manager;
 	struct list_head *workspace;
 
 	wsm->ops = ops;
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 7091eae063e5..10f82e791769 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -120,8 +120,7 @@ struct workspace_manager {
 	wait_queue_head_t ws_wait;
 };
 
-void btrfs_init_workspace_manager(struct workspace_manager *wsm,
-				  const struct btrfs_compress_op *ops);
+void btrfs_init_workspace_manager(int type);
 struct list_head *btrfs_get_workspace(struct workspace_manager *wsm,
 				      unsigned int level);
 void btrfs_put_workspace(struct workspace_manager *wsm, struct list_head *ws);
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index aff105cc80e7..5b8470041bf6 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -65,7 +65,7 @@ static struct workspace_manager wsm;
 
 static void lzo_init_workspace_manager(void)
 {
-	btrfs_init_workspace_manager(&wsm, &btrfs_lzo_compress);
+	btrfs_init_workspace_manager(BTRFS_COMPRESS_LZO);
 }
 
 static void lzo_cleanup_workspace_manager(void)
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index a5e8f0207473..be964128dba3 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -31,7 +31,7 @@ static struct workspace_manager wsm;
 
 static void zlib_init_workspace_manager(void)
 {
-	btrfs_init_workspace_manager(&wsm, &btrfs_zlib_compress);
+	btrfs_init_workspace_manager(BTRFS_COMPRESS_ZLIB);
 }
 
 static void zlib_cleanup_workspace_manager(void)
-- 
2.23.0


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

* [PATCH 05/15] btrfs: compression: inline init_workspace_manager
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
                   ` (3 preceding siblings ...)
  2019-10-14 12:22 ` [PATCH 04/15] btrfs: compression: let workspace manager init take only the type David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17 11:06   ` Johannes Thumshirn
  2019-10-14 12:22 ` [PATCH 06/15] btrfs: compression: let workspace manager cleanup take only the type David Sterba
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

Replace loop calling to all algos with a list of direct calls to the
init manager callback. When that becomes trivial it is replaced by
direct call to the helper.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 17 ++++++-----------
 fs/btrfs/compression.h |  3 ---
 fs/btrfs/lzo.c         |  6 ------
 fs/btrfs/zlib.c        |  6 ------
 fs/btrfs/zstd.c        |  3 +--
 5 files changed, 7 insertions(+), 28 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 6adc7f6857d7..61b9cf095ee5 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -52,6 +52,7 @@ int zstd_decompress_bio(struct list_head *ws, struct compressed_bio *cb);
 int zstd_decompress(struct list_head *ws, unsigned char *data_in,
 		struct page *dest_page, unsigned long start_byte, size_t srclen,
 		size_t destlen);
+void zstd_init_workspace_manager(void);
 
 static const char* const btrfs_compress_types[] = { "", "zlib", "lzo", "zstd" };
 
@@ -860,11 +861,6 @@ struct heuristic_ws {
 
 static struct workspace_manager heuristic_wsm;
 
-static void heuristic_init_workspace_manager(void)
-{
-	btrfs_init_workspace_manager(BTRFS_COMPRESS_NONE);
-}
-
 static void heuristic_cleanup_workspace_manager(void)
 {
 	btrfs_cleanup_workspace_manager(&heuristic_wsm);
@@ -921,7 +917,6 @@ static struct list_head *alloc_heuristic_ws(unsigned int level)
 
 const struct btrfs_compress_op btrfs_heuristic_compress = {
 	.workspace_manager = &heuristic_wsm,
-	.init_workspace_manager = heuristic_init_workspace_manager,
 	.cleanup_workspace_manager = heuristic_cleanup_workspace_manager,
 	.get_workspace = heuristic_get_workspace,
 	.put_workspace = heuristic_put_workspace,
@@ -937,7 +932,7 @@ static const struct btrfs_compress_op * const btrfs_compress_op[] = {
 	&btrfs_zstd_compress,
 };
 
-void btrfs_init_workspace_manager(int type)
+static void btrfs_init_workspace_manager(int type)
 {
 	const struct btrfs_compress_op *ops = btrfs_compress_op[type];
 	struct workspace_manager *wsm = ops->workspace_manager;
@@ -1194,10 +1189,10 @@ int btrfs_decompress(int type, unsigned char *data_in, struct page *dest_page,
 
 void __init btrfs_init_compress(void)
 {
-	int i;
-
-	for (i = 0; i < BTRFS_NR_WORKSPACE_MANAGERS; i++)
-		btrfs_compress_op[i]->init_workspace_manager();
+	btrfs_init_workspace_manager(BTRFS_COMPRESS_NONE);
+	btrfs_init_workspace_manager(BTRFS_COMPRESS_ZLIB);
+	btrfs_init_workspace_manager(BTRFS_COMPRESS_LZO);
+	zstd_init_workspace_manager();
 }
 
 void __cold btrfs_exit_compress(void)
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 10f82e791769..12a46139c389 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -120,15 +120,12 @@ struct workspace_manager {
 	wait_queue_head_t ws_wait;
 };
 
-void btrfs_init_workspace_manager(int type);
 struct list_head *btrfs_get_workspace(struct workspace_manager *wsm,
 				      unsigned int level);
 void btrfs_put_workspace(struct workspace_manager *wsm, struct list_head *ws);
 void btrfs_cleanup_workspace_manager(struct workspace_manager *wsm);
 
 struct btrfs_compress_op {
-	void (*init_workspace_manager)(void);
-
 	void (*cleanup_workspace_manager)(void);
 
 	struct list_head *(*get_workspace)(unsigned int level);
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index 5b8470041bf6..a55079477edf 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -63,11 +63,6 @@ struct workspace {
 
 static struct workspace_manager wsm;
 
-static void lzo_init_workspace_manager(void)
-{
-	btrfs_init_workspace_manager(BTRFS_COMPRESS_LZO);
-}
-
 static void lzo_cleanup_workspace_manager(void)
 {
 	btrfs_cleanup_workspace_manager(&wsm);
@@ -504,7 +499,6 @@ int lzo_decompress(struct list_head *ws, unsigned char *data_in,
 
 const struct btrfs_compress_op btrfs_lzo_compress = {
 	.workspace_manager	= &wsm,
-	.init_workspace_manager	= lzo_init_workspace_manager,
 	.cleanup_workspace_manager = lzo_cleanup_workspace_manager,
 	.get_workspace		= lzo_get_workspace,
 	.put_workspace		= lzo_put_workspace,
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index be964128dba3..39f1d0f1b286 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -29,11 +29,6 @@ struct workspace {
 
 static struct workspace_manager wsm;
 
-static void zlib_init_workspace_manager(void)
-{
-	btrfs_init_workspace_manager(BTRFS_COMPRESS_ZLIB);
-}
-
 static void zlib_cleanup_workspace_manager(void)
 {
 	btrfs_cleanup_workspace_manager(&wsm);
@@ -415,7 +410,6 @@ int zlib_decompress(struct list_head *ws, unsigned char *data_in,
 
 const struct btrfs_compress_op btrfs_zlib_compress = {
 	.workspace_manager	= &wsm,
-	.init_workspace_manager	= zlib_init_workspace_manager,
 	.cleanup_workspace_manager = zlib_cleanup_workspace_manager,
 	.get_workspace		= zlib_get_workspace,
 	.put_workspace		= zlib_put_workspace,
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 4791e89e43e3..8e7a804b05ee 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -168,7 +168,7 @@ static void zstd_calc_ws_mem_sizes(void)
 	}
 }
 
-static void zstd_init_workspace_manager(void)
+void zstd_init_workspace_manager(void)
 {
 	struct list_head *ws;
 	int i;
@@ -709,7 +709,6 @@ int zstd_decompress(struct list_head *ws, unsigned char *data_in,
 const struct btrfs_compress_op btrfs_zstd_compress = {
 	/* ZSTD uses own workspace manager */
 	.workspace_manager = NULL,
-	.init_workspace_manager = zstd_init_workspace_manager,
 	.cleanup_workspace_manager = zstd_cleanup_workspace_manager,
 	.get_workspace = zstd_get_workspace,
 	.put_workspace = zstd_put_workspace,
-- 
2.23.0


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

* [PATCH 06/15] btrfs: compression: let workspace manager cleanup take only the type
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
                   ` (4 preceding siblings ...)
  2019-10-14 12:22 ` [PATCH 05/15] btrfs: compression: inline init_workspace_manager David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17 11:10   ` Johannes Thumshirn
  2019-10-14 12:22 ` [PATCH 07/15] btrfs: compression: inline cleanup_workspace_manager David Sterba
                   ` (10 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

With the access to the workspace structures, we can look it up together
with the compression ops inside the workspace manager cleanup helper.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 6 ++++--
 fs/btrfs/compression.h | 2 +-
 fs/btrfs/lzo.c         | 2 +-
 fs/btrfs/zlib.c        | 2 +-
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 61b9cf095ee5..6c4dc62b9ab0 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -863,7 +863,7 @@ static struct workspace_manager heuristic_wsm;
 
 static void heuristic_cleanup_workspace_manager(void)
 {
-	btrfs_cleanup_workspace_manager(&heuristic_wsm);
+	btrfs_cleanup_workspace_manager(BTRFS_COMPRESS_NONE);
 }
 
 static struct list_head *heuristic_get_workspace(unsigned int level)
@@ -960,10 +960,12 @@ static void btrfs_init_workspace_manager(int type)
 	}
 }
 
-void btrfs_cleanup_workspace_manager(struct workspace_manager *wsman)
+void btrfs_cleanup_workspace_manager(int type)
 {
+	struct workspace_manager *wsman;
 	struct list_head *ws;
 
+	wsman = btrfs_compress_op[type]->workspace_manager;
 	while (!list_empty(&wsman->idle_ws)) {
 		ws = wsman->idle_ws.next;
 		list_del(ws);
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 12a46139c389..0deaa8e03836 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -123,7 +123,7 @@ struct workspace_manager {
 struct list_head *btrfs_get_workspace(struct workspace_manager *wsm,
 				      unsigned int level);
 void btrfs_put_workspace(struct workspace_manager *wsm, struct list_head *ws);
-void btrfs_cleanup_workspace_manager(struct workspace_manager *wsm);
+void btrfs_cleanup_workspace_manager(int type);
 
 struct btrfs_compress_op {
 	void (*cleanup_workspace_manager)(void);
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index a55079477edf..6aa602040506 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -65,7 +65,7 @@ static struct workspace_manager wsm;
 
 static void lzo_cleanup_workspace_manager(void)
 {
-	btrfs_cleanup_workspace_manager(&wsm);
+	btrfs_cleanup_workspace_manager(BTRFS_COMPRESS_LZO);
 }
 
 static struct list_head *lzo_get_workspace(unsigned int level)
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 39f1d0f1b286..7319e9f3d484 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -31,7 +31,7 @@ static struct workspace_manager wsm;
 
 static void zlib_cleanup_workspace_manager(void)
 {
-	btrfs_cleanup_workspace_manager(&wsm);
+	btrfs_cleanup_workspace_manager(BTRFS_COMPRESS_ZLIB);
 }
 
 static struct list_head *zlib_get_workspace(unsigned int level)
-- 
2.23.0


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

* [PATCH 07/15] btrfs: compression: inline cleanup_workspace_manager
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
                   ` (5 preceding siblings ...)
  2019-10-14 12:22 ` [PATCH 06/15] btrfs: compression: let workspace manager cleanup take only the type David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17 11:31   ` Johannes Thumshirn
  2019-10-14 12:22 ` [PATCH 08/15] btrfs: compression: export alloc/free/get/put callbacks of all algos David Sterba
                   ` (9 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

Replace loop calling to all algos with a list of direct calls to the
cleanup manager callback. When that becomes trivial it is replaced by
direct call to the helper.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 17 ++++++-----------
 fs/btrfs/compression.h |  3 ---
 fs/btrfs/lzo.c         |  6 ------
 fs/btrfs/zlib.c        |  6 ------
 fs/btrfs/zstd.c        |  3 +--
 5 files changed, 7 insertions(+), 28 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 6c4dc62b9ab0..34921a120829 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -53,6 +53,7 @@ int zstd_decompress(struct list_head *ws, unsigned char *data_in,
 		struct page *dest_page, unsigned long start_byte, size_t srclen,
 		size_t destlen);
 void zstd_init_workspace_manager(void);
+void zstd_cleanup_workspace_manager(void);
 
 static const char* const btrfs_compress_types[] = { "", "zlib", "lzo", "zstd" };
 
@@ -861,11 +862,6 @@ struct heuristic_ws {
 
 static struct workspace_manager heuristic_wsm;
 
-static void heuristic_cleanup_workspace_manager(void)
-{
-	btrfs_cleanup_workspace_manager(BTRFS_COMPRESS_NONE);
-}
-
 static struct list_head *heuristic_get_workspace(unsigned int level)
 {
 	return btrfs_get_workspace(&heuristic_wsm, level);
@@ -917,7 +913,6 @@ static struct list_head *alloc_heuristic_ws(unsigned int level)
 
 const struct btrfs_compress_op btrfs_heuristic_compress = {
 	.workspace_manager = &heuristic_wsm,
-	.cleanup_workspace_manager = heuristic_cleanup_workspace_manager,
 	.get_workspace = heuristic_get_workspace,
 	.put_workspace = heuristic_put_workspace,
 	.alloc_workspace = alloc_heuristic_ws,
@@ -960,7 +955,7 @@ static void btrfs_init_workspace_manager(int type)
 	}
 }
 
-void btrfs_cleanup_workspace_manager(int type)
+static void btrfs_cleanup_workspace_manager(int type)
 {
 	struct workspace_manager *wsman;
 	struct list_head *ws;
@@ -1199,10 +1194,10 @@ void __init btrfs_init_compress(void)
 
 void __cold btrfs_exit_compress(void)
 {
-	int i;
-
-	for (i = 0; i < BTRFS_NR_WORKSPACE_MANAGERS; i++)
-		btrfs_compress_op[i]->cleanup_workspace_manager();
+	btrfs_cleanup_workspace_manager(BTRFS_COMPRESS_NONE);
+	btrfs_cleanup_workspace_manager(BTRFS_COMPRESS_ZLIB);
+	btrfs_cleanup_workspace_manager(BTRFS_COMPRESS_LZO);
+	zstd_cleanup_workspace_manager();
 }
 
 /*
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 0deaa8e03836..cf667e599b70 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -123,11 +123,8 @@ struct workspace_manager {
 struct list_head *btrfs_get_workspace(struct workspace_manager *wsm,
 				      unsigned int level);
 void btrfs_put_workspace(struct workspace_manager *wsm, struct list_head *ws);
-void btrfs_cleanup_workspace_manager(int type);
 
 struct btrfs_compress_op {
-	void (*cleanup_workspace_manager)(void);
-
 	struct list_head *(*get_workspace)(unsigned int level);
 
 	void (*put_workspace)(struct list_head *ws);
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index 6aa602040506..6f4619e76c11 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -63,11 +63,6 @@ struct workspace {
 
 static struct workspace_manager wsm;
 
-static void lzo_cleanup_workspace_manager(void)
-{
-	btrfs_cleanup_workspace_manager(BTRFS_COMPRESS_LZO);
-}
-
 static struct list_head *lzo_get_workspace(unsigned int level)
 {
 	return btrfs_get_workspace(&wsm, level);
@@ -499,7 +494,6 @@ int lzo_decompress(struct list_head *ws, unsigned char *data_in,
 
 const struct btrfs_compress_op btrfs_lzo_compress = {
 	.workspace_manager	= &wsm,
-	.cleanup_workspace_manager = lzo_cleanup_workspace_manager,
 	.get_workspace		= lzo_get_workspace,
 	.put_workspace		= lzo_put_workspace,
 	.alloc_workspace	= lzo_alloc_workspace,
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 7319e9f3d484..03c632c7deac 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -29,11 +29,6 @@ struct workspace {
 
 static struct workspace_manager wsm;
 
-static void zlib_cleanup_workspace_manager(void)
-{
-	btrfs_cleanup_workspace_manager(BTRFS_COMPRESS_ZLIB);
-}
-
 static struct list_head *zlib_get_workspace(unsigned int level)
 {
 	struct list_head *ws = btrfs_get_workspace(&wsm, level);
@@ -410,7 +405,6 @@ int zlib_decompress(struct list_head *ws, unsigned char *data_in,
 
 const struct btrfs_compress_op btrfs_zlib_compress = {
 	.workspace_manager	= &wsm,
-	.cleanup_workspace_manager = zlib_cleanup_workspace_manager,
 	.get_workspace		= zlib_get_workspace,
 	.put_workspace		= zlib_put_workspace,
 	.alloc_workspace	= zlib_alloc_workspace,
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 8e7a804b05ee..f575ce77ea3d 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -194,7 +194,7 @@ void zstd_init_workspace_manager(void)
 	}
 }
 
-static void zstd_cleanup_workspace_manager(void)
+void zstd_cleanup_workspace_manager(void)
 {
 	struct workspace *workspace;
 	int i;
@@ -709,7 +709,6 @@ int zstd_decompress(struct list_head *ws, unsigned char *data_in,
 const struct btrfs_compress_op btrfs_zstd_compress = {
 	/* ZSTD uses own workspace manager */
 	.workspace_manager = NULL,
-	.cleanup_workspace_manager = zstd_cleanup_workspace_manager,
 	.get_workspace = zstd_get_workspace,
 	.put_workspace = zstd_put_workspace,
 	.alloc_workspace = zstd_alloc_workspace,
-- 
2.23.0


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

* [PATCH 08/15] btrfs: compression: export alloc/free/get/put callbacks of all algos
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
                   ` (6 preceding siblings ...)
  2019-10-14 12:22 ` [PATCH 07/15] btrfs: compression: inline cleanup_workspace_manager David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17 11:44   ` Johannes Thumshirn
  2019-10-14 12:22 ` [PATCH 09/15] btrfs: compression: inline get_workspace David Sterba
                   ` (8 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

The indirect calls will be replaced by a switch in compression.c.
(Switch is faster than indirect calls with when Spectre mitigations are
enabled).

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 12 ++++++++++++
 fs/btrfs/lzo.c         |  8 ++++----
 fs/btrfs/zlib.c        |  8 ++++----
 fs/btrfs/zstd.c        | 13 ++++++-------
 4 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 34921a120829..f707db4a9a53 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -36,6 +36,10 @@ int zlib_decompress_bio(struct list_head *ws, struct compressed_bio *cb);
 int zlib_decompress(struct list_head *ws, unsigned char *data_in,
 		struct page *dest_page, unsigned long start_byte, size_t srclen,
 		size_t destlen);
+struct list_head *zlib_alloc_workspace(unsigned int level);
+void zlib_free_workspace(struct list_head *ws);
+struct list_head *zlib_get_workspace(unsigned int level);
+void zlib_put_workspace(struct list_head *ws);
 
 int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
 		u64 start, struct page **pages, unsigned long *out_pages,
@@ -44,6 +48,10 @@ int lzo_decompress_bio(struct list_head *ws, struct compressed_bio *cb);
 int lzo_decompress(struct list_head *ws, unsigned char *data_in,
 		struct page *dest_page, unsigned long start_byte, size_t srclen,
 		size_t destlen);
+struct list_head *lzo_alloc_workspace(unsigned int level);
+void lzo_free_workspace(struct list_head *ws);
+struct list_head *lzo_get_workspace(unsigned int level);
+void lzo_put_workspace(struct list_head *ws);
 
 int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
 		u64 start, struct page **pages, unsigned long *out_pages,
@@ -54,6 +62,10 @@ int zstd_decompress(struct list_head *ws, unsigned char *data_in,
 		size_t destlen);
 void zstd_init_workspace_manager(void);
 void zstd_cleanup_workspace_manager(void);
+struct list_head *zstd_alloc_workspace(unsigned int level);
+void zstd_free_workspace(struct list_head *ws);
+struct list_head *zstd_get_workspace(unsigned int level);
+void zstd_put_workspace(struct list_head *ws);
 
 static const char* const btrfs_compress_types[] = { "", "zlib", "lzo", "zstd" };
 
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index 6f4619e76c11..18018619c019 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -63,17 +63,17 @@ struct workspace {
 
 static struct workspace_manager wsm;
 
-static struct list_head *lzo_get_workspace(unsigned int level)
+struct list_head *lzo_get_workspace(unsigned int level)
 {
 	return btrfs_get_workspace(&wsm, level);
 }
 
-static void lzo_put_workspace(struct list_head *ws)
+void lzo_put_workspace(struct list_head *ws)
 {
 	btrfs_put_workspace(&wsm, ws);
 }
 
-static void lzo_free_workspace(struct list_head *ws)
+void lzo_free_workspace(struct list_head *ws)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
 
@@ -83,7 +83,7 @@ static void lzo_free_workspace(struct list_head *ws)
 	kfree(workspace);
 }
 
-static struct list_head *lzo_alloc_workspace(unsigned int level)
+struct list_head *lzo_alloc_workspace(unsigned int level)
 {
 	struct workspace *workspace;
 
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 03c632c7deac..f2a56e999e5f 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -29,7 +29,7 @@ struct workspace {
 
 static struct workspace_manager wsm;
 
-static struct list_head *zlib_get_workspace(unsigned int level)
+struct list_head *zlib_get_workspace(unsigned int level)
 {
 	struct list_head *ws = btrfs_get_workspace(&wsm, level);
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
@@ -39,12 +39,12 @@ static struct list_head *zlib_get_workspace(unsigned int level)
 	return ws;
 }
 
-static void zlib_put_workspace(struct list_head *ws)
+void zlib_put_workspace(struct list_head *ws)
 {
 	btrfs_put_workspace(&wsm, ws);
 }
 
-static void zlib_free_workspace(struct list_head *ws)
+void zlib_free_workspace(struct list_head *ws)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
 
@@ -53,7 +53,7 @@ static void zlib_free_workspace(struct list_head *ws)
 	kfree(workspace);
 }
 
-static struct list_head *zlib_alloc_workspace(unsigned int level)
+struct list_head *zlib_alloc_workspace(unsigned int level)
 {
 	struct workspace *workspace;
 	int workspacesize;
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index f575ce77ea3d..2caf08e06e2f 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -91,9 +91,8 @@ static inline struct workspace *list_to_workspace(struct list_head *list)
 	return container_of(list, struct workspace, list);
 }
 
-static void zstd_free_workspace(struct list_head *ws);
-static struct list_head *zstd_alloc_workspace(unsigned int level);
-
+void zstd_free_workspace(struct list_head *ws);
+struct list_head *zstd_alloc_workspace(unsigned int level);
 /*
  * zstd_reclaim_timer_fn - reclaim timer
  * @t: timer
@@ -261,7 +260,7 @@ static struct list_head *zstd_find_workspace(unsigned int level)
  * attempt to allocate a new workspace.  If we fail to allocate one due to
  * memory pressure, go to sleep waiting for the max level workspace to free up.
  */
-static struct list_head *zstd_get_workspace(unsigned int level)
+struct list_head *zstd_get_workspace(unsigned int level)
 {
 	struct list_head *ws;
 	unsigned int nofs_flag;
@@ -302,7 +301,7 @@ static struct list_head *zstd_get_workspace(unsigned int level)
  * isn't set, it is also set here.  Only the max level workspace tries and wakes
  * up waiting workspaces.
  */
-static void zstd_put_workspace(struct list_head *ws)
+void zstd_put_workspace(struct list_head *ws)
 {
 	struct workspace *workspace = list_to_workspace(ws);
 
@@ -332,7 +331,7 @@ static void zstd_put_workspace(struct list_head *ws)
 		cond_wake_up(&wsm.wait);
 }
 
-static void zstd_free_workspace(struct list_head *ws)
+void zstd_free_workspace(struct list_head *ws)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
 
@@ -341,7 +340,7 @@ static void zstd_free_workspace(struct list_head *ws)
 	kfree(workspace);
 }
 
-static struct list_head *zstd_alloc_workspace(unsigned int level)
+struct list_head *zstd_alloc_workspace(unsigned int level)
 {
 	struct workspace *workspace;
 
-- 
2.23.0


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

* [PATCH 09/15] btrfs: compression: inline get_workspace
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
                   ` (7 preceding siblings ...)
  2019-10-14 12:22 ` [PATCH 08/15] btrfs: compression: export alloc/free/get/put callbacks of all algos David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17 11:46   ` Johannes Thumshirn
  2019-10-14 12:22 ` [PATCH 10/15] btrfs: compression: inline put_workspace David Sterba
                   ` (7 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

Majority of the callbacks is trivial, we don't gain anything by the
indirection, so replace them by a switch function.

ZLIB needs to adjust level in the callback and ZSTD workspace management
is complex, the rest is call to the helper.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 23 +++++++++++++++--------
 fs/btrfs/compression.h |  2 --
 fs/btrfs/lzo.c         |  6 ------
 fs/btrfs/zlib.c        |  1 -
 fs/btrfs/zstd.c        |  1 -
 5 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index f707db4a9a53..de9b06574f42 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -50,7 +50,6 @@ int lzo_decompress(struct list_head *ws, unsigned char *data_in,
 		size_t destlen);
 struct list_head *lzo_alloc_workspace(unsigned int level);
 void lzo_free_workspace(struct list_head *ws);
-struct list_head *lzo_get_workspace(unsigned int level);
 void lzo_put_workspace(struct list_head *ws);
 
 int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
@@ -874,11 +873,6 @@ struct heuristic_ws {
 
 static struct workspace_manager heuristic_wsm;
 
-static struct list_head *heuristic_get_workspace(unsigned int level)
-{
-	return btrfs_get_workspace(&heuristic_wsm, level);
-}
-
 static void heuristic_put_workspace(struct list_head *ws)
 {
 	btrfs_put_workspace(&heuristic_wsm, ws);
@@ -925,7 +919,6 @@ static struct list_head *alloc_heuristic_ws(unsigned int level)
 
 const struct btrfs_compress_op btrfs_heuristic_compress = {
 	.workspace_manager = &heuristic_wsm,
-	.get_workspace = heuristic_get_workspace,
 	.put_workspace = heuristic_put_workspace,
 	.alloc_workspace = alloc_heuristic_ws,
 	.free_workspace = free_heuristic_ws,
@@ -1067,7 +1060,21 @@ struct list_head *btrfs_get_workspace(struct workspace_manager *wsm,
 
 static struct list_head *get_workspace(int type, int level)
 {
-	return btrfs_compress_op[type]->get_workspace(level);
+	struct workspace_manager *wsm;
+
+	wsm = btrfs_compress_op[type]->workspace_manager;
+	switch (type) {
+	case BTRFS_COMPRESS_NONE: return btrfs_get_workspace(wsm, level);
+	case BTRFS_COMPRESS_ZLIB: return zlib_get_workspace(level);
+	case BTRFS_COMPRESS_LZO:  return btrfs_get_workspace(wsm, level);
+	case BTRFS_COMPRESS_ZSTD: return zstd_get_workspace(level);
+	default:
+		/*
+		 * This can't happen, the type is validated several times
+		 * before we get here.
+		 */
+		BUG();
+	}
 }
 
 /*
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index cf667e599b70..df7274c1a5d8 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -125,8 +125,6 @@ struct list_head *btrfs_get_workspace(struct workspace_manager *wsm,
 void btrfs_put_workspace(struct workspace_manager *wsm, struct list_head *ws);
 
 struct btrfs_compress_op {
-	struct list_head *(*get_workspace)(unsigned int level);
-
 	void (*put_workspace)(struct list_head *ws);
 
 	struct list_head *(*alloc_workspace)(unsigned int level);
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index 18018619c019..821e5c137971 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -63,11 +63,6 @@ struct workspace {
 
 static struct workspace_manager wsm;
 
-struct list_head *lzo_get_workspace(unsigned int level)
-{
-	return btrfs_get_workspace(&wsm, level);
-}
-
 void lzo_put_workspace(struct list_head *ws)
 {
 	btrfs_put_workspace(&wsm, ws);
@@ -494,7 +489,6 @@ int lzo_decompress(struct list_head *ws, unsigned char *data_in,
 
 const struct btrfs_compress_op btrfs_lzo_compress = {
 	.workspace_manager	= &wsm,
-	.get_workspace		= lzo_get_workspace,
 	.put_workspace		= lzo_put_workspace,
 	.alloc_workspace	= lzo_alloc_workspace,
 	.free_workspace		= lzo_free_workspace,
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index f2a56e999e5f..7caa468efe94 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -405,7 +405,6 @@ int zlib_decompress(struct list_head *ws, unsigned char *data_in,
 
 const struct btrfs_compress_op btrfs_zlib_compress = {
 	.workspace_manager	= &wsm,
-	.get_workspace		= zlib_get_workspace,
 	.put_workspace		= zlib_put_workspace,
 	.alloc_workspace	= zlib_alloc_workspace,
 	.free_workspace		= zlib_free_workspace,
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 2caf08e06e2f..c9fe0e2bd107 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -708,7 +708,6 @@ int zstd_decompress(struct list_head *ws, unsigned char *data_in,
 const struct btrfs_compress_op btrfs_zstd_compress = {
 	/* ZSTD uses own workspace manager */
 	.workspace_manager = NULL,
-	.get_workspace = zstd_get_workspace,
 	.put_workspace = zstd_put_workspace,
 	.alloc_workspace = zstd_alloc_workspace,
 	.free_workspace = zstd_free_workspace,
-- 
2.23.0


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

* [PATCH 10/15] btrfs: compression: inline put_workspace
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
                   ` (8 preceding siblings ...)
  2019-10-14 12:22 ` [PATCH 09/15] btrfs: compression: inline get_workspace David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17 11:49   ` Johannes Thumshirn
  2019-10-14 12:22 ` [PATCH 11/15] btrfs: compression: pass type to btrfs_get_workspace David Sterba
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

Similar to get_workspace, majority of the callbacks is trivial, we don't
gain anything by the indirection, so replace them by a switch function.
Trivial callback implementations use the helper.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 24 +++++++++++++++---------
 fs/btrfs/compression.h |  2 --
 fs/btrfs/lzo.c         |  6 ------
 fs/btrfs/zlib.c        |  6 ------
 fs/btrfs/zstd.c        |  1 -
 5 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index de9b06574f42..9b9638557e19 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -39,7 +39,6 @@ int zlib_decompress(struct list_head *ws, unsigned char *data_in,
 struct list_head *zlib_alloc_workspace(unsigned int level);
 void zlib_free_workspace(struct list_head *ws);
 struct list_head *zlib_get_workspace(unsigned int level);
-void zlib_put_workspace(struct list_head *ws);
 
 int lzo_compress_pages(struct list_head *ws, struct address_space *mapping,
 		u64 start, struct page **pages, unsigned long *out_pages,
@@ -50,7 +49,6 @@ int lzo_decompress(struct list_head *ws, unsigned char *data_in,
 		size_t destlen);
 struct list_head *lzo_alloc_workspace(unsigned int level);
 void lzo_free_workspace(struct list_head *ws);
-void lzo_put_workspace(struct list_head *ws);
 
 int zstd_compress_pages(struct list_head *ws, struct address_space *mapping,
 		u64 start, struct page **pages, unsigned long *out_pages,
@@ -873,11 +871,6 @@ struct heuristic_ws {
 
 static struct workspace_manager heuristic_wsm;
 
-static void heuristic_put_workspace(struct list_head *ws)
-{
-	btrfs_put_workspace(&heuristic_wsm, ws);
-}
-
 static void free_heuristic_ws(struct list_head *ws)
 {
 	struct heuristic_ws *workspace;
@@ -919,7 +912,6 @@ static struct list_head *alloc_heuristic_ws(unsigned int level)
 
 const struct btrfs_compress_op btrfs_heuristic_compress = {
 	.workspace_manager = &heuristic_wsm,
-	.put_workspace = heuristic_put_workspace,
 	.alloc_workspace = alloc_heuristic_ws,
 	.free_workspace = free_heuristic_ws,
 };
@@ -1112,7 +1104,21 @@ void btrfs_put_workspace(struct workspace_manager *wsm, struct list_head *ws)
 
 static void put_workspace(int type, struct list_head *ws)
 {
-	return btrfs_compress_op[type]->put_workspace(ws);
+	struct workspace_manager *wsm;
+
+	wsm = btrfs_compress_op[type]->workspace_manager;
+	switch (type) {
+	case BTRFS_COMPRESS_NONE: return btrfs_put_workspace(wsm, ws);
+	case BTRFS_COMPRESS_ZLIB: return btrfs_put_workspace(wsm, ws);
+	case BTRFS_COMPRESS_LZO:  return btrfs_put_workspace(wsm, ws);
+	case BTRFS_COMPRESS_ZSTD: return zstd_put_workspace(ws);
+	default:
+		/*
+		 * This can't happen, the type is validated several times
+		 * before we get here.
+		 */
+		BUG();
+	}
 }
 
 /*
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index df7274c1a5d8..b8ed97fc6db4 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -125,8 +125,6 @@ struct list_head *btrfs_get_workspace(struct workspace_manager *wsm,
 void btrfs_put_workspace(struct workspace_manager *wsm, struct list_head *ws);
 
 struct btrfs_compress_op {
-	void (*put_workspace)(struct list_head *ws);
-
 	struct list_head *(*alloc_workspace)(unsigned int level);
 
 	void (*free_workspace)(struct list_head *workspace);
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index 821e5c137971..bbf917c5ff2d 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -63,11 +63,6 @@ struct workspace {
 
 static struct workspace_manager wsm;
 
-void lzo_put_workspace(struct list_head *ws)
-{
-	btrfs_put_workspace(&wsm, ws);
-}
-
 void lzo_free_workspace(struct list_head *ws)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
@@ -489,7 +484,6 @@ int lzo_decompress(struct list_head *ws, unsigned char *data_in,
 
 const struct btrfs_compress_op btrfs_lzo_compress = {
 	.workspace_manager	= &wsm,
-	.put_workspace		= lzo_put_workspace,
 	.alloc_workspace	= lzo_alloc_workspace,
 	.free_workspace		= lzo_free_workspace,
 	.max_level		= 1,
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 7caa468efe94..610765640c8e 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -39,11 +39,6 @@ struct list_head *zlib_get_workspace(unsigned int level)
 	return ws;
 }
 
-void zlib_put_workspace(struct list_head *ws)
-{
-	btrfs_put_workspace(&wsm, ws);
-}
-
 void zlib_free_workspace(struct list_head *ws)
 {
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
@@ -405,7 +400,6 @@ int zlib_decompress(struct list_head *ws, unsigned char *data_in,
 
 const struct btrfs_compress_op btrfs_zlib_compress = {
 	.workspace_manager	= &wsm,
-	.put_workspace		= zlib_put_workspace,
 	.alloc_workspace	= zlib_alloc_workspace,
 	.free_workspace		= zlib_free_workspace,
 	.max_level		= 9,
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index c9fe0e2bd107..a346f1187fae 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -708,7 +708,6 @@ int zstd_decompress(struct list_head *ws, unsigned char *data_in,
 const struct btrfs_compress_op btrfs_zstd_compress = {
 	/* ZSTD uses own workspace manager */
 	.workspace_manager = NULL,
-	.put_workspace = zstd_put_workspace,
 	.alloc_workspace = zstd_alloc_workspace,
 	.free_workspace = zstd_free_workspace,
 	.max_level	= ZSTD_BTRFS_MAX_LEVEL,
-- 
2.23.0


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

* [PATCH 11/15] btrfs: compression: pass type to btrfs_get_workspace
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
                   ` (9 preceding siblings ...)
  2019-10-14 12:22 ` [PATCH 10/15] btrfs: compression: inline put_workspace David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17 11:53   ` Johannes Thumshirn
  2019-10-14 12:22 ` [PATCH 12/15] btrfs: compression: inline alloc_workspace David Sterba
                   ` (5 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

We can infer the workspace_manager from type and the type will be used
in the following patch to call a common helper for alloc_workspace.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 12 +++++-------
 fs/btrfs/compression.h |  3 +--
 fs/btrfs/zlib.c        |  2 +-
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 9b9638557e19..ffc94e15d86e 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -972,9 +972,9 @@ static void btrfs_cleanup_workspace_manager(int type)
  * Preallocation makes a forward progress guarantees and we do not return
  * errors.
  */
-struct list_head *btrfs_get_workspace(struct workspace_manager *wsm,
-				      unsigned int level)
+struct list_head *btrfs_get_workspace(int type, unsigned int level)
 {
+	struct workspace_manager *wsm;
 	struct list_head *workspace;
 	int cpus = num_online_cpus();
 	unsigned nofs_flag;
@@ -984,6 +984,7 @@ struct list_head *btrfs_get_workspace(struct workspace_manager *wsm,
 	wait_queue_head_t *ws_wait;
 	int *free_ws;
 
+	wsm = btrfs_compress_op[type]->workspace_manager;
 	idle_ws	 = &wsm->idle_ws;
 	ws_lock	 = &wsm->ws_lock;
 	total_ws = &wsm->total_ws;
@@ -1052,13 +1053,10 @@ struct list_head *btrfs_get_workspace(struct workspace_manager *wsm,
 
 static struct list_head *get_workspace(int type, int level)
 {
-	struct workspace_manager *wsm;
-
-	wsm = btrfs_compress_op[type]->workspace_manager;
 	switch (type) {
-	case BTRFS_COMPRESS_NONE: return btrfs_get_workspace(wsm, level);
+	case BTRFS_COMPRESS_NONE: return btrfs_get_workspace(type, level);
 	case BTRFS_COMPRESS_ZLIB: return zlib_get_workspace(level);
-	case BTRFS_COMPRESS_LZO:  return btrfs_get_workspace(wsm, level);
+	case BTRFS_COMPRESS_LZO:  return btrfs_get_workspace(type, level);
 	case BTRFS_COMPRESS_ZSTD: return zstd_get_workspace(level);
 	default:
 		/*
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index b8ed97fc6db4..accb1d61df87 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -120,8 +120,7 @@ struct workspace_manager {
 	wait_queue_head_t ws_wait;
 };
 
-struct list_head *btrfs_get_workspace(struct workspace_manager *wsm,
-				      unsigned int level);
+struct list_head *btrfs_get_workspace(int type, unsigned int level);
 void btrfs_put_workspace(struct workspace_manager *wsm, struct list_head *ws);
 
 struct btrfs_compress_op {
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 610765640c8e..5679a2e41a52 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -31,7 +31,7 @@ static struct workspace_manager wsm;
 
 struct list_head *zlib_get_workspace(unsigned int level)
 {
-	struct list_head *ws = btrfs_get_workspace(&wsm, level);
+	struct list_head *ws = btrfs_get_workspace(BTRFS_COMPRESS_ZLIB, level);
 	struct workspace *workspace = list_entry(ws, struct workspace, list);
 
 	workspace->level = level;
-- 
2.23.0


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

* [PATCH 12/15] btrfs: compression: inline alloc_workspace
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
                   ` (10 preceding siblings ...)
  2019-10-14 12:22 ` [PATCH 11/15] btrfs: compression: pass type to btrfs_get_workspace David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17 11:56   ` Johannes Thumshirn
  2019-10-14 12:22 ` [PATCH 13/15] btrfs: compression: pass type to btrfs_put_workspace David Sterba
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

Replace indirect calls to alloc_workspace by switch and calls to the
specific callbacks. This is mainly to get rid of the indirection due to
spectre vulnerability mitigations.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 21 ++++++++++++++++++---
 fs/btrfs/compression.h |  2 --
 fs/btrfs/lzo.c         |  1 -
 fs/btrfs/zlib.c        |  1 -
 fs/btrfs/zstd.c        |  1 -
 5 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index ffc94e15d86e..4a8dab961f88 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -912,7 +912,6 @@ static struct list_head *alloc_heuristic_ws(unsigned int level)
 
 const struct btrfs_compress_op btrfs_heuristic_compress = {
 	.workspace_manager = &heuristic_wsm,
-	.alloc_workspace = alloc_heuristic_ws,
 	.free_workspace = free_heuristic_ws,
 };
 
@@ -924,6 +923,22 @@ static const struct btrfs_compress_op * const btrfs_compress_op[] = {
 	&btrfs_zstd_compress,
 };
 
+static struct list_head *alloc_workspace(int type, unsigned int level)
+{
+	switch (type) {
+	case BTRFS_COMPRESS_NONE: return alloc_heuristic_ws(level);
+	case BTRFS_COMPRESS_ZLIB: return zlib_alloc_workspace(level);
+	case BTRFS_COMPRESS_LZO:  return lzo_alloc_workspace(level);
+	case BTRFS_COMPRESS_ZSTD: return zstd_alloc_workspace(level);
+	default:
+		/*
+		 * This can't happen, the type is validated several times
+		 * before we get here.
+		 */
+		BUG();
+	}
+}
+
 static void btrfs_init_workspace_manager(int type)
 {
 	const struct btrfs_compress_op *ops = btrfs_compress_op[type];
@@ -941,7 +956,7 @@ static void btrfs_init_workspace_manager(int type)
 	 * Preallocate one workspace for each compression type so we can
 	 * guarantee forward progress in the worst case
 	 */
-	workspace = wsm->ops->alloc_workspace(0);
+	workspace = alloc_workspace(type, 0);
 	if (IS_ERR(workspace)) {
 		pr_warn(
 	"BTRFS: cannot preallocate compression workspace, will try later\n");
@@ -1020,7 +1035,7 @@ struct list_head *btrfs_get_workspace(int type, unsigned int level)
 	 * context of btrfs_compress_bio/btrfs_compress_pages
 	 */
 	nofs_flag = memalloc_nofs_save();
-	workspace = wsm->ops->alloc_workspace(level);
+	workspace = alloc_workspace(type, level);
 	memalloc_nofs_restore(nofs_flag);
 
 	if (IS_ERR(workspace)) {
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index accb1d61df87..8336c2ef6b5a 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -124,8 +124,6 @@ struct list_head *btrfs_get_workspace(int type, unsigned int level);
 void btrfs_put_workspace(struct workspace_manager *wsm, struct list_head *ws);
 
 struct btrfs_compress_op {
-	struct list_head *(*alloc_workspace)(unsigned int level);
-
 	void (*free_workspace)(struct list_head *workspace);
 
 	struct workspace_manager *workspace_manager;
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index bbf917c5ff2d..39b2cf80f77c 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -484,7 +484,6 @@ int lzo_decompress(struct list_head *ws, unsigned char *data_in,
 
 const struct btrfs_compress_op btrfs_lzo_compress = {
 	.workspace_manager	= &wsm,
-	.alloc_workspace	= lzo_alloc_workspace,
 	.free_workspace		= lzo_free_workspace,
 	.max_level		= 1,
 	.default_level		= 1,
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 5679a2e41a52..c5dfab3ab082 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -400,7 +400,6 @@ int zlib_decompress(struct list_head *ws, unsigned char *data_in,
 
 const struct btrfs_compress_op btrfs_zlib_compress = {
 	.workspace_manager	= &wsm,
-	.alloc_workspace	= zlib_alloc_workspace,
 	.free_workspace		= zlib_free_workspace,
 	.max_level		= 9,
 	.default_level		= BTRFS_ZLIB_DEFAULT_LEVEL,
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index a346f1187fae..9413f741c2f6 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -708,7 +708,6 @@ int zstd_decompress(struct list_head *ws, unsigned char *data_in,
 const struct btrfs_compress_op btrfs_zstd_compress = {
 	/* ZSTD uses own workspace manager */
 	.workspace_manager = NULL,
-	.alloc_workspace = zstd_alloc_workspace,
 	.free_workspace = zstd_free_workspace,
 	.max_level	= ZSTD_BTRFS_MAX_LEVEL,
 	.default_level	= ZSTD_BTRFS_DEFAULT_LEVEL,
-- 
2.23.0


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

* [PATCH 13/15] btrfs: compression: pass type to btrfs_put_workspace
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
                   ` (11 preceding siblings ...)
  2019-10-14 12:22 ` [PATCH 12/15] btrfs: compression: inline alloc_workspace David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17 11:57   ` Johannes Thumshirn
  2019-10-14 12:22 ` [PATCH 14/15] btrfs: compression: inline free_workspace David Sterba
                   ` (3 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

We can infer the workspace_manager from type and the type will be used
in the following patch to call a common helper for free_workspace.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 13 ++++++-------
 fs/btrfs/compression.h |  2 +-
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 4a8dab961f88..2a77c91c194b 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -1086,14 +1086,16 @@ static struct list_head *get_workspace(int type, int level)
  * put a workspace struct back on the list or free it if we have enough
  * idle ones sitting around
  */
-void btrfs_put_workspace(struct workspace_manager *wsm, struct list_head *ws)
+void btrfs_put_workspace(int type, struct list_head *ws)
 {
+	struct workspace_manager *wsm;
 	struct list_head *idle_ws;
 	spinlock_t *ws_lock;
 	atomic_t *total_ws;
 	wait_queue_head_t *ws_wait;
 	int *free_ws;
 
+	wsm = btrfs_compress_op[type]->workspace_manager;
 	idle_ws	 = &wsm->idle_ws;
 	ws_lock	 = &wsm->ws_lock;
 	total_ws = &wsm->total_ws;
@@ -1117,13 +1119,10 @@ void btrfs_put_workspace(struct workspace_manager *wsm, struct list_head *ws)
 
 static void put_workspace(int type, struct list_head *ws)
 {
-	struct workspace_manager *wsm;
-
-	wsm = btrfs_compress_op[type]->workspace_manager;
 	switch (type) {
-	case BTRFS_COMPRESS_NONE: return btrfs_put_workspace(wsm, ws);
-	case BTRFS_COMPRESS_ZLIB: return btrfs_put_workspace(wsm, ws);
-	case BTRFS_COMPRESS_LZO:  return btrfs_put_workspace(wsm, ws);
+	case BTRFS_COMPRESS_NONE: return btrfs_put_workspace(type, ws);
+	case BTRFS_COMPRESS_ZLIB: return btrfs_put_workspace(type, ws);
+	case BTRFS_COMPRESS_LZO:  return btrfs_put_workspace(type, ws);
 	case BTRFS_COMPRESS_ZSTD: return zstd_put_workspace(ws);
 	default:
 		/*
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 8336c2ef6b5a..664b029cd5e4 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -121,7 +121,7 @@ struct workspace_manager {
 };
 
 struct list_head *btrfs_get_workspace(int type, unsigned int level);
-void btrfs_put_workspace(struct workspace_manager *wsm, struct list_head *ws);
+void btrfs_put_workspace(int type, struct list_head *ws);
 
 struct btrfs_compress_op {
 	void (*free_workspace)(struct list_head *workspace);
-- 
2.23.0


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

* [PATCH 14/15] btrfs: compression: inline free_workspace
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
                   ` (12 preceding siblings ...)
  2019-10-14 12:22 ` [PATCH 13/15] btrfs: compression: pass type to btrfs_put_workspace David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17 11:58   ` Johannes Thumshirn
  2019-10-14 12:22 ` [PATCH 15/15] btrfs: compression: remove ops pointer from workspace_manager David Sterba
                   ` (2 subsequent siblings)
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

Replace indirect calls to free_workspace by switch and calls to the
specific callbacks. This is mainly to get rid of the indirection due to
spectre vulnerability mitigations.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 21 ++++++++++++++++++---
 fs/btrfs/compression.h |  2 --
 fs/btrfs/lzo.c         |  1 -
 fs/btrfs/zlib.c        |  1 -
 fs/btrfs/zstd.c        |  1 -
 5 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 2a77c91c194b..b2342f99b093 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -912,7 +912,6 @@ static struct list_head *alloc_heuristic_ws(unsigned int level)
 
 const struct btrfs_compress_op btrfs_heuristic_compress = {
 	.workspace_manager = &heuristic_wsm,
-	.free_workspace = free_heuristic_ws,
 };
 
 static const struct btrfs_compress_op * const btrfs_compress_op[] = {
@@ -939,6 +938,22 @@ static struct list_head *alloc_workspace(int type, unsigned int level)
 	}
 }
 
+static void free_workspace(int type, struct list_head *ws)
+{
+	switch (type) {
+	case BTRFS_COMPRESS_NONE: return free_heuristic_ws(ws);
+	case BTRFS_COMPRESS_ZLIB: return zlib_free_workspace(ws);
+	case BTRFS_COMPRESS_LZO:  return lzo_free_workspace(ws);
+	case BTRFS_COMPRESS_ZSTD: return zstd_free_workspace(ws);
+	default:
+		/*
+		 * This can't happen, the type is validated several times
+		 * before we get here.
+		 */
+		BUG();
+	}
+}
+
 static void btrfs_init_workspace_manager(int type)
 {
 	const struct btrfs_compress_op *ops = btrfs_compress_op[type];
@@ -976,7 +991,7 @@ static void btrfs_cleanup_workspace_manager(int type)
 	while (!list_empty(&wsman->idle_ws)) {
 		ws = wsman->idle_ws.next;
 		list_del(ws);
-		wsman->ops->free_workspace(ws);
+		free_workspace(type, ws);
 		atomic_dec(&wsman->total_ws);
 	}
 }
@@ -1111,7 +1126,7 @@ void btrfs_put_workspace(int type, struct list_head *ws)
 	}
 	spin_unlock(ws_lock);
 
-	wsm->ops->free_workspace(ws);
+	free_workspace(type, ws);
 	atomic_dec(total_ws);
 wake:
 	cond_wake_up(ws_wait);
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 664b029cd5e4..14057498dcbb 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -124,8 +124,6 @@ struct list_head *btrfs_get_workspace(int type, unsigned int level);
 void btrfs_put_workspace(int type, struct list_head *ws);
 
 struct btrfs_compress_op {
-	void (*free_workspace)(struct list_head *workspace);
-
 	struct workspace_manager *workspace_manager;
 	/* Maximum level supported by the compression algorithm */
 	unsigned int max_level;
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index 39b2cf80f77c..aa9cd11f4b78 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -484,7 +484,6 @@ int lzo_decompress(struct list_head *ws, unsigned char *data_in,
 
 const struct btrfs_compress_op btrfs_lzo_compress = {
 	.workspace_manager	= &wsm,
-	.free_workspace		= lzo_free_workspace,
 	.max_level		= 1,
 	.default_level		= 1,
 };
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index c5dfab3ab082..a6c90a003c12 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -400,7 +400,6 @@ int zlib_decompress(struct list_head *ws, unsigned char *data_in,
 
 const struct btrfs_compress_op btrfs_zlib_compress = {
 	.workspace_manager	= &wsm,
-	.free_workspace		= zlib_free_workspace,
 	.max_level		= 9,
 	.default_level		= BTRFS_ZLIB_DEFAULT_LEVEL,
 };
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 9413f741c2f6..9a4871636c6c 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -708,7 +708,6 @@ int zstd_decompress(struct list_head *ws, unsigned char *data_in,
 const struct btrfs_compress_op btrfs_zstd_compress = {
 	/* ZSTD uses own workspace manager */
 	.workspace_manager = NULL,
-	.free_workspace = zstd_free_workspace,
 	.max_level	= ZSTD_BTRFS_MAX_LEVEL,
 	.default_level	= ZSTD_BTRFS_DEFAULT_LEVEL,
 };
-- 
2.23.0


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

* [PATCH 15/15] btrfs: compression: remove ops pointer from workspace_manager
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
                   ` (13 preceding siblings ...)
  2019-10-14 12:22 ` [PATCH 14/15] btrfs: compression: inline free_workspace David Sterba
@ 2019-10-14 12:22 ` David Sterba
  2019-10-17 11:59   ` Johannes Thumshirn
  2019-10-17 15:02 ` [PATCH 00/15] Remove callback indirections in compression code Nikolay Borisov
  2019-10-17 18:19 ` David Sterba
  16 siblings, 1 reply; 34+ messages in thread
From: David Sterba @ 2019-10-14 12:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba

We can infer the ops from the type that is now passed to all functions
that would need it, this makes workspace_manager::ops redundant and can
be removed.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/compression.c | 6 ++----
 fs/btrfs/compression.h | 1 -
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index b2342f99b093..53aee0db9d71 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -956,12 +956,10 @@ static void free_workspace(int type, struct list_head *ws)
 
 static void btrfs_init_workspace_manager(int type)
 {
-	const struct btrfs_compress_op *ops = btrfs_compress_op[type];
-	struct workspace_manager *wsm = ops->workspace_manager;
+	struct workspace_manager *wsm;
 	struct list_head *workspace;
 
-	wsm->ops = ops;
-
+	wsm = btrfs_compress_op[type]->workspace_manager;
 	INIT_LIST_HEAD(&wsm->idle_ws);
 	spin_lock_init(&wsm->ws_lock);
 	atomic_set(&wsm->total_ws, 0);
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 14057498dcbb..d253f7aa8ed5 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -109,7 +109,6 @@ enum btrfs_compression_type {
 };
 
 struct workspace_manager {
-	const struct btrfs_compress_op *ops;
 	struct list_head idle_ws;
 	spinlock_t ws_lock;
 	/* Number of free workspaces */
-- 
2.23.0


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

* Re: [PATCH 01/15] btrfs: export compression and decompression callbacks
  2019-10-14 12:22 ` [PATCH 01/15] btrfs: export compression and decompression callbacks David Sterba
@ 2019-10-17  9:39   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17  9:39 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 02/15] btrfs: switch compression callbacks to direct calls
  2019-10-14 12:22 ` [PATCH 02/15] btrfs: switch compression callbacks to direct calls David Sterba
@ 2019-10-17  9:42   ` Johannes Thumshirn
  2019-10-17 11:24     ` David Sterba
  0 siblings, 1 reply; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17  9:42 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

On 14/10/2019 14:22, David Sterba wrote:
> +static int compression_decompress_bio(int type, struct list_head *ws,
> +		struct compressed_bio *cb)
> +{
> +	switch (type) {
> +	case BTRFS_COMPRESS_ZLIB: return zlib_decompress_bio(ws, cb);
> +	case BTRFS_COMPRESS_LZO:  return lzo_decompress_bio(ws, cb);
> +	case BTRFS_COMPRESS_ZSTD: return zstd_decompress_bio(ws, cb);
> +	case BTRFS_COMPRESS_NONE:
> +	default:
> +		/*
> +		 * This can't happen, the type is validated several times
> +		 * before we get here.
> +		 */
> +		BUG();
> +	}
> +}
> +
> +static int compression_decompress(int type, struct list_head *ws,
> +               unsigned char *data_in, struct page *dest_page,
> +               unsigned long start_byte, size_t srclen, size_t destlen)
> +{
> +	switch (type) {
> +	case BTRFS_COMPRESS_ZLIB: return zlib_decompress(ws, data_in, dest_page,
> +						start_byte, srclen, destlen);
> +	case BTRFS_COMPRESS_LZO:  return lzo_decompress(ws, data_in, dest_page,
> +						start_byte, srclen, destlen);
> +	case BTRFS_COMPRESS_ZSTD: return zstd_decompress(ws, data_in, dest_page,
> +						start_byte, srclen, destlen);
> +	case BTRFS_COMPRESS_NONE:
> +	default:
> +		/*
> +		 * This can't happen, the type is validated several times
> +		 * before we get here.
> +		 */
> +		BUG();
> +	}
> +}

Hmm should we really BUG() here? Maybe throw in an ASSERT(), so we can
catch it in debug builds but not halt the machine (even if it
theoretically can't happen).

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 04/15] btrfs: compression: let workspace manager init take only the type
  2019-10-14 12:22 ` [PATCH 04/15] btrfs: compression: let workspace manager init take only the type David Sterba
@ 2019-10-17 11:03   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17 11:03 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 05/15] btrfs: compression: inline init_workspace_manager
  2019-10-14 12:22 ` [PATCH 05/15] btrfs: compression: inline init_workspace_manager David Sterba
@ 2019-10-17 11:06   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17 11:06 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 03/15] btrfs: compression: attach workspace manager to the ops
  2019-10-14 12:22 ` [PATCH 03/15] btrfs: compression: attach workspace manager to the ops David Sterba
@ 2019-10-17 11:08   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17 11:08 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 06/15] btrfs: compression: let workspace manager cleanup take only the type
  2019-10-14 12:22 ` [PATCH 06/15] btrfs: compression: let workspace manager cleanup take only the type David Sterba
@ 2019-10-17 11:10   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17 11:10 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 02/15] btrfs: switch compression callbacks to direct calls
  2019-10-17  9:42   ` Johannes Thumshirn
@ 2019-10-17 11:24     ` David Sterba
  0 siblings, 0 replies; 34+ messages in thread
From: David Sterba @ 2019-10-17 11:24 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: David Sterba, linux-btrfs

On Thu, Oct 17, 2019 at 11:42:53AM +0200, Johannes Thumshirn wrote:
> On 14/10/2019 14:22, David Sterba wrote:
> > +static int compression_decompress_bio(int type, struct list_head *ws,
> > +		struct compressed_bio *cb)
> > +{
> > +	switch (type) {
> > +	case BTRFS_COMPRESS_ZLIB: return zlib_decompress_bio(ws, cb);
> > +	case BTRFS_COMPRESS_LZO:  return lzo_decompress_bio(ws, cb);
> > +	case BTRFS_COMPRESS_ZSTD: return zstd_decompress_bio(ws, cb);
> > +	case BTRFS_COMPRESS_NONE:
> > +	default:
> > +		/*
> > +		 * This can't happen, the type is validated several times
> > +		 * before we get here.
> > +		 */
> > +		BUG();
> > +	}
> > +}
> > +
> > +static int compression_decompress(int type, struct list_head *ws,
> > +               unsigned char *data_in, struct page *dest_page,
> > +               unsigned long start_byte, size_t srclen, size_t destlen)
> > +{
> > +	switch (type) {
> > +	case BTRFS_COMPRESS_ZLIB: return zlib_decompress(ws, data_in, dest_page,
> > +						start_byte, srclen, destlen);
> > +	case BTRFS_COMPRESS_LZO:  return lzo_decompress(ws, data_in, dest_page,
> > +						start_byte, srclen, destlen);
> > +	case BTRFS_COMPRESS_ZSTD: return zstd_decompress(ws, data_in, dest_page,
> > +						start_byte, srclen, destlen);
> > +	case BTRFS_COMPRESS_NONE:
> > +	default:
> > +		/*
> > +		 * This can't happen, the type is validated several times
> > +		 * before we get here.
> > +		 */
> > +		BUG();
> > +	}
> > +}
> 
> Hmm should we really BUG() here? Maybe throw in an ASSERT(), so we can
> catch it in debug builds but not halt the machine (even if it
> theoretically can't happen).

Debug builds would catch it for sure, regardless of bug or assert, but
assert would not be on builds without CONFIG_BTRFS_ASSERT and continuing
here is not correct. If the code reatches the switch with a wrong value,
then the wrong value has been used to dereference an array in the
caller, so the state of the system is not defined. I don't see a better
option than BUG, thouth we don't want to add new ones. At least with the
comment it should be clear that it's not somebody's laziness to do
proper error handling, here it's not possible to do it at all.

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

* Re: [PATCH 07/15] btrfs: compression: inline cleanup_workspace_manager
  2019-10-14 12:22 ` [PATCH 07/15] btrfs: compression: inline cleanup_workspace_manager David Sterba
@ 2019-10-17 11:31   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17 11:31 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 08/15] btrfs: compression: export alloc/free/get/put callbacks of all algos
  2019-10-14 12:22 ` [PATCH 08/15] btrfs: compression: export alloc/free/get/put callbacks of all algos David Sterba
@ 2019-10-17 11:44   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17 11:44 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 09/15] btrfs: compression: inline get_workspace
  2019-10-14 12:22 ` [PATCH 09/15] btrfs: compression: inline get_workspace David Sterba
@ 2019-10-17 11:46   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17 11:46 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 10/15] btrfs: compression: inline put_workspace
  2019-10-14 12:22 ` [PATCH 10/15] btrfs: compression: inline put_workspace David Sterba
@ 2019-10-17 11:49   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17 11:49 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 11/15] btrfs: compression: pass type to btrfs_get_workspace
  2019-10-14 12:22 ` [PATCH 11/15] btrfs: compression: pass type to btrfs_get_workspace David Sterba
@ 2019-10-17 11:53   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17 11:53 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 12/15] btrfs: compression: inline alloc_workspace
  2019-10-14 12:22 ` [PATCH 12/15] btrfs: compression: inline alloc_workspace David Sterba
@ 2019-10-17 11:56   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17 11:56 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 13/15] btrfs: compression: pass type to btrfs_put_workspace
  2019-10-14 12:22 ` [PATCH 13/15] btrfs: compression: pass type to btrfs_put_workspace David Sterba
@ 2019-10-17 11:57   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17 11:57 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 14/15] btrfs: compression: inline free_workspace
  2019-10-14 12:22 ` [PATCH 14/15] btrfs: compression: inline free_workspace David Sterba
@ 2019-10-17 11:58   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17 11:58 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 15/15] btrfs: compression: remove ops pointer from workspace_manager
  2019-10-14 12:22 ` [PATCH 15/15] btrfs: compression: remove ops pointer from workspace_manager David Sterba
@ 2019-10-17 11:59   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2019-10-17 11:59 UTC (permalink / raw)
  To: David Sterba, linux-btrfs

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

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

* Re: [PATCH 00/15] Remove callback indirections in compression code
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
                   ` (14 preceding siblings ...)
  2019-10-14 12:22 ` [PATCH 15/15] btrfs: compression: remove ops pointer from workspace_manager David Sterba
@ 2019-10-17 15:02 ` Nikolay Borisov
  2019-10-17 18:19 ` David Sterba
  16 siblings, 0 replies; 34+ messages in thread
From: Nikolay Borisov @ 2019-10-17 15:02 UTC (permalink / raw)
  To: David Sterba, linux-btrfs



On 14.10.19 г. 15:22 ч., David Sterba wrote:
> The series removes all per-compression algorithm callbacks and replaces
> them with a switches. Lots of indirections are simplified and while it
> looks nice from design POV, we don't need to over-engineer it as we have
> only fixed known number of the users and not public API.
> 
> The cost of indirect function calls is also higher with enabled
> mitigations of the spectre vulnerability, so they've been incrementally
> removed from the whole kernel and from btrfs as well.
> 
> David Sterba (15):
>   btrfs: export compression and decompression callbacks
>   btrfs: switch compression callbacks to direct calls
>   btrfs: compression: attach workspace manager to the ops
>   btrfs: compression: let workspace manager init take only the type
>   btrfs: compression: inline init_workspace_manager
>   btrfs: compression: let workspace manager cleanup take only the type
>   btrfs: compression: inline cleanup_workspace_manager
>   btrfs: compression: export alloc/free/get/put callbacks of all algos
>   btrfs: compression: inline get_workspace
>   btrfs: compression: inline put_workspace
>   btrfs: compression: pass type to btrfs_get_workspace
>   btrfs: compression: inline alloc_workspace
>   btrfs: compression: pass type to btrfs_put_workspace
>   btrfs: compression: inline free_workspace
>   btrfs: compression: remove ops pointer from workspace_manager
> 
>  fs/btrfs/compression.c | 241 +++++++++++++++++++++++++++++++----------
>  fs/btrfs/compression.h |  39 +------
>  fs/btrfs/lzo.c         |  53 ++-------
>  fs/btrfs/zlib.c        |  52 ++-------
>  fs/btrfs/zstd.c        |  47 +++-----
>  5 files changed, 227 insertions(+), 205 deletions(-)

The whole series look good.

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> 

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

* Re: [PATCH 00/15] Remove callback indirections in compression code
  2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
                   ` (15 preceding siblings ...)
  2019-10-17 15:02 ` [PATCH 00/15] Remove callback indirections in compression code Nikolay Borisov
@ 2019-10-17 18:19 ` David Sterba
  16 siblings, 0 replies; 34+ messages in thread
From: David Sterba @ 2019-10-17 18:19 UTC (permalink / raw)
  To: David Sterba; +Cc: linux-btrfs

On Mon, Oct 14, 2019 at 02:22:21PM +0200, David Sterba wrote:
> The series removes all per-compression algorithm callbacks and replaces
> them with a switches. Lots of indirections are simplified and while it
> looks nice from design POV, we don't need to over-engineer it as we have
> only fixed known number of the users and not public API.
> 
> The cost of indirect function calls is also higher with enabled
> mitigations of the spectre vulnerability, so they've been incrementally
> removed from the whole kernel and from btrfs as well.
> 
> David Sterba (15):
>   btrfs: export compression and decompression callbacks
>   btrfs: switch compression callbacks to direct calls
>   btrfs: compression: attach workspace manager to the ops
>   btrfs: compression: let workspace manager init take only the type
>   btrfs: compression: inline init_workspace_manager
>   btrfs: compression: let workspace manager cleanup take only the type
>   btrfs: compression: inline cleanup_workspace_manager
>   btrfs: compression: export alloc/free/get/put callbacks of all algos
>   btrfs: compression: inline get_workspace
>   btrfs: compression: inline put_workspace
>   btrfs: compression: pass type to btrfs_get_workspace
>   btrfs: compression: inline alloc_workspace
>   btrfs: compression: pass type to btrfs_put_workspace
>   btrfs: compression: inline free_workspace
>   btrfs: compression: remove ops pointer from workspace_manager

Added to misc-next.

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

end of thread, other threads:[~2019-10-17 18:19 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-14 12:22 [PATCH 00/15] Remove callback indirections in compression code David Sterba
2019-10-14 12:22 ` [PATCH 01/15] btrfs: export compression and decompression callbacks David Sterba
2019-10-17  9:39   ` Johannes Thumshirn
2019-10-14 12:22 ` [PATCH 02/15] btrfs: switch compression callbacks to direct calls David Sterba
2019-10-17  9:42   ` Johannes Thumshirn
2019-10-17 11:24     ` David Sterba
2019-10-14 12:22 ` [PATCH 03/15] btrfs: compression: attach workspace manager to the ops David Sterba
2019-10-17 11:08   ` Johannes Thumshirn
2019-10-14 12:22 ` [PATCH 04/15] btrfs: compression: let workspace manager init take only the type David Sterba
2019-10-17 11:03   ` Johannes Thumshirn
2019-10-14 12:22 ` [PATCH 05/15] btrfs: compression: inline init_workspace_manager David Sterba
2019-10-17 11:06   ` Johannes Thumshirn
2019-10-14 12:22 ` [PATCH 06/15] btrfs: compression: let workspace manager cleanup take only the type David Sterba
2019-10-17 11:10   ` Johannes Thumshirn
2019-10-14 12:22 ` [PATCH 07/15] btrfs: compression: inline cleanup_workspace_manager David Sterba
2019-10-17 11:31   ` Johannes Thumshirn
2019-10-14 12:22 ` [PATCH 08/15] btrfs: compression: export alloc/free/get/put callbacks of all algos David Sterba
2019-10-17 11:44   ` Johannes Thumshirn
2019-10-14 12:22 ` [PATCH 09/15] btrfs: compression: inline get_workspace David Sterba
2019-10-17 11:46   ` Johannes Thumshirn
2019-10-14 12:22 ` [PATCH 10/15] btrfs: compression: inline put_workspace David Sterba
2019-10-17 11:49   ` Johannes Thumshirn
2019-10-14 12:22 ` [PATCH 11/15] btrfs: compression: pass type to btrfs_get_workspace David Sterba
2019-10-17 11:53   ` Johannes Thumshirn
2019-10-14 12:22 ` [PATCH 12/15] btrfs: compression: inline alloc_workspace David Sterba
2019-10-17 11:56   ` Johannes Thumshirn
2019-10-14 12:22 ` [PATCH 13/15] btrfs: compression: pass type to btrfs_put_workspace David Sterba
2019-10-17 11:57   ` Johannes Thumshirn
2019-10-14 12:22 ` [PATCH 14/15] btrfs: compression: inline free_workspace David Sterba
2019-10-17 11:58   ` Johannes Thumshirn
2019-10-14 12:22 ` [PATCH 15/15] btrfs: compression: remove ops pointer from workspace_manager David Sterba
2019-10-17 11:59   ` Johannes Thumshirn
2019-10-17 15:02 ` [PATCH 00/15] Remove callback indirections in compression code Nikolay Borisov
2019-10-17 18:19 ` David Sterba

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).