On Mon, 2022-06-06 at 13:17 +0800, Xiubo Li wrote: > On 6/4/22 4:39 AM, Jeff Layton wrote: > > When handle_cap_grant is called on an IMPORT op, then the snap_rwsem is > > held and the function is expected to release it before returning. It > > currently fails to do that in all cases which could lead to a deadlock. > > > > URL: https://tracker.ceph.com/issues/55857 > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > --- > > fs/ceph/caps.c | 27 +++++++++++++-------------- > > 1 file changed, 13 insertions(+), 14 deletions(-) > > > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c > > index 258093e9074d..0a48bf829671 100644 > > --- a/fs/ceph/caps.c > > +++ b/fs/ceph/caps.c > > @@ -3579,24 +3579,23 @@ 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 (newcaps & ~extra_info->issued) > > - wake = true; > > + if (le32_to_cpu(grant->op) == CEPH_CAP_OP_IMPORT) { > > + if (ci->i_auth_cap == cap) { > > + 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_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); > > } > > - > > - 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); > > } > > + spin_unlock(&ci->i_ceph_lock); > > > > if (fill_inline) > > ceph_fill_inline_data(inode, NULL, extra_info->inline_data, > > I am not sure what the following code in Line#4139 wants to do, more > detail please see [1]. > > 4131 if (msg_version >= 3) { > 4132 if (op == CEPH_CAP_OP_IMPORT) { > 4133 if (p + sizeof(*peer) > end) > 4134 goto bad; > 4135 peer = p; > 4136 p += sizeof(*peer); > 4137 } else if (op == CEPH_CAP_OP_EXPORT) { > 4138 /* recorded in unused fields */ > 4139 peer = (void *)&h->size; > 4140 } > 4141 } > > For the export case the members in the 'peer' are always assigned to > 'random' values. And seems could cause this issue. > > [1] https://github.com/ceph/ceph-client/blob/for-linus/fs/ceph/caps.c#L4134 > > I am still reading the code. Any idea ? > > I'm not sure that this is the case. It looks like most of the places in the mds code that make a CEPH_CAP_OP_EXPORT message call set_cap_peer just afterward. Why do you think this is given random values? -- Jeff Layton <jlayton@xxxxxxxxxx>
