diff mbox

[v2] ceph: add retry logic for error -ERANGE in ceph_get_acl()

Message ID 20180620074255.29124-1-cgxu519@gmx.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chengguang Xu June 20, 2018, 7:42 a.m. UTC
When the size of acl extended attribution is larger than pre-allocated
value buffer size, we will hit error '-ERANGE' and it's probabaly caused
by concurrent get/set acl from different clients. In this case, current
logic just sets acl to NULL so that we cannot get proper information but
the operation looks successful.

This patch adds retry logic for error -ERANGE and return -EIO if fail
from the retry. Additionally, print real errno when failing from
__ceph_getxattr().

Signed-off-by: Chengguang Xu <cgxu519@gmx.com>
---
v2:
- Increase retry count to 10.
- Print real errno when failing from __ceph_getxattr().

 fs/ceph/acl.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Comments

Yan, Zheng June 20, 2018, 12:13 p.m. UTC | #1
On Wed, Jun 20, 2018 at 4:51 PM Chengguang Xu <cgxu519@gmx.com> wrote:
>
> When the size of acl extended attribution is larger than pre-allocated
> value buffer size, we will hit error '-ERANGE' and it's probabaly caused
> by concurrent get/set acl from different clients. In this case, current
> logic just sets acl to NULL so that we cannot get proper information but
> the operation looks successful.
>
> This patch adds retry logic for error -ERANGE and return -EIO if fail
> from the retry. Additionally, print real errno when failing from
> __ceph_getxattr().
>
> Signed-off-by: Chengguang Xu <cgxu519@gmx.com>
> ---
> v2:
> - Increase retry count to 10.
> - Print real errno when failing from __ceph_getxattr().
>

Applied, thanks

Yan, Zheng

>  fs/ceph/acl.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
> index 59cb307b15fb..28290ac409d2 100644
> --- a/fs/ceph/acl.c
> +++ b/fs/ceph/acl.c
> @@ -45,6 +45,7 @@ static inline void ceph_set_cached_acl(struct inode *inode,
>  struct posix_acl *ceph_get_acl(struct inode *inode, int type)
>  {
>         int size;
> +       unsigned int retry_cnt = 0;
>         const char *name;
>         char *value = NULL;
>         struct posix_acl *acl;
> @@ -60,6 +61,7 @@ struct posix_acl *ceph_get_acl(struct inode *inode, int type)
>                 BUG();
>         }
>
> +retry:
>         size = __ceph_getxattr(inode, name, "", 0);
>         if (size > 0) {
>                 value = kzalloc(size, GFP_NOFS);
> @@ -68,12 +70,21 @@ struct posix_acl *ceph_get_acl(struct inode *inode, int type)
>                 size = __ceph_getxattr(inode, name, value, size);
>         }
>
> -       if (size > 0)
> +       if (size == -ERANGE && retry_cnt < 10) {
> +               retry_cnt++;
> +               kfree(value);
> +               value = NULL;
> +               goto retry;
> +       }
> +
> +       if (size > 0) {
>                 acl = posix_acl_from_xattr(&init_user_ns, value, size);
> -       else if (size == -ERANGE || size == -ENODATA || size == 0)
> +       } else if (size == -ENODATA || size == 0) {
>                 acl = NULL;
> -       else
> +       } else {
> +               pr_err_ratelimited("get acl failed, errno = %d\n", size);
>                 acl = ERR_PTR(-EIO);
> +       }
>
>         kfree(value);
>
> --
> 2.17.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
index 59cb307b15fb..28290ac409d2 100644
--- a/fs/ceph/acl.c
+++ b/fs/ceph/acl.c
@@ -45,6 +45,7 @@  static inline void ceph_set_cached_acl(struct inode *inode,
 struct posix_acl *ceph_get_acl(struct inode *inode, int type)
 {
 	int size;
+	unsigned int retry_cnt = 0;
 	const char *name;
 	char *value = NULL;
 	struct posix_acl *acl;
@@ -60,6 +61,7 @@  struct posix_acl *ceph_get_acl(struct inode *inode, int type)
 		BUG();
 	}
 
+retry:
 	size = __ceph_getxattr(inode, name, "", 0);
 	if (size > 0) {
 		value = kzalloc(size, GFP_NOFS);
@@ -68,12 +70,21 @@  struct posix_acl *ceph_get_acl(struct inode *inode, int type)
 		size = __ceph_getxattr(inode, name, value, size);
 	}
 
-	if (size > 0)
+	if (size == -ERANGE && retry_cnt < 10) {
+		retry_cnt++;
+		kfree(value);
+		value = NULL;
+		goto retry;
+	}
+
+	if (size > 0) {
 		acl = posix_acl_from_xattr(&init_user_ns, value, size);
-	else if (size == -ERANGE || size == -ENODATA || size == 0)
+	} else if (size == -ENODATA || size == 0) {
 		acl = NULL;
-	else
+	} else {
+		pr_err_ratelimited("get acl failed, errno = %d\n", size);
 		acl = ERR_PTR(-EIO);
+	}
 
 	kfree(value);