From 2f7bc31bf6e65f2898f345f96fc648250b59634e Mon Sep 17 00:00:00 2001 From: Fan Yong Date: Wed, 20 Dec 2017 21:57:09 +0800 Subject: [PATCH] LU-10422 lfsck: misc fixes to avoid unexpected repairing There are several issues that will misguide LFSCK to trigger unexpected RPC or repairing by wrong, including: 1) object_update_result_insert() should pack the OUT RPC result (not the return value) into the reply buffer via object_update_result::our_data. But it did that in some wrong address. 2) out_xattr_get() used wrong index to obtain the EA buffer as to may overwrite former update (such as OUT_XATTR_GET) results. 3) osp_declare_xattr_get() does not consider the last '0' of the EA name for the length parameter for osp_insert_async_request(). 4) osp_xattr_get_interpterer() missed to handle the positive value for the given parameter @rc. That will cause the PFID EA to be double read when the target OST-object has it. Signed-off-by: Fan Yong Change-Id: Ibf0e095ae2735c60b9b88e4b0992389c906728f9 Reviewed-on: https://review.whamcloud.com/30612 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: Lai Siyao Reviewed-by: Oleg Drokin --- lustre/include/lustre_update.h | 10 ++-------- lustre/osp/osp_object.c | 42 ++++++++++++++++++------------------------ lustre/target/out_handler.c | 7 +++++-- 3 files changed, 25 insertions(+), 34 deletions(-) diff --git a/lustre/include/lustre_update.h b/lustre/include/lustre_update.h index d1ceb12..f8766f6 100644 --- a/lustre/include/lustre_update.h +++ b/lustre/include/lustre_update.h @@ -241,20 +241,14 @@ object_update_result_insert(struct object_update_reply *reply, int rc) { struct object_update_result *update_result; - char *ptr; update_result = object_update_result_get(reply, index, NULL); LASSERT(update_result); update_result->our_rc = ptlrpc_status_hton(rc); if (rc >= 0) { - if (data_len > 0) { - LASSERT(data); - - ptr = (char *)update_result + - cfs_size_round(sizeof(struct object_update_reply)); - memcpy(ptr, data, data_len); - } + if (data_len > 0 && data) + memcpy(update_result->our_data, data, data_len); update_result->our_datalen = data_len; } diff --git a/lustre/osp/osp_object.c b/lustre/osp/osp_object.c index ae35a71..0604892 100644 --- a/lustre/osp/osp_object.c +++ b/lustre/osp/osp_object.c @@ -769,42 +769,36 @@ static int osp_xattr_get_interpterer(const struct lu_env *env, void *data, int index, int rc) { struct osp_xattr_entry *oxe = data; - struct lu_buf *rbuf = &osp_env_info(env)->osi_lb2; - if (!rc) { + spin_lock(&obj->opo_lock); + if (rc >= 0) { + struct lu_buf *rbuf = &osp_env_info(env)->osi_lb2; size_t len = sizeof(*oxe) + oxe->oxe_namelen + 1; rc = object_update_result_data_get(reply, rbuf, index); - spin_lock(&obj->opo_lock); - if (rc < 0 || rbuf->lb_len == 0 || - rbuf->lb_len > (oxe->oxe_buflen - len)) { - if (unlikely(rc == -ENODATA)) { - oxe->oxe_exist = 0; - oxe->oxe_ready = 1; - } else { - oxe->oxe_ready = 0; - } - spin_unlock(&obj->opo_lock); - /* Put the reference obtained in the - * osp_declare_xattr_get(). */ - osp_oac_xattr_put(oxe); + if (rc == -ENOENT || rc == -ENODATA || rc == 0) { + oxe->oxe_exist = 0; + oxe->oxe_ready = 1; + goto unlock; + } - return rc < 0 ? rc : -ERANGE; + if (unlikely(rc < 0) || + rbuf->lb_len > (oxe->oxe_buflen - len)) { + oxe->oxe_ready = 0; + goto unlock; } __osp_oac_xattr_assignment(obj, oxe, rbuf); - spin_unlock(&obj->opo_lock); } else if (rc == -ENOENT || rc == -ENODATA) { - spin_lock(&obj->opo_lock); oxe->oxe_exist = 0; oxe->oxe_ready = 1; - spin_unlock(&obj->opo_lock); } else { - spin_lock(&obj->opo_lock); oxe->oxe_ready = 0; - spin_unlock(&obj->opo_lock); } +unlock: + spin_unlock(&obj->opo_lock); + /* Put the reference obtained in the osp_declare_xattr_get(). */ osp_oac_xattr_put(oxe); @@ -835,8 +829,8 @@ static int osp_declare_xattr_get(const struct lu_env *env, struct dt_object *dt, struct osp_object *obj = dt2osp_obj(dt); struct osp_device *osp = lu2osp_dev(dt->do_lu.lo_dev); struct osp_xattr_entry *oxe; - __u16 namelen; int rc = 0; + __u16 len; LASSERT(buf != NULL); LASSERT(name != NULL); @@ -848,10 +842,10 @@ static int osp_declare_xattr_get(const struct lu_env *env, struct dt_object *dt, if (oxe == NULL) return -ENOMEM; - namelen = strlen(name); + len = strlen(name) + 1; mutex_lock(&osp->opd_async_requests_mutex); rc = osp_insert_async_request(env, OUT_XATTR_GET, obj, 1, - &namelen, (const void **)&name, + &len, (const void **)&name, oxe, buf->lb_len, osp_xattr_get_interpterer); if (rc != 0) { diff --git a/lustre/target/out_handler.c b/lustre/target/out_handler.c index 481877b..91f0012 100644 --- a/lustre/target/out_handler.c +++ b/lustre/target/out_handler.c @@ -266,7 +266,7 @@ static int out_xattr_get(struct tgt_session_info *tsi) RETURN(PTR_ERR(name)); } - update_result = object_update_result_get(reply, 0, NULL); + update_result = object_update_result_get(reply, idx, NULL); if (update_result == NULL) { CERROR("%s: empty name for xattr get: rc = %d\n", tgt_name(tsi->tsi_tgt), -EPROTO); @@ -294,7 +294,10 @@ static int out_xattr_get(struct tgt_session_info *tsi) tgt_name(tsi->tsi_tgt), PFID(lu_object_fid(&obj->do_lu)), name, (int)lbuf->lb_len, rc); - object_update_result_insert(reply, lbuf->lb_buf, lbuf->lb_len, idx, rc); + /* Since we directly use update_result->our_data as the lbuf->lb_buf, + * then use NULL for result_insert to avoid unnecessary memory copy. */ + object_update_result_insert(reply, NULL, lbuf->lb_len, idx, rc); + RETURN(0); } -- 1.8.3.1