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 <fan.yong@intel.com>
Change-Id: Iaac2e414b5f1fd197303bb7ec7d5e2763b6f3e9a
Reviewed-on: https://review.whamcloud.com/31514
Reviewed-by: Alex Zhuravlev <alexey.zhuravlev@intel.com>
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: John L. Hammond <john.hammond@intel.com>

diff --git a/lustre/osd-zfs/osd_internal.h b/lustre/osd-zfs/osd_internal.h
index 4e19b20..f4a8146 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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)