| Message ID | 20190523081345.20410-1-zyan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/8] ceph: fix error handling in ceph_get_caps() | expand |
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>
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; }
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(-)