* [PATCH 0/2] Compression level API cleanups
@ 2019-08-09 14:55 David Sterba
2019-08-09 14:55 ` [PATCH 1/2] btrfs: define compression levels statically David Sterba
2019-08-09 14:55 ` [PATCH 2/2] btrfs: compression: replace set_level callbacks by a common helper David Sterba
0 siblings, 2 replies; 7+ messages in thread
From: David Sterba @ 2019-08-09 14:55 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
Remove the callback indirection.
David Sterba (2):
btrfs: define compression levels statically
btrfs: compression: replace set_level callbacks by a common helper
fs/btrfs/compression.c | 20 ++++++++++++++++++--
fs/btrfs/compression.h | 11 +++++------
fs/btrfs/lzo.c | 8 ++------
fs/btrfs/zlib.c | 11 ++---------
fs/btrfs/zstd.c | 11 ++---------
5 files changed, 29 insertions(+), 32 deletions(-)
--
2.22.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] btrfs: define compression levels statically
2019-08-09 14:55 [PATCH 0/2] Compression level API cleanups David Sterba
@ 2019-08-09 14:55 ` David Sterba
2019-08-12 8:30 ` Nikolay Borisov
2019-08-09 14:55 ` [PATCH 2/2] btrfs: compression: replace set_level callbacks by a common helper David Sterba
1 sibling, 1 reply; 7+ messages in thread
From: David Sterba @ 2019-08-09 14:55 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
The maximum and default levels do not change and can be defined
directly. The set_level callback was a temporary solution and will be
removed.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/compression.h | 4 ++++
fs/btrfs/lzo.c | 2 ++
fs/btrfs/zlib.c | 2 ++
fs/btrfs/zstd.c | 2 ++
4 files changed, 10 insertions(+)
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 2035b8eb1290..07b2009dc63f 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -162,6 +162,10 @@ struct btrfs_compress_op {
* if the level is out of bounds or the default if 0 is passed in.
*/
unsigned int (*set_level)(unsigned int level);
+
+ /* Maximum level supported by the compression algorithm */
+ int max_level;
+ int default_level;
};
/* The heuristic workspaces are managed via the 0th workspace manager */
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index 579d53ae256f..adac6cb30d65 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -523,4 +523,6 @@ const struct btrfs_compress_op btrfs_lzo_compress = {
.decompress_bio = lzo_decompress_bio,
.decompress = lzo_decompress,
.set_level = lzo_set_level,
+ .max_level = 1,
+ .default_level = 1,
};
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index b86b7ad6b900..03d6c3683bd9 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -437,4 +437,6 @@ const struct btrfs_compress_op btrfs_zlib_compress = {
.decompress_bio = zlib_decompress_bio,
.decompress = zlib_decompress,
.set_level = zlib_set_level,
+ .max_level = 9,
+ .default_level = BTRFS_ZLIB_DEFAULT_LEVEL,
};
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 3837ca180d52..b2b23a6a497d 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -729,4 +729,6 @@ const struct btrfs_compress_op btrfs_zstd_compress = {
.decompress_bio = zstd_decompress_bio,
.decompress = zstd_decompress,
.set_level = zstd_set_level,
+ .max_level = ZSTD_BTRFS_MAX_LEVEL,
+ .default_level = ZSTD_BTRFS_DEFAULT_LEVEL,
};
--
2.22.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] btrfs: compression: replace set_level callbacks by a common helper
2019-08-09 14:55 [PATCH 0/2] Compression level API cleanups David Sterba
2019-08-09 14:55 ` [PATCH 1/2] btrfs: define compression levels statically David Sterba
@ 2019-08-09 14:55 ` David Sterba
2019-08-11 13:05 ` kbuild test robot
2019-08-11 13:08 ` kbuild test robot
1 sibling, 2 replies; 7+ messages in thread
From: David Sterba @ 2019-08-09 14:55 UTC (permalink / raw)
To: linux-btrfs; +Cc: David Sterba
The set_level callbacks do not do anything special and can be replaced
by a helper that uses the levels defined in the tables.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/compression.c | 20 ++++++++++++++++++--
fs/btrfs/compression.h | 9 ++-------
fs/btrfs/lzo.c | 6 ------
fs/btrfs/zlib.c | 9 ---------
fs/btrfs/zstd.c | 9 ---------
5 files changed, 20 insertions(+), 33 deletions(-)
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 60c47b417a4b..9d08d56f2896 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -1039,7 +1039,7 @@ int btrfs_compress_pages(unsigned int type_level, struct address_space *mapping,
struct list_head *workspace;
int ret;
- level = btrfs_compress_op[type]->set_level(level);
+ level = btrfs_compress_set_level(type, level);
workspace = get_workspace(type, level);
ret = btrfs_compress_op[type]->compress_pages(workspace, mapping,
start, pages,
@@ -1611,7 +1611,23 @@ unsigned int btrfs_compress_str2level(unsigned int type, const char *str)
level = 0;
}
- level = btrfs_compress_op[type]->set_level(level);
+ level = btrfs_compress_set_level(type, level);
+
+ return level;
+}
+
+/*
+ * Adjust @level according to the limits of the compression algorithm or
+ * fallback to default
+ */
+unsigned int btrfs_compress_get_level(int type, unsigned level)
+{
+ const struct btrfs_compress_op *ops = btrfs_compress_op[type];
+
+ if (level == 0)
+ level = ops->default_level;
+ else
+ level = min_t(unsigned int, level, ops->max_level);
return level;
}
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 07b2009dc63f..f0fc6304dfae 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -156,13 +156,6 @@ struct btrfs_compress_op {
unsigned long start_byte,
size_t srclen, size_t destlen);
- /*
- * This bounds the level set by the user to be within range of a
- * particular compression type. It returns the level that will be used
- * if the level is out of bounds or the default if 0 is passed in.
- */
- unsigned int (*set_level)(unsigned int level);
-
/* Maximum level supported by the compression algorithm */
int max_level;
int default_level;
@@ -179,6 +172,8 @@ extern const struct btrfs_compress_op btrfs_zstd_compress;
const char* btrfs_compress_type2str(enum btrfs_compression_type type);
bool btrfs_compress_is_valid_type(const char *str, size_t len);
+unsigned int btrfs_compress_set_level(int type, unsigned level);
+
int btrfs_compress_heuristic(struct inode *inode, u64 start, u64 end);
#endif
diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
index adac6cb30d65..acad4174f68d 100644
--- a/fs/btrfs/lzo.c
+++ b/fs/btrfs/lzo.c
@@ -507,11 +507,6 @@ static int lzo_decompress(struct list_head *ws, unsigned char *data_in,
return ret;
}
-static unsigned int lzo_set_level(unsigned int level)
-{
- return 0;
-}
-
const struct btrfs_compress_op btrfs_lzo_compress = {
.init_workspace_manager = lzo_init_workspace_manager,
.cleanup_workspace_manager = lzo_cleanup_workspace_manager,
@@ -522,7 +517,6 @@ const struct btrfs_compress_op btrfs_lzo_compress = {
.compress_pages = lzo_compress_pages,
.decompress_bio = lzo_decompress_bio,
.decompress = lzo_decompress,
- .set_level = lzo_set_level,
.max_level = 1,
.default_level = 1,
};
diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 03d6c3683bd9..df1aace5df50 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -418,14 +418,6 @@ static int zlib_decompress(struct list_head *ws, unsigned char *data_in,
return ret;
}
-static unsigned int zlib_set_level(unsigned int level)
-{
- if (!level)
- return BTRFS_ZLIB_DEFAULT_LEVEL;
-
- return min_t(unsigned int, level, 9);
-}
-
const struct btrfs_compress_op btrfs_zlib_compress = {
.init_workspace_manager = zlib_init_workspace_manager,
.cleanup_workspace_manager = zlib_cleanup_workspace_manager,
@@ -436,7 +428,6 @@ const struct btrfs_compress_op btrfs_zlib_compress = {
.compress_pages = zlib_compress_pages,
.decompress_bio = zlib_decompress_bio,
.decompress = zlib_decompress,
- .set_level = zlib_set_level,
.max_level = 9,
.default_level = BTRFS_ZLIB_DEFAULT_LEVEL,
};
diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index b2b23a6a497d..0af4a5cd4313 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -710,14 +710,6 @@ static int zstd_decompress(struct list_head *ws, unsigned char *data_in,
return ret;
}
-static unsigned int zstd_set_level(unsigned int level)
-{
- if (!level)
- return ZSTD_BTRFS_DEFAULT_LEVEL;
-
- return min_t(unsigned int, level, ZSTD_BTRFS_MAX_LEVEL);
-}
-
const struct btrfs_compress_op btrfs_zstd_compress = {
.init_workspace_manager = zstd_init_workspace_manager,
.cleanup_workspace_manager = zstd_cleanup_workspace_manager,
@@ -728,7 +720,6 @@ const struct btrfs_compress_op btrfs_zstd_compress = {
.compress_pages = zstd_compress_pages,
.decompress_bio = zstd_decompress_bio,
.decompress = zstd_decompress,
- .set_level = zstd_set_level,
.max_level = ZSTD_BTRFS_MAX_LEVEL,
.default_level = ZSTD_BTRFS_DEFAULT_LEVEL,
};
--
2.22.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] btrfs: compression: replace set_level callbacks by a common helper
2019-08-09 14:55 ` [PATCH 2/2] btrfs: compression: replace set_level callbacks by a common helper David Sterba
@ 2019-08-11 13:05 ` kbuild test robot
2019-08-11 13:08 ` kbuild test robot
1 sibling, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2019-08-11 13:05 UTC (permalink / raw)
To: David Sterba; +Cc: kbuild-all, linux-btrfs, David Sterba
[-- Attachment #1: Type: text/plain, Size: 904 bytes --]
Hi David,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc3 next-20190809]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/David-Sterba/Compression-level-API-cleanups/20190811-193645
config: x86_64-randconfig-h003-201932 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-10) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> ERROR: "btrfs_compress_set_level" [fs/btrfs/btrfs.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36610 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] btrfs: compression: replace set_level callbacks by a common helper
2019-08-09 14:55 ` [PATCH 2/2] btrfs: compression: replace set_level callbacks by a common helper David Sterba
2019-08-11 13:05 ` kbuild test robot
@ 2019-08-11 13:08 ` kbuild test robot
1 sibling, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2019-08-11 13:08 UTC (permalink / raw)
To: David Sterba; +Cc: kbuild-all, linux-btrfs, David Sterba
[-- Attachment #1: Type: text/plain, Size: 3027 bytes --]
Hi David,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc3 next-20190809]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/David-Sterba/Compression-level-API-cleanups/20190811-193645
config: x86_64-randconfig-h002-201932 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-10) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
ld: fs/btrfs/compression.o: in function `btrfs_compress_pages':
>> fs/btrfs/compression.c:1042: undefined reference to `btrfs_compress_set_level'
ld: fs/btrfs/compression.o: in function `btrfs_compress_str2level':
fs/btrfs/compression.c:1614: undefined reference to `btrfs_compress_set_level'
vim +1042 fs/btrfs/compression.c
1007
1008 /*
1009 * Given an address space and start and length, compress the bytes into @pages
1010 * that are allocated on demand.
1011 *
1012 * @type_level is encoded algorithm and level, where level 0 means whatever
1013 * default the algorithm chooses and is opaque here;
1014 * - compression algo are 0-3
1015 * - the level are bits 4-7
1016 *
1017 * @out_pages is an in/out parameter, holds maximum number of pages to allocate
1018 * and returns number of actually allocated pages
1019 *
1020 * @total_in is used to return the number of bytes actually read. It
1021 * may be smaller than the input length if we had to exit early because we
1022 * ran out of room in the pages array or because we cross the
1023 * max_out threshold.
1024 *
1025 * @total_out is an in/out parameter, must be set to the input length and will
1026 * be also used to return the total number of compressed bytes
1027 *
1028 * @max_out tells us the max number of bytes that we're allowed to
1029 * stuff into pages
1030 */
1031 int btrfs_compress_pages(unsigned int type_level, struct address_space *mapping,
1032 u64 start, struct page **pages,
1033 unsigned long *out_pages,
1034 unsigned long *total_in,
1035 unsigned long *total_out)
1036 {
1037 int type = btrfs_compress_type(type_level);
1038 int level = btrfs_compress_level(type_level);
1039 struct list_head *workspace;
1040 int ret;
1041
> 1042 level = btrfs_compress_set_level(type, level);
1043 workspace = get_workspace(type, level);
1044 ret = btrfs_compress_op[type]->compress_pages(workspace, mapping,
1045 start, pages,
1046 out_pages,
1047 total_in, total_out);
1048 put_workspace(type, workspace);
1049 return ret;
1050 }
1051
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30830 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] btrfs: define compression levels statically
2019-08-09 14:55 ` [PATCH 1/2] btrfs: define compression levels statically David Sterba
@ 2019-08-12 8:30 ` Nikolay Borisov
2019-08-20 14:35 ` David Sterba
0 siblings, 1 reply; 7+ messages in thread
From: Nikolay Borisov @ 2019-08-12 8:30 UTC (permalink / raw)
To: David Sterba, linux-btrfs
On 9.08.19 г. 17:55 ч., David Sterba wrote:
> The maximum and default levels do not change and can be defined
> directly. The set_level callback was a temporary solution and will be
> removed.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
> fs/btrfs/compression.h | 4 ++++
> fs/btrfs/lzo.c | 2 ++
> fs/btrfs/zlib.c | 2 ++
> fs/btrfs/zstd.c | 2 ++
> 4 files changed, 10 insertions(+)
>
> diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
> index 2035b8eb1290..07b2009dc63f 100644
> --- a/fs/btrfs/compression.h
> +++ b/fs/btrfs/compression.h
> @@ -162,6 +162,10 @@ struct btrfs_compress_op {
> * if the level is out of bounds or the default if 0 is passed in.
> */
> unsigned int (*set_level)(unsigned int level);
> +
> + /* Maximum level supported by the compression algorithm */
> + int max_level;
> + int default_level;
can levels be negative? If not just define those as unsigned ints and in
the next patch it won't be necessary to use min_t but plain min.
> };
>
> /* The heuristic workspaces are managed via the 0th workspace manager */
> diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c
> index 579d53ae256f..adac6cb30d65 100644
> --- a/fs/btrfs/lzo.c
> +++ b/fs/btrfs/lzo.c
> @@ -523,4 +523,6 @@ const struct btrfs_compress_op btrfs_lzo_compress = {
> .decompress_bio = lzo_decompress_bio,
> .decompress = lzo_decompress,
> .set_level = lzo_set_level,
> + .max_level = 1,
> + .default_level = 1,
> };
> diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
> index b86b7ad6b900..03d6c3683bd9 100644
> --- a/fs/btrfs/zlib.c
> +++ b/fs/btrfs/zlib.c
> @@ -437,4 +437,6 @@ const struct btrfs_compress_op btrfs_zlib_compress = {
> .decompress_bio = zlib_decompress_bio,
> .decompress = zlib_decompress,
> .set_level = zlib_set_level,
> + .max_level = 9,
> + .default_level = BTRFS_ZLIB_DEFAULT_LEVEL,
> };
> diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
> index 3837ca180d52..b2b23a6a497d 100644
> --- a/fs/btrfs/zstd.c
> +++ b/fs/btrfs/zstd.c
> @@ -729,4 +729,6 @@ const struct btrfs_compress_op btrfs_zstd_compress = {
> .decompress_bio = zstd_decompress_bio,
> .decompress = zstd_decompress,
> .set_level = zstd_set_level,
> + .max_level = ZSTD_BTRFS_MAX_LEVEL,
> + .default_level = ZSTD_BTRFS_DEFAULT_LEVEL,
> };
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] btrfs: define compression levels statically
2019-08-12 8:30 ` Nikolay Borisov
@ 2019-08-20 14:35 ` David Sterba
0 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2019-08-20 14:35 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: David Sterba, linux-btrfs
On Mon, Aug 12, 2019 at 11:30:42AM +0300, Nikolay Borisov wrote:
>
>
> On 9.08.19 г. 17:55 ч., David Sterba wrote:
> > The maximum and default levels do not change and can be defined
> > directly. The set_level callback was a temporary solution and will be
> > removed.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> > fs/btrfs/compression.h | 4 ++++
> > fs/btrfs/lzo.c | 2 ++
> > fs/btrfs/zlib.c | 2 ++
> > fs/btrfs/zstd.c | 2 ++
> > 4 files changed, 10 insertions(+)
> >
> > diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
> > index 2035b8eb1290..07b2009dc63f 100644
> > --- a/fs/btrfs/compression.h
> > +++ b/fs/btrfs/compression.h
> > @@ -162,6 +162,10 @@ struct btrfs_compress_op {
> > * if the level is out of bounds or the default if 0 is passed in.
> > */
> > unsigned int (*set_level)(unsigned int level);
> > +
> > + /* Maximum level supported by the compression algorithm */
> > + int max_level;
> > + int default_level;
>
> can levels be negative? If not just define those as unsigned ints and in
> the next patch it won't be necessary to use min_t but plain min.
No, levels should be >= 0.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-08-20 14:34 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-09 14:55 [PATCH 0/2] Compression level API cleanups David Sterba
2019-08-09 14:55 ` [PATCH 1/2] btrfs: define compression levels statically David Sterba
2019-08-12 8:30 ` Nikolay Borisov
2019-08-20 14:35 ` David Sterba
2019-08-09 14:55 ` [PATCH 2/2] btrfs: compression: replace set_level callbacks by a common helper David Sterba
2019-08-11 13:05 ` kbuild test robot
2019-08-11 13:08 ` kbuild test robot
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).