From 67794c3814408d497b4c3e12e2fdcc25a51b0595 Mon Sep 17 00:00:00 2001 From: Patrick Farrell Date: Sat, 11 Nov 2023 15:39:22 -0500 Subject: [PATCH] EX-7601 obd: move type switching to alloc_compr callers The code is much cleaner if we can eliminated applied type and handle that issue once per compression or decompression rather than for every chunk. This requires moving the type switching inside alloc_compr. (Also improve some error messages - alloc_compr can fail with ENOMEM as well.) The compression code currently allocates a transform for every chunk on the client. This is relatively cheap, but it also complicates the code by repeatedly checking if a particular compression type is supported (this is the "applied type" code). Moving alloc_compr to compress/decompress request makes the code much simpler. Signed-off-by: Patrick Farrell Change-Id: I162e81577db721a9715d57b3f262fcabbcbf308a Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/53103 Reviewed-by: Artem Blagodarenko Reviewed-by: Andreas Dilger Tested-by: jenkins Tested-by: Andreas Dilger --- lustre/include/lustre_compr.h | 16 ++--- lustre/obdclass/lustre_compr.c | 148 +++++++++++++++++++---------------------- lustre/osc/osc_compress.c | 60 ++++++++++------- 3 files changed, 112 insertions(+), 112 deletions(-) diff --git a/lustre/include/lustre_compr.h b/lustre/include/lustre_compr.h index dc40fd1..962ea74 100644 --- a/lustre/include/lustre_compr.h +++ b/lustre/include/lustre_compr.h @@ -28,14 +28,14 @@ #include -int alloc_compr(enum ll_compr_type *type, unsigned int lvl, - struct crypto_comp **cc); +int alloc_compr(const char *obd_name, enum ll_compr_type *type, + unsigned int lvl, struct crypto_comp **cc, bool decompress); -int compress_chunk(const char *obd_name, const unsigned char *in, - unsigned int in_len, unsigned char *out, - unsigned int *out_len, enum ll_compr_type type, - unsigned int lvl, unsigned int chunk_bits, - enum ll_compr_type *applied_type); +int compress_chunk(const char *obd_name, struct crypto_comp *cc, + const unsigned char *in, unsigned int in_len, + unsigned char *out, unsigned int *out_len, + enum ll_compr_type type, unsigned int lvl, + unsigned int chunk_bits); static inline struct page *mem_to_page(void *addr) { @@ -47,7 +47,7 @@ static inline struct page *mem_to_page(void *addr) int is_chunk_start(struct page *page, struct ll_compr_hdr **ret_header); -int decompress_chunk(const char *obd_name, +int decompress_chunk(const char *obd_name, struct crypto_comp *cc, unsigned char *in, unsigned int in_len, unsigned char *out, unsigned int *out_len, enum ll_compr_type type, unsigned int lvl); diff --git a/lustre/obdclass/lustre_compr.c b/lustre/obdclass/lustre_compr.c index b900c88..cf601b4 100644 --- a/lustre/obdclass/lustre_compr.c +++ b/lustre/obdclass/lustre_compr.c @@ -77,44 +77,76 @@ void load_crypto_module(enum ll_compr_type type) } } -int alloc_compr(enum ll_compr_type *type, unsigned int lvl, - struct crypto_comp **cc) +#define compr_type_fast LL_COMPR_TYPE_LZ4FAST +#define compr_type_best LL_COMPR_TYPE_GZIP + +int alloc_compr(const char *obd_name, enum ll_compr_type *type, + unsigned int lvl, struct crypto_comp **cc, bool decompress) { + int ret = 0; + + ENTRY; + if (OBD_FAIL_CHECK(OBD_FAIL_OSC_WRONG_COMP_ALG)) - return -EIO; + GOTO(out, ret = -EIO); /* * We should never have a chunk that is actually compressed with BEST * or FAST type, so it should never apply during decompression. - * For compression path these types are changed to the appropriate algo - * in the compress_chunk(). */ - LASSERT(*type != LL_COMPR_TYPE_BEST); - LASSERT(*type != LL_COMPR_TYPE_FAST); + LASSERT(ergo(decompress, *type != LL_COMPR_TYPE_BEST && + *type != LL_COMPR_TYPE_FAST)); + + if (*type == LL_COMPR_TYPE_BEST) + *type = compr_type_best; + else if (*type == LL_COMPR_TYPE_FAST) + *type = compr_type_fast; load_crypto_module(*type); +again: *cc = crypto_alloc_comp(crypto_name_from_type(*type, lvl), 0, 0); if (IS_ERR(*cc)) { - int ret = PTR_ERR(*cc); - CERROR("Cannot initialize compressor %i, error %i\n", *type, + ret = PTR_ERR(*cc); + CERROR("Cannot initialize compressor %i, error %i.\n", *type, ret); - /* - * This shouldn't return "ENOENT" to the caller when the crypto - * type is missing. That will be confusing to see "no such file - * or directory" message, so let's change it to "Can not access - * a needed shared library" here. + /* decompression must be performed with the requested type */ + if (decompress) { + CERROR("Decompression failed, rc = %d\n", ret); + GOTO(out, ret); + } + /* LZO should be universally available, so we fall back to LZO, + * and it's not available, we just give up on compression. */ - if (ret == -ENOENT) - ret = -ELIBACC; - *cc = NULL; - return ret; + if (*type == LL_COMPR_TYPE_LZO) { + CWARN("%s: LZO(%i) crypto setup failed, left plain: rc = %d\n", + obd_name, *type, ret); + *type = LL_COMPR_TYPE_NONE; + GOTO(out, ret = 1); + } else if (*type != compr_type_fast) { + CWARN("%s: type %i crypto setup failed, try FAST(%i): rc = %d\n", + obd_name, *type, compr_type_fast, ret); + *type = compr_type_fast; + goto again; + } else { + CWARN("%s: type %i crypto setup failed, try LZO(%i): rc = %d\n", + obd_name, *type, LL_COMPR_TYPE_LZO, ret); + *type = LL_COMPR_TYPE_LZO; + goto again; + } } - if (lvl != -1) - ll_crypto_comp_set_level(*cc, lvl); +out: + if (ret) { + *cc = NULL; + } else /* success */ { + if (lvl != -1) + ll_crypto_comp_set_level(*cc, lvl); + CDEBUG(D_SEC, "%s: crypto_comp allocated, type %i, level %i\n", + obd_name, *type, lvl); + } - return 0; + RETURN(ret); } EXPORT_SYMBOL(alloc_compr); @@ -124,67 +156,31 @@ EXPORT_SYMBOL(alloc_compr); * use the compressed one. */ #define COMP_GAP 4096 -int compress_chunk(const char *obd_name, const unsigned char *in, - unsigned int in_len, unsigned char *out, - unsigned int *out_len, enum ll_compr_type type, - unsigned int lvl, unsigned int chunk_bits, - enum ll_compr_type *applied_type) +int compress_chunk(const char *obd_name, struct crypto_comp *cc, + const unsigned char *in, unsigned int in_len, + unsigned char *out, unsigned int *out_len, + enum ll_compr_type type, unsigned int lvl, + unsigned int chunk_bits) { struct ll_compr_hdr *llch; unsigned int len = *out_len - sizeof(*llch); - struct crypto_comp *cc = NULL; - enum ll_compr_type compr_type_fast = LL_COMPR_TYPE_LZ4FAST; - enum ll_compr_type compr_type_best = LL_COMPR_TYPE_GZIP; int rc; - /* - * Once compress_chunk() faced with unsupported compression - * algorithm, it replaces algorithm used for whole request. - */ - if (*applied_type != LL_COMPR_TYPE_UNCHANGED) - type = *applied_type; - - if (type == LL_COMPR_TYPE_BEST) - type = compr_type_best; - else if (type == LL_COMPR_TYPE_FAST) - type = compr_type_fast; - -again: - rc = alloc_compr(&type, lvl, &cc); - if (!rc) { - CDEBUG(D_SEC, "%s: crypto_comp allocated, type %i, level %i\n", - obd_name, type, lvl); - } else if (type == LL_COMPR_TYPE_LZO) { - CWARN("%s: LZO(%i) unsupported, left plain: rc = %d\n", - obd_name, type, rc); - *out_len = in_len; - *applied_type = LL_COMPR_TYPE_NONE; - return 0; - } else if (type != compr_type_fast) { - CWARN("%s: type %i unsupported, try FAST(%i): rc = %d\n", - obd_name, type, compr_type_fast, rc); - type = compr_type_fast; - goto again; - } else { - CWARN("%s: type %i unsupported, try LZO(%i): rc = %d\n", - obd_name, type, LL_COMPR_TYPE_LZO, rc); - type = LL_COMPR_TYPE_LZO; - goto again; - } - rc = crypto_comp_compress(cc, in, in_len, out + sizeof(*llch), &len); if (rc) { - crypto_free_comp(cc); + *out_len = in_len; + CERROR("%s: Compression failed, type %i, lvl %d, %d\n", + obd_name, type, lvl, rc); return 0; } if (len + sizeof(*llch) + COMP_GAP > in_len) { - CDEBUG(D_SEC, ", compressed %u, plain %u, leaving uncompressed\n", - len, in_len); + CDEBUG(D_SEC, + "Compressed %u + overhead %lu > plain %u, leaving uncompressed\n", + len, sizeof(*llch) + COMP_GAP, in_len); *out_len = in_len; - crypto_free_comp(cc); return 0; } @@ -204,9 +200,6 @@ again: *out_len = len + sizeof(*llch); - if (cc) - crypto_free_comp(cc); - return 1; } EXPORT_SYMBOL(compress_chunk); @@ -231,20 +224,15 @@ int is_chunk_start(struct page *page, struct ll_compr_hdr **ret_header) } EXPORT_SYMBOL(is_chunk_start); -int decompress_chunk(const char *obd_name, +int decompress_chunk(const char *obd_name, struct crypto_comp *cc, unsigned char *in, unsigned int in_len, unsigned char *out, unsigned int *out_len, enum ll_compr_type type, unsigned int lvl) { int rc = 0; - struct crypto_comp *cc = NULL; - rc = alloc_compr(&type, lvl, &cc); - if (rc) { - CERROR("%s: Unsupported compression type %i: rc = %d\n", - obd_name, type, rc); - goto fail; - } + CDEBUG(D_SEC, "in_len %u, out_len %u, type %d, lvl %u\n", in_len, + *out_len, type, lvl); rc = crypto_comp_decompress(cc, in, in_len, out, out_len); if (rc) { @@ -253,8 +241,6 @@ int decompress_chunk(const char *obd_name, } fail: - if (cc) - crypto_free_comp(cc); return rc; } EXPORT_SYMBOL(decompress_chunk); diff --git a/lustre/osc/osc_compress.c b/lustre/osc/osc_compress.c index 462e93a..ea6bda2 100644 --- a/lustre/osc/osc_compress.c +++ b/lustre/osc/osc_compress.c @@ -111,13 +111,13 @@ int fill_cpga(struct brw_page **cpga, struct brw_page **pga, return 0; } - +/* returns 0 on success, non-zero on failure to compress */ int compress_request(const char *obd_name, struct obdo *oa, struct brw_page **pga, struct brw_page ***cpga, u32 *page_count, __u64 kms) { struct cl_page *clpage; - enum ll_compr_type applied_type = LL_COMPR_TYPE_UNCHANGED; + struct crypto_comp *cc; enum ll_compr_type type; bool compressed = false; unsigned int src_size; @@ -146,9 +146,17 @@ int compress_request(const char *obd_name, struct obdo *oa, chunk_size = (1 << chunk_bits); pages_per_chunk = chunk_size / PAGE_SIZE; - OBD_ALLOC(*cpga, *page_count * sizeof(**cpga)); src_buf_bits = chunk_bits; dest_buf_bits = chunk_bits + 1; + + rc = alloc_compr(obd_name, &type, lvl, &cc, false); + /* if we're unable to setup compression, alloc_compr prints a warning + * but we do not fail the IO - just write data uncompressed + */ + if (rc) + GOTO(out, rc); + + OBD_ALLOC(*cpga, *page_count * sizeof(**cpga)); sptlrpc_pool_get_pages(&src, src_buf_bits); if (*cpga == NULL || src == NULL) @@ -170,23 +178,21 @@ int compress_request(const char *obd_name, struct obdo *oa, merge_chunk(pga, NULL, chunk_start, pga_i + 1 - chunk_start, src, &src_size); - /* - * - applied_type == 0 if no supported algorithms - * found during the previous compress_chunk call - */ - if (applied_type) { - compressed = compress_chunk(obd_name, src, - src_size, dst, &dst_size, - type, lvl, chunk_bits, - &applied_type); - CDEBUG(D_SEC, - "%s: rc %d: inode "DFID"\n", - obd_name, rc, oa->o_parent_seq, - oa->o_parent_oid, oa->o_parent_ver); - } else { - compressed = false; + compressed = compress_chunk(obd_name, cc, src, + src_size, dst, + &dst_size, type, + lvl, chunk_bits); + if (!compressed) dst_size = src_size; - } + + CDEBUG(D_SEC, + "%s: rc %d: inode "DFID"\n", + obd_name, rc, oa->o_parent_seq, + oa->o_parent_oid, oa->o_parent_ver); + + CDEBUG(D_SEC, + "Compressed %u, plain %u\n", + dst_size, src_size); rc = fill_cpga(*cpga, pga, compressed ? dst : NULL, chunk_start, cpga_i, dst_size); @@ -212,17 +218,18 @@ int compress_request(const char *obd_name, struct obdo *oa, } } + *page_count = cpga_i; + CDEBUG(D_SEC, "Compressed content: %i pages (%i chunks)\n", cpga_i, chunk_count); out: + if (cc) + crypto_free_comp(cc); if (src != NULL) sptlrpc_pool_put_pages(&src, src_buf_bits); - if (rc != 0 && *cpga != NULL) free_cpga(*cpga, *page_count); - if (rc == 0) - *page_count = cpga_i; RETURN(rc); } @@ -234,6 +241,7 @@ int decompress_request(struct osc_brw_async_args *aa, int page_count) char *obd_name = aa->aa_cli->cl_import->imp_obd->obd_name; enum ll_compr_type type; struct cl_page *clpage; + struct crypto_comp *cc; unsigned int src_size; unsigned int dst_size; int pages_in_chunk; @@ -260,6 +268,9 @@ int decompress_request(struct osc_brw_async_args *aa, int page_count) buf_bits = chunk_bits + 1; pages_in_chunk = chunk_size / PAGE_SIZE; + rc = alloc_compr(obd_name, &type, lvl, &cc, true); + if (rc) + GOTO(out, rc); /* the RPC may not start on a chunk boundary, find the first chunk * boundary, then start iterating by chunk_size at that point (in the * next loop) @@ -323,7 +334,8 @@ int decompress_request(struct osc_brw_async_args *aa, int page_count) CDEBUG(D_SEC, "Compressed size %lu, type %i\n", llch->llch_compr_size + sizeof(*llch), type); - rc = decompress_chunk(obd_name, src + llch->llch_header_size, + rc = decompress_chunk(obd_name, cc, + src + llch->llch_header_size, llch->llch_compr_size, dst, &dst_size, type, lvl); if (rc) @@ -339,6 +351,8 @@ int decompress_request(struct osc_brw_async_args *aa, int page_count) CDEBUG(D_SEC, "RPC is %d pages, decompressed %d chunks\n", page_count, count); out: + if (cc) + crypto_free_comp(cc); if (src != NULL) sptlrpc_pool_put_pages(&src, buf_bits); -- 1.8.3.1