From 0e51abb8a512213b58579e66a9d74133001877ff Mon Sep 17 00:00:00 2001 From: Fan Yong Date: Mon, 5 Mar 2018 23:39:29 +0800 Subject: [PATCH] LU-10769 osd-zfs: fix deadlock on osd_object::oo_guard There is race condition inside osd-zfs, it may cause deadlock. Consider the following scenarios: 1) The Thread1 calls osd_attr_set() to set flags on the object. The osd_attr_set() will call the osd_xattr_get() with holding the read mode semaphore on the object::oo_guard. 2) The Thread2 calls the osd_declare_destroy() to destroy such object, it will down_write() on the object::oo_gurad, but be blocked by the Thread1's granted read mode semaphore. 3) The osd_xattr_get() triggered by the osd_xattr_set() will also down_read() on the object::oo_guard. But it will be blocked by the Thread2's pending down_write() request. Then the Thread1 and the Thread2 deadlock. This patch makes the osd_attr_set() to call the lockless version xattr_get osd_xattr_get_internal() to avoid such deadlock. Master-change: https://review.whamcloud.com/31511 Signed-off-by: Fan Yong Change-Id: Iaac2e414b5f1fd197303bb7ec7d5e2763b6f3e9a Reviewed-on: https://review.whamcloud.com/31514 Reviewed-by: Alex Zhuravlev Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: Tony Hutter Reviewed-by: John L. Hammond --- lustre/osd-zfs/osd_internal.h | 2 ++ lustre/osd-zfs/osd_object.c | 18 ++++++++++++------ lustre/osd-zfs/osd_xattr.c | 6 +++--- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/lustre/osd-zfs/osd_internal.h b/lustre/osd-zfs/osd_internal.h index 4e19b20..f4a8146 100644 --- a/lustre/osd-zfs/osd_internal.h +++ b/lustre/osd-zfs/osd_internal.h @@ -566,6 +566,8 @@ int __osd_xattr_load(struct osd_device *osd, sa_handle_t *hdl, int __osd_xattr_get_large(const struct lu_env *env, struct osd_device *osd, uint64_t xattr, struct lu_buf *buf, const char *name, int *sizep); +int osd_xattr_get_internal(const struct lu_env *env, struct osd_object *obj, + struct lu_buf *buf, const char *name, int *sizep); int osd_xattr_get(const struct lu_env *env, struct dt_object *dt, struct lu_buf *buf, const char *name); int osd_declare_xattr_set(const struct lu_env *env, struct dt_object *dt, diff --git a/lustre/osd-zfs/osd_object.c b/lustre/osd-zfs/osd_object.c index 0a94a91..73fb376 100644 --- a/lustre/osd-zfs/osd_object.c +++ b/lustre/osd-zfs/osd_object.c @@ -187,8 +187,8 @@ osd_object_sa_bulk_update(struct osd_object *obj, sa_bulk_attr_t *attrs, /* * Retrieve the attributes of a DMU object */ -int __osd_object_attr_get(const struct lu_env *env, struct osd_device *o, - struct osd_object *obj, struct lu_attr *la) +static int __osd_object_attr_get(const struct lu_env *env, struct osd_device *o, + struct osd_object *obj, struct lu_attr *la) { struct osa_attr *osa = &osd_oti_get(env)->oti_osa; sa_bulk_attr_t *bulk = osd_oti_get(env)->oti_attr_bulk; @@ -330,7 +330,7 @@ struct lu_object *osd_object_alloc(const struct lu_env *env, /* * Concurrency: shouldn't matter. */ -int osd_object_init0(const struct lu_env *env, struct osd_object *obj) +static int osd_object_init0(const struct lu_env *env, struct osd_object *obj) { struct osd_device *osd = osd_obj2dev(obj); const struct lu_fid *fid = lu_object_fid(&obj->oo_dt.do_lu); @@ -1057,15 +1057,21 @@ static int osd_attr_set(const struct lu_env *env, struct dt_object *dt, if (valid & LA_FLAGS) { struct lustre_mdt_attrs *lma; struct lu_buf buf; + int size = 0; if (la->la_flags & LUSTRE_LMA_FL_MASKS) { CLASSERT(sizeof(info->oti_buf) >= sizeof(*lma)); lma = (struct lustre_mdt_attrs *)&info->oti_buf; buf.lb_buf = lma; buf.lb_len = sizeof(info->oti_buf); - rc = osd_xattr_get(env, &obj->oo_dt, &buf, - XATTR_NAME_LMA); - if (rc > 0) { + + /* Please NOT call osd_xattr_get() directly, that + * will cause recursively down_read() on oo_gurad. */ + rc = osd_xattr_get_internal(env, obj, &buf, + XATTR_NAME_LMA, &size); + if (!rc && unlikely(size < sizeof(*lma))) + rc = -EINVAL; + if (!rc) { lma->lma_incompat = le32_to_cpu(lma->lma_incompat); lma->lma_incompat |= diff --git a/lustre/osd-zfs/osd_xattr.c b/lustre/osd-zfs/osd_xattr.c index 949c9bc..995a0c1 100644 --- a/lustre/osd-zfs/osd_xattr.c +++ b/lustre/osd-zfs/osd_xattr.c @@ -206,8 +206,8 @@ out_rele: * \retval 0 on success * \retval negative negated errno on failure */ -int __osd_xattr_get(const struct lu_env *env, struct osd_object *obj, - struct lu_buf *buf, const char *name, int *sizep) +int osd_xattr_get_internal(const struct lu_env *env, struct osd_object *obj, + struct lu_buf *buf, const char *name, int *sizep) { int rc; @@ -239,7 +239,7 @@ int osd_xattr_get(const struct lu_env *env, struct dt_object *dt, RETURN(-EOPNOTSUPP); down_read(&obj->oo_guard); - rc = __osd_xattr_get(env, obj, buf, name, &size); + rc = osd_xattr_get_internal(env, obj, buf, name, &size); up_read(&obj->oo_guard); if (rc == -ENOENT) -- 1.8.3.1