[1/8] ceph: fix error handling in ceph_get_caps()
diff mbox series

Message ID 20190523081345.20410-1-zyan@redhat.com
State New
Headers show
Series
  • [1/8] ceph: fix error handling in ceph_get_caps()
Related show

Commit Message

Yan, Zheng May 23, 2019, 8:13 a.m. UTC
The function return 0 even when interrupted or try_get_cap_refs()
return error.

Introduce by commit 1199d7da2d "ceph: simplify arguments and return
semantics of try_get_cap_refs"

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
---
 fs/ceph/caps.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Comments

Jeff Layton May 23, 2019, 7:54 p.m. UTC | #1
On Thu, 2019-05-23 at 16:13 +0800, Yan, Zheng wrote:
> The function return 0 even when interrupted or try_get_cap_refs()
> return error.
> 
> Introduce by commit 1199d7da2d "ceph: simplify arguments and return
> semantics of try_get_cap_refs"
> 

Should change that last paragraph to:

Fixes: 1199d7da2d ("ceph: simplify arguments and return semantics of try_get_cap_refs")

> Signed-off-by: "Yan, Zheng" <zyan@redhat.com>
> ---
>  fs/ceph/caps.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 72f8e1311392..079d0df9650c 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -2738,15 +2738,13 @@ int ceph_get_caps(struct ceph_inode_info *ci, int need, int want,
>  		_got = 0;
>  		ret = try_get_cap_refs(ci, need, want, endoff,
>  				       false, &_got);
> -		if (ret == -EAGAIN) {
> +		if (ret == -EAGAIN)
>  			continue;
> -		} else if (!ret) {
> -			int err;
> -
> +		if (!ret) {
>  			DEFINE_WAIT_FUNC(wait, woken_wake_function);
>  			add_wait_queue(&ci->i_cap_wq, &wait);
>  
> -			while (!(err = try_get_cap_refs(ci, need, want, endoff,
> +			while (!(ret = try_get_cap_refs(ci, need, want, endoff,
>  							true, &_got))) {
>  				if (signal_pending(current)) {
>  					ret = -ERESTARTSYS;
> @@ -2756,14 +2754,16 @@ int ceph_get_caps(struct ceph_inode_info *ci, int need, int want,
>  			}
>  
>  			remove_wait_queue(&ci->i_cap_wq, &wait);
> -			if (err == -EAGAIN)
> +			if (ret == -EAGAIN)
>  				continue;
>  		}
> -		if (ret == -ESTALE) {
> -			/* session was killed, try renew caps */
> -			ret = ceph_renew_caps(&ci->vfs_inode);
> -			if (ret == 0)
> -				continue;
> +		if (ret < 0) {
> +			if (ret == -ESTALE) {
> +				/* session was killed, try renew caps */
> +				ret = ceph_renew_caps(&ci->vfs_inode);
> +				if (ret == 0)
> +					continue;
> +			}
>  			return ret;
>  		}
>  

Nice catch. The error handling in this function is really nasty. I wish
we could simplify it further. Anyway:

Reviewed-by: Jeff Layton <jlayton@redhat.com>

Patch
diff mbox series

diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 72f8e1311392..079d0df9650c 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -2738,15 +2738,13 @@  int ceph_get_caps(struct ceph_inode_info *ci, int need, int want,
 		_got = 0;
 		ret = try_get_cap_refs(ci, need, want, endoff,
 				       false, &_got);
-		if (ret == -EAGAIN) {
+		if (ret == -EAGAIN)
 			continue;
-		} else if (!ret) {
-			int err;
-
+		if (!ret) {
 			DEFINE_WAIT_FUNC(wait, woken_wake_function);
 			add_wait_queue(&ci->i_cap_wq, &wait);
 
-			while (!(err = try_get_cap_refs(ci, need, want, endoff,
+			while (!(ret = try_get_cap_refs(ci, need, want, endoff,
 							true, &_got))) {
 				if (signal_pending(current)) {
 					ret = -ERESTARTSYS;
@@ -2756,14 +2754,16 @@  int ceph_get_caps(struct ceph_inode_info *ci, int need, int want,
 			}
 
 			remove_wait_queue(&ci->i_cap_wq, &wait);
-			if (err == -EAGAIN)
+			if (ret == -EAGAIN)
 				continue;
 		}
-		if (ret == -ESTALE) {
-			/* session was killed, try renew caps */
-			ret = ceph_renew_caps(&ci->vfs_inode);
-			if (ret == 0)
-				continue;
+		if (ret < 0) {
+			if (ret == -ESTALE) {
+				/* session was killed, try renew caps */
+				ret = ceph_renew_caps(&ci->vfs_inode);
+				if (ret == 0)
+					continue;
+			}
 			return ret;
 		}