Re: [PATCH] ceph: wake waiters on any IMPORT that grants new caps

["Yan, Zheng" <ukernel@gmail.com>]

On Sat, Jan 29, 2022 at 2:32 AM Jeff Layton <jlayton@kernel.org> wrote:
>
> I've noticed an intermittent hang waiting for caps in some testing. What
> I see is that the client will try to get caps for an operation (e.g. a
> read), and ends up waiting on the waitqueue forever. The caps debugfs
> file however shows that the caps it's waiting on have already been
> granted.
>
> The current grant handling code will wake the waitqueue when it sees
> that there are newly-granted caps in the issued set. On an import
> however, we'll end up adding a new cap first, which fools the logic into
> thinking that nothing has changed. A later hack in the code works around
> this, but only for auth caps.

not right. handle_cap_import() saves old issued to extra_info->issued.

>
> Ensure we wake the waiters whenever we get an IMPORT that grants new
> caps for the inode.
>
> URL: https://tracker.ceph.com/issues/54044
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/caps.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
>
> I'm still testing this patch, but I think this may be the cause of some
> mysterious hangs I've hit in testing.
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index e668cdb9c99e..06b65a68e920 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -3541,21 +3541,22 @@ static void handle_cap_grant(struct inode *inode,
>                         fill_inline = true;
>         }
>
> -       if (ci->i_auth_cap == cap &&
> -           le32_to_cpu(grant->op) == CEPH_CAP_OP_IMPORT) {
> +       if (le32_to_cpu(grant->op) == CEPH_CAP_OP_IMPORT) {
>                 if (newcaps & ~extra_info->issued)
>                         wake = true;
>
> -               if (ci->i_requested_max_size > max_size ||
> -                   !(le32_to_cpu(grant->wanted) & CEPH_CAP_ANY_FILE_WR)) {
> -                       /* re-request max_size if necessary */
> -                       ci->i_requested_max_size = 0;
> -                       wake = true;
> -               }
> +               if (ci->i_auth_cap == cap) {
> +                       if (ci->i_requested_max_size > max_size ||
> +                           !(le32_to_cpu(grant->wanted) & CEPH_CAP_ANY_FILE_WR)) {
> +                               /* re-request max_size if necessary */
> +                               ci->i_requested_max_size = 0;
> +                               wake = true;
> +                       }
>
> -               ceph_kick_flushing_inode_caps(session, ci);
> -               spin_unlock(&ci->i_ceph_lock);
> -               up_read(&session->s_mdsc->snap_rwsem);
> +                       ceph_kick_flushing_inode_caps(session, ci);
> +                       spin_unlock(&ci->i_ceph_lock);
> +                       up_read(&session->s_mdsc->snap_rwsem);
> +               }
>         } else {
>                 spin_unlock(&ci->i_ceph_lock);
>         }
> --
> 2.34.1
>