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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
