From dda46f422e6beb2e2fc504fe323261455a652c5e Mon Sep 17 00:00:00 2001 From: Patrick Farrell Date: Tue, 21 Nov 2023 12:34:31 -0500 Subject: [PATCH] EX-7601 osc: remove &pga usage in compress_request The usage of 'pga' and '&pga' in compress_request is confusing, but also, compress_request modifies &pga by allocating a new compressed page array. Except if we fail in compress_request, we free that new page array. This means failing in compress_request replaces 'pga' with a pointer to freed memory. Instead, create an explicit cpga pointer in the caller and use that. This allows compress_request to fail safely. Signed-off-by: Patrick Farrell Change-Id: Idaf592103c57b0e9ce76ab520a69b819d4f37be9 Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/53120 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger --- lustre/osc/osc_compress.c | 2 +- lustre/osc/osc_compress.h | 2 +- lustre/osc/osc_request.c | 43 ++++++++++++++++++++++++++++++++++++++----- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/lustre/osc/osc_compress.c b/lustre/osc/osc_compress.c index 1de528e..462e93a 100644 --- a/lustre/osc/osc_compress.c +++ b/lustre/osc/osc_compress.c @@ -114,7 +114,7 @@ int fill_cpga(struct brw_page **cpga, struct brw_page **pga, int compress_request(const char *obd_name, struct obdo *oa, struct brw_page **pga, struct brw_page ***cpga, - u32 *page_count) + u32 *page_count, __u64 kms) { struct cl_page *clpage; enum ll_compr_type applied_type = LL_COMPR_TYPE_UNCHANGED; diff --git a/lustre/osc/osc_compress.h b/lustre/osc/osc_compress.h index 542f42c..c1d2feb 100644 --- a/lustre/osc/osc_compress.h +++ b/lustre/osc/osc_compress.h @@ -29,7 +29,7 @@ int compress_request(const char *obd_name, struct obdo *oa, struct brw_page **pga, struct brw_page ***cpga, - u32 *page_count); + u32 *page_count, __u64 kms); int decompress_request(struct osc_brw_async_args *aa, int page_count); diff --git a/lustre/osc/osc_request.c b/lustre/osc/osc_request.c index 38e7ac1..624aa90 100644 --- a/lustre/osc/osc_request.c +++ b/lustre/osc/osc_request.c @@ -1526,6 +1526,8 @@ osc_brw_prep_request(int cmd, struct client_obd *cli, struct obdo *oa, bool page_access_allowed = true; bool enable_checksum = true; bool compressed = false; + int chunk_size; + struct osc_async_page *oap = brw_page2oap(pga[0]); bool directio = false; int short_io_size = 0; void *short_io_buf; @@ -1606,7 +1608,37 @@ osc_brw_prep_request(int cmd, struct client_obd *cli, struct obdo *oa, RETURN(-EINVAL); if (opc == OST_WRITE && compressed) { - rc = compress_request(obd_name, oa, pga, &pga, &page_count); + struct brw_page **cpga = NULL; + __u64 kms = -1; + + clpage = oap2cl_page(oap); + + /* if a write is beyond the current known minimum size, we + * know there is no data in the chunk, so no possibility of + * read-modify write. this means we can compress that chunk + * of data even it is unaligned (this is critical because it + * lets the client compress the last chunk of a file) + * + * Note: direct IO (CPT_TRANSIENT) is usually lockless, so the + * client does not get a current KMS, so we can't do this for + * direct I/O + */ + if (clpage->cp_type != CPT_TRANSIENT) { + struct brw_page *brwpg = pga[page_count - 1]; + struct osc_async_page *last = brw_page2oap(brwpg); + struct cl_object *clobj = osc2cl(last->oap_obj); + struct lov_oinfo *loi = cl2osc(clobj)->oo_oinfo; + + cl_object_attr_lock(clobj); + if (loi->loi_kms_valid) + kms = loi->loi_kms; + cl_object_attr_unlock(clobj); + } + + chunk_size = (1 << cl_page_compr_bits(clpage)); + + rc = compress_request(obd_name, oa, pga, &cpga, &page_count, + kms); if (rc) { /* * TDB: disable for a file if e.g. 512KB+ of @@ -1617,7 +1649,8 @@ osc_brw_prep_request(int cmd, struct client_obd *cli, struct obdo *oa, compressed = 0; } else /* success */ { *pcount = page_count; - *orig_pga = pga; + *orig_pga = cpga; + pga = cpga; } } else if (opc == OST_READ && compressed) { for (i = 0; i < page_count; i++) { @@ -1726,13 +1759,13 @@ retry_encrypt: if (pa) OBD_FREE_PTR_ARRAY_LARGE(pa, page_count); } else if (opc == OST_WRITE && inode && IS_ENCRYPTED(inode)) { - struct osc_async_page *oap = brw_page2oap(pga[0]); - struct cl_page *clpage = oap2cl_page(oap); - struct cl_object *clobj = clpage->cp_obj; struct cl_attr attr = { 0 }; + struct cl_object *clobj; struct lu_env *env; __u16 refcheck; + clpage = oap2cl_page(oap); + clobj = clpage->cp_obj; env = cl_env_get(&refcheck); if (IS_ERR(env)) { rc = PTR_ERR(env); -- 1.8.3.1